Add an option to allow cert-auth to return metadata about client cert 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

builtin/credential/cert/backend_test.go

@@ -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),
 		},
 	})
 }
@@ -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,
 		},
 	}
 }
@@ -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
 		},
 	}
@@ -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),

builtin/credential/cert/path_config.go

@@ -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{
@@ -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
 	}
@@ -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{
@@ -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"`
 }

builtin/credential/cert/path_login.go

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

builtin/credential/cert/path_login_test.go

@@ -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"
 )
 
@@ -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)
 			}
@@ -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"),
+		},
+	})
 }

changelog/29044.txt

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

sdk/logical/response.go

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

sdk/logical/response_test.go

@@ -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())
+}

vault/identity_store_entities.go

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

website/content/api-docs/auth/cert.mdx

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