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 <hi@aead.dev>
This commit is contained in:
Andreas Auernhammer 2021-11-05 10:16:26 +01:00 committed by GitHub
parent df9f479d58
commit 8774d10bdf
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 25 additions and 0 deletions

View File

@ -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