From 8774d10bdf25fc345e9deaf51131e3582a7f6e23 Mon Sep 17 00:00:00 2001 From: Andreas Auernhammer Date: Fri, 5 Nov 2021 10:16:26 +0100 Subject: [PATCH] sts: always verify the key usage of client certificates (#13583) This commit makes the MinIO server behavior more consistent w.r.t. key usage verification. When MinIO verifies the client certificates it also checks that the client certificate is valid of client authentication (or any (i.e. wildcard) usage). However, the MinIO server used to not verify the client key usage when client certificate verification was disabled. Now, the MinIO server verifies the client key usage even when client certificate verification has been disabled. This makes the MinIO behavior more consistent from a client's perspective. Now, a client certificate has to be valid for client authentication in all cases. Signed-off-by: Andreas Auernhammer --- cmd/sts-handlers.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/cmd/sts-handlers.go b/cmd/sts-handlers.go index 3b6f3a1be..6a75c80ee 100644 --- a/cmd/sts-handlers.go +++ b/cmd/sts-handlers.go @@ -742,6 +742,31 @@ func (sts *stsAPIHandlers) AssumeRoleWithCertificate(w http.ResponseWriter, r *h writeSTSErrorResponse(ctx, w, true, ErrSTSInvalidClientCertificate, err) return } + } else { + // Technically, there is no security argument for verifying the key usage + // when we don't verify that the certificate has been issued by a trusted CA. + // Any client can create a certificate with arbitrary key usage settings. + // + // However, this check ensures that a certificate with an invalid key usage + // gets rejected even when we skip certificate verification. This helps + // clients detect malformed certificates during testing instead of e.g. + // a self-signed certificate that works while a comparable certificate + // issued by a trusted CA fails due to the MinIO server being less strict + // w.r.t. key usage verification. + // + // Basically, MinIO is more consistent (from a client perspective) when + // we verify the key usage all the time. + var validKeyUsage bool + for _, usage := range certificate.ExtKeyUsage { + if usage == x509.ExtKeyUsageAny || usage == x509.ExtKeyUsageClientAuth { + validKeyUsage = true + break + } + } + if !validKeyUsage { + writeSTSErrorResponse(ctx, w, true, ErrSTSMissingParameter, errors.New("certificate is not valid for client authentication")) + return + } } // We map the X.509 subject common name to the policy. So, a client