Skip to content

Commit

Permalink
Add an option to allow cert-auth to return metadata about client cert…
Browse files Browse the repository at this point in the history
… that fails login (#29044)

* Add an option to allow cert-auth to return metadata about client certs that fail login

* Add cl

* Update SPDX header for sdk/logical/response_test.go
  • Loading branch information
stevendpclark authored Nov 29, 2024
1 parent e7d0165 commit 9bf3d11
Show file tree
Hide file tree
Showing 9 changed files with 232 additions and 14 deletions.
34 changes: 34 additions & 0 deletions builtin/credential/cert/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1317,6 +1317,8 @@ func TestBackend_ext_singleCert(t *testing.T) {
testAccStepLoginWithMetadata(t, connState, "web", map[string]string{"2-1-1-1": "A UTF8String Extension"}, true),
testAccStepCert(t, "web", ca, "foo", allowed{metadata_ext: "1.2.3.45"}, false),
testAccStepLoginWithMetadata(t, connState, "web", map[string]string{}, true),
testAccStepSetConfig(t, config{EnableMetadataOnFailures: true}, connState),
testAccStepReadConfig(t, config{EnableMetadataOnFailures: true}, connState),
},
})
}
Expand Down Expand Up @@ -1728,6 +1730,7 @@ func testAccStepSetConfig(t *testing.T, conf config, connState tls.ConnectionSta
ConnState: &connState,
Data: map[string]interface{}{
"enable_identity_alias_metadata": conf.EnableIdentityAliasMetadata,
"enable_metadata_on_failures": conf.EnableMetadataOnFailures,
},
}
}
Expand All @@ -1752,6 +1755,20 @@ func testAccStepReadConfig(t *testing.T, conf config, connState tls.ConnectionSt
t.Fatalf("bad: expected enable_identity_alias_metadata to be %t, got %t", conf.EnableIdentityAliasMetadata, b)
}

metaValueRaw, ok := resp.Data["enable_metadata_on_failures"]
if !ok {
t.Fatalf("enable_metadata_on_failures not found in response")
}

metaValue, ok := metaValueRaw.(bool)
if !ok {
t.Fatalf("bad: expected enable_metadata_on_failures to be a bool")
}

if metaValue != conf.EnableMetadataOnFailures {
t.Fatalf("bad: expected enable_metadata_on_failures to be %t, got %t", conf.EnableMetadataOnFailures, metaValue)
}

return nil
},
}
Expand Down Expand Up @@ -1936,6 +1953,23 @@ func testAccStepCert(t *testing.T, name string, cert []byte, policies string, te
return testAccStepCertWithExtraParams(t, name, cert, policies, testData, expectError, nil)
}

func testStepEnableMetadataFailures() logicaltest.TestStep {
return logicaltest.TestStep{
Operation: logical.UpdateOperation,
Path: "config",
ErrorOk: false,
Data: map[string]interface{}{
"enable_metadata_on_failures": true,
},
Check: func(resp *logical.Response) error {
if resp != nil && resp.IsError() {
return fmt.Errorf("expected nil response got a response error: %v", resp)
}
return nil
},
}
}

func testAccStepCertWithExtraParams(t *testing.T, name string, cert []byte, policies string, testData allowed, expectError bool, extraParams map[string]interface{}) logicaltest.TestStep {
data := map[string]interface{}{
"certificate": string(cert),
Expand Down
10 changes: 10 additions & 0 deletions builtin/credential/cert/path_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ func pathConfig(b *backend) *framework.Path {
Default: defaultRoleCacheSize,
Description: `The size of the in memory role cache`,
},
"enable_metadata_on_failures": {
Type: framework.TypeBool,
Default: false,
Description: `If set, metadata of the client certificate will be returned on authentication failures.`,
},
},

Operations: map[logical.Operation]framework.OperationHandler{
Expand Down Expand Up @@ -87,6 +92,9 @@ func (b *backend) pathConfigWrite(ctx context.Context, req *logical.Request, dat
}
config.RoleCacheSize = cacheSize
}
if enableMetadataOnFailures, ok := data.GetOk("enable_metadata_on_failures"); ok {
config.EnableMetadataOnFailures = enableMetadataOnFailures.(bool)
}
if err := b.storeConfig(ctx, req.Storage, config); err != nil {
return nil, err
}
Expand All @@ -104,6 +112,7 @@ func (b *backend) pathConfigRead(ctx context.Context, req *logical.Request, d *f
"enable_identity_alias_metadata": cfg.EnableIdentityAliasMetadata,
"ocsp_cache_size": cfg.OcspCacheSize,
"role_cache_size": cfg.RoleCacheSize,
"enable_metadata_on_failures": cfg.EnableMetadataOnFailures,
}

return &logical.Response{
Expand Down Expand Up @@ -133,4 +142,5 @@ type config struct {
EnableIdentityAliasMetadata bool `json:"enable_identity_alias_metadata"`
OcspCacheSize int `json:"ocsp_cache_size"`
RoleCacheSize int `json:"role_cache_size"`
EnableMetadataOnFailures bool `json:"enable_metadata_on_failures"`
}
64 changes: 61 additions & 3 deletions builtin/credential/cert/path_login.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,20 @@ func (b *backend) loginPathWrapper(wrappedOp func(ctx context.Context, req *logi
}

func (b *backend) pathLoginResolveRole(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
config, err := b.Config(ctx, req.Storage)
if err != nil {
return nil, err
}
if b.configUpdated.Load() {
b.updatedConfig(config)
}

var matched *ParsedCert

if verifyResp, resp, err := b.verifyCredentials(ctx, req, data); err != nil {
return nil, err
} else if resp != nil {
return resp, nil
return certAuthLoginFailureResponse(config, resp, req), nil
} else {
matched = verifyResp
}
Expand Down Expand Up @@ -118,7 +126,7 @@ func (b *backend) pathLogin(ctx context.Context, req *logical.Request, data *fra
if verifyResp, resp, err := b.verifyCredentials(ctx, req, data); err != nil {
return nil, err
} else if resp != nil {
return resp, nil
return certAuthLoginFailureResponse(config, resp, req), nil
} else {
matched = verifyResp
}
Expand Down Expand Up @@ -181,6 +189,56 @@ func (b *backend) pathLogin(ctx context.Context, req *logical.Request, data *fra
}, nil
}

func certAuthLoginFailureResponse(config *config, resp *logical.Response, req *logical.Request) *logical.Response {
if !config.EnableMetadataOnFailures || !resp.IsError() {
return resp
}
var initialErrMsg string
if err := resp.Error(); err != nil {
initialErrMsg = err.Error()
}

clientCert, exists := getClientCert(req)
if !exists {
return logical.ErrorResponse("no client certificate found\n" + initialErrMsg)
}

// Trim these values as they can be anything from any sort of failed certificate
// and we don't want to expose audit entries to randomly large strings.
const maxChars = 100
metadata := map[string]string{
"common_name": trimToMaxChars(clientCert.Subject.CommonName, maxChars),
"serial_number": trimToMaxChars(clientCert.SerialNumber.String(), maxChars),
"subject_key_id": trimToMaxChars(certutil.GetHexFormatted(clientCert.SubjectKeyId, ":"), maxChars),
"authority_key_id": trimToMaxChars(certutil.GetHexFormatted(clientCert.AuthorityKeyId, ":"), maxChars),
}

return logical.ErrorResponseWithData(metadata, initialErrMsg)
}

func getClientCert(req *logical.Request) (*x509.Certificate, bool) {
if req == nil || req.Connection == nil || req.Connection.ConnState == nil || req.Connection.ConnState.PeerCertificates == nil {
return nil, false
}
clientCerts := req.Connection.ConnState.PeerCertificates
if len(clientCerts) == 0 {
return nil, false
}
clientCert := clientCerts[0]
if clientCert == nil || clientCert.IsCA {
return nil, false
}
return clientCert, true
}

func trimToMaxChars(formatted string, maxSize int) string {
if len(formatted) > maxSize {
return formatted[:maxSize-3] + "..."
}

return formatted
}

func (b *backend) pathLoginRenew(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) {
config, err := b.Config(ctx, req.Storage)
if err != nil {
Expand All @@ -195,7 +253,7 @@ func (b *backend) pathLoginRenew(ctx context.Context, req *logical.Request, d *f
if verifyResp, resp, err := b.verifyCredentials(ctx, req, d); err != nil {
return nil, err
} else if resp != nil {
return resp, nil
return certAuthLoginFailureResponse(config, resp, req), nil
} else {
matched = verifyResp
}
Expand Down
88 changes: 86 additions & 2 deletions builtin/credential/cert/path_login_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
logicaltest "github.com/hashicorp/vault/helper/testhelpers/logical"
"github.com/hashicorp/vault/sdk/helper/certutil"
"github.com/hashicorp/vault/sdk/logical"
"github.com/stretchr/testify/require"
"golang.org/x/crypto/ocsp"
)

Expand Down Expand Up @@ -203,6 +204,51 @@ func testAccStepResolveRoleExpectRoleResolutionToFail(t *testing.T, connState tl
t.Fatal("Error not part of response.")
}

if _, dataKeyExists := resp.Data["data"]; dataKeyExists {
t.Fatal("metadata key 'data' existed in response without feature enabled")
}

if !strings.Contains(errString, certAuthFailMsg) {
t.Fatalf("Error was not due to invalid role name. Error: %s", errString)
}
return nil
},
Data: map[string]interface{}{
"name": certName,
},
}
}

func testAccStepResolveRoleExpectRoleResolutionToFailWithData(t *testing.T, connState tls.ConnectionState, certName string) logicaltest.TestStep {
return logicaltest.TestStep{
Operation: logical.ResolveRoleOperation,
Path: "login",
Unauthenticated: true,
ConnState: &connState,
ErrorOk: true,
Check: func(resp *logical.Response) error {
if resp == nil && !resp.IsError() {
t.Fatalf("Response was not an error: resp:%#v", resp)
}

errString, ok := resp.Data["error"].(string)
if !ok {
t.Fatal("Error not part of response.")
}

dataKeysRaw, dataKeyExists := resp.Data["data"]
if !dataKeyExists {
t.Fatal("metadata key 'data' did not exist in response feature enabled")
}
dataKeys, ok := dataKeysRaw.(map[string]string)
if !ok {
t.Fatalf("the 'data' field was not a map: %T", dataKeysRaw)
}

for _, key := range []string{"common_name", "serial_number", "authority_key_id", "subject_key_id"} {
require.Contains(t, dataKeys, key, "response metadata key %s was missing in response: %v", key, resp)
}

if !strings.Contains(errString, certAuthFailMsg) {
t.Fatalf("Error was not due to invalid role name. Error: %s", errString)
}
Expand Down Expand Up @@ -420,6 +466,44 @@ func TestCert_RoleResolveOCSP(t *testing.T) {
}
}

func serialFromBigInt(serial *big.Int) string {
return strings.TrimSpace(certutil.GetHexFormatted(serial.Bytes(), ":"))
// TestCert_MetadataOnFailure verifies that we return the cert metadata
// in the response on failures if the configuration option is enabled.
func TestCert_MetadataOnFailure(t *testing.T) {
certTemplate := &x509.Certificate{
Subject: pkix.Name{
CommonName: "example.com",
},
DNSNames: []string{"example.com"},
IPAddresses: []net.IP{net.ParseIP("127.0.0.1")},
ExtKeyUsage: []x509.ExtKeyUsage{
x509.ExtKeyUsageServerAuth,
x509.ExtKeyUsageClientAuth,
},
KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment | x509.KeyUsageKeyAgreement,
SerialNumber: big.NewInt(mathrand.Int63()),
NotBefore: time.Now().Add(-30 * time.Second),
NotAfter: time.Now().Add(262980 * time.Hour),
}

tempDir, connState, err := generateTestCertAndConnState(t, certTemplate)
if tempDir != "" {
defer os.RemoveAll(tempDir)
}
if err != nil {
t.Fatalf("error testing connection state: %v", err)
}
ca, err := ioutil.ReadFile(filepath.Join(tempDir, "ca_cert.pem"))
if err != nil {
t.Fatalf("err: %v", err)
}

logicaltest.Test(t, logicaltest.TestCase{
CredentialBackend: testFactory(t),
Steps: []logicaltest.TestStep{
testStepEnableMetadataFailures(),
testAccStepCert(t, "web", ca, "foo", allowed{dns: "example.com"}, false),
testAccStepLoginWithName(t, connState, "web"),
testAccStepResolveRoleExpectRoleResolutionToFailWithData(t, connState, "notweb"),
},
})
}
3 changes: 3 additions & 0 deletions changelog/29044.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
auth/cert: Add new configuration option `enable_metadata_on_failures` to add client cert metadata on login failures to audit log and response
```
9 changes: 9 additions & 0 deletions sdk/logical/response.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,15 @@ func ErrorResponse(text string, vargs ...interface{}) *Response {
}
}

// ErrorResponseWithData is used to format an error response with additional data returned
// within the "data" sub-field of the Data field. Useful to return additional information to the client
// and or appear within audited responses.
func ErrorResponseWithData(data interface{}, text string, vargs ...interface{}) *Response {
resp := ErrorResponse(text, vargs...)
resp.Data["data"] = data
return resp
}

// ListResponse is used to format a response to a list operation.
func ListResponse(keys []string) *Response {
resp := &Response{
Expand Down
24 changes: 24 additions & 0 deletions sdk/logical/response_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: MPL-2.0

package logical

import (
"testing"

"github.com/stretchr/testify/assert"
)

// TestResponse_ErrorResponse validates that our helper functions produce responses
// that we consider errors.
func TestResponse_ErrorResponse(t *testing.T) {
simpleResp := ErrorResponse("a test %s", "error")
assert.True(t, simpleResp.IsError())

dataMap := map[string]string{
"test1": "testing",
}

withDataResp := ErrorResponseWithData(dataMap, "a test %s", "error")
assert.True(t, withDataResp.IsError())
}
11 changes: 2 additions & 9 deletions vault/identity_store_entities.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
"strings"

"github.com/golang/protobuf/ptypes"
memdb "github.com/hashicorp/go-memdb"
"github.com/hashicorp/go-memdb"
"github.com/hashicorp/go-multierror"
"github.com/hashicorp/go-secure-stdlib/strutil"
"github.com/hashicorp/vault/helper/identity"
Expand Down Expand Up @@ -286,14 +286,7 @@ func (i *IdentityStore) pathEntityMergeID() framework.OperationFunc {
return logical.ErrorResponse(userErr.Error()), nil
}
// Alias clash error, so include additional details
resp := &logical.Response{
Data: map[string]interface{}{
"error": userErr.Error(),
"data": aliases,
},
}

return resp, nil
return logical.ErrorResponseWithData(aliases, userErr.Error()), nil
}
if intErr != nil {
return nil, intErr
Expand Down
3 changes: 3 additions & 0 deletions website/content/api-docs/auth/cert.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,9 @@ Configuration options for the method.
that this cache is used for all configured certificates.
- `role_cache_size` `(int: 200)` - The size of the role cache. Use `-1` to disable
role caching.
- `enable_metadata_on_failures` `(boolean: false)` - If set, metadata of the client
certificate such as common name, serial, subject key id and authority key id will
be returned on authentication failures and appear in auditing records.

### Sample payload

Expand Down

0 comments on commit 9bf3d11

Please sign in to comment.