Skip to content

Commit

Permalink
Address cert auth error message logic error (#27202)
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
stevendpclark authored May 24, 2024
1 parent e6c9bbb commit 2c3b41c
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 7 deletions.
13 changes: 8 additions & 5 deletions builtin/credential/cert/path_login.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions builtin/credential/cert/path_login_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions changelog/27202.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
auth/cert: Merge error messages returned in login failures and include error when present
```

0 comments on commit 2c3b41c

Please sign in to comment.