From 1a28208cf3b654e1afcd22ec0c88ca20629e2d24 Mon Sep 17 00:00:00 2001 From: Steven Clark Date: Thu, 23 May 2024 12:09:07 -0400 Subject: [PATCH] Address cert auth error message logic error - Cert auth had a logic error when crafting the error response when configured with a leaf certificate to validate against. It would generate an error response that used a nil error. - Make the cert auth error messages the same when we fail to match constraints --- builtin/credential/cert/path_login.go | 13 ++++++++----- builtin/credential/cert/path_login_test.go | 4 ++-- changelog/27202.txt | 3 +++ 3 files changed, 13 insertions(+), 7 deletions(-) create mode 100644 changelog/27202.txt diff --git a/builtin/credential/cert/path_login.go b/builtin/credential/cert/path_login.go index 33f62818ec09..47b902a5c87e 100644 --- a/builtin/credential/cert/path_login.go +++ b/builtin/credential/cert/path_login.go @@ -34,6 +34,8 @@ type ParsedCert struct { Certificates []*x509.Certificate } +const certAuthFailMsg = "failed to match all constraints for this login certificate" + func pathLogin(b *backend) *framework.Path { return &framework.Path{ Pattern: "login", @@ -312,10 +314,11 @@ func (b *backend) verifyCredentials(ctx context.Context, req *logical.Request, d // If no trusted chain was found, client is not authenticated // This check happens after checking for a matching configured non-CA certs if len(trustedChains) == 0 { - if retErr == nil { - return nil, logical.ErrorResponse(fmt.Sprintf("invalid certificate or no client certificate supplied; additionally got errors during verification: %v", retErr)), nil + if retErr != nil { + return nil, logical.ErrorResponse(fmt.Sprintf("%s; additionally got errors during verification: %v", certAuthFailMsg, retErr)), nil } - return nil, logical.ErrorResponse("invalid certificate or no client certificate supplied"), nil + + return nil, logical.ErrorResponse(certAuthFailMsg), nil } // Search for a ParsedCert that intersects with the validated chains and any additional constraints @@ -350,10 +353,10 @@ func (b *backend) verifyCredentials(ctx context.Context, req *logical.Request, d } if retErr != nil { - return nil, logical.ErrorResponse(fmt.Sprintf("no chain matching all constraints could be found for this login certificate; additionally got errors during verification: %v", retErr)), nil + return nil, logical.ErrorResponse(fmt.Sprintf("%s; additionally got errors during verification: %v", certAuthFailMsg, retErr)), nil } - return nil, logical.ErrorResponse("no chain matching all constraints could be found for this login certificate"), nil + return nil, logical.ErrorResponse(certAuthFailMsg), nil } func (b *backend) matchesConstraints(ctx context.Context, clientCert *x509.Certificate, trustedChain []*x509.Certificate, diff --git a/builtin/credential/cert/path_login_test.go b/builtin/credential/cert/path_login_test.go index 508c20b157ce..51d670ce8c40 100644 --- a/builtin/credential/cert/path_login_test.go +++ b/builtin/credential/cert/path_login_test.go @@ -202,7 +202,7 @@ func testAccStepResolveRoleExpectRoleResolutionToFail(t *testing.T, connState tl t.Fatal("Error not part of response.") } - if !strings.Contains(errString, "invalid certificate") { + if !strings.Contains(errString, certAuthFailMsg) { t.Fatalf("Error was not due to invalid role name. Error: %s", errString) } return nil @@ -230,7 +230,7 @@ func testAccStepResolveRoleOCSPFail(t *testing.T, connState tls.ConnectionState, t.Fatal("Error not part of response.") } - if !strings.Contains(errString, "no chain matching") { + if !strings.Contains(errString, certAuthFailMsg) { t.Fatalf("Error was not due to OCSP failure. Error: %s", errString) } return nil diff --git a/changelog/27202.txt b/changelog/27202.txt new file mode 100644 index 000000000000..224f976bba09 --- /dev/null +++ b/changelog/27202.txt @@ -0,0 +1,3 @@ +```release-note:bug +auth/cert: Merge error messages returned in login failures and include error when present +```