Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Address cert auth error message logic error #27202

Merged
merged 1 commit into from
May 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
```
Loading