High· 7.5GHSA-jg74-mwgw-v6x3 CVE-2024-7594CWE-732Published Sep 26, 2024

Vault SSH Secrets Engine Configuration Did Not Restrict Valid Principals By Default

Research is free — Hunters explains how the bug works, the root-cause code pattern, how the fix addresses it, and how to test whether a target is affected, in chat. Investigate & write exploit is a paid run — the engine reads the advisory and fix commits, then builds and validates a working proof-of-concept exploit with reproduction steps.

Affected versions

1.7.7 → fixed in 1.17.60.1.0 and later (no fix listed)0 → fixed in 0.0.0-20241003222810-d5b4e9224698

Details

Vault’s SSH secrets engine did not require the valid_principals list to contain a value by default. If the valid_principals and default_user fields of the SSH secrets engine configuration are not set, an SSH certificate requested by an authorized user to Vault’s SSH secrets engine could be used to authenticate as any user on the host. Fixed in Vault Community Edition 1.17.6, and in Vault Enterprise 1.17.6, 1.16.10, and 1.15.15.

The fix

Deny empty valid_principals during SSH issuance

Alexander Scheel· Sep 29, 2024, 02:16 PM+82−136052a411d0

builtin/logical/ssh/backend_test.go+43 −11

@@ -1261,6 +1261,7 @@ func TestBackend_AllowedUserKeyLengths(t *testing.T) {
 			createRoleStep("weakkey", map[string]interface{}{
 				"key_type":                "ca",
 				"allow_user_certificates": true,
+				"allowed_users":           "toor",
 				"allowed_user_key_lengths": map[string]interface{}{
 					"rsa": 4096,
 				},
@@ -1269,7 +1270,8 @@ func TestBackend_AllowedUserKeyLengths(t *testing.T) {
 				Operation: logical.UpdateOperation,
 				Path:      "sign/weakkey",
 				Data: map[string]interface{}{
-					"public_key": testCAPublicKey,
+					"public_key":       testCAPublicKey,
+					"valid_principals": "toor",
 				},
 				ErrorOk: true,
 				Check: func(resp *logical.Response) error {
@@ -1282,6 +1284,7 @@ func TestBackend_AllowedUserKeyLengths(t *testing.T) {
 			createRoleStep("stdkey", map[string]interface{}{
 				"key_type":                "ca",
 				"allow_user_certificates": true,
+				"allowed_users":           "*", // validate we can bypass allow_empty_principals=false
 				"allowed_user_key_lengths": map[string]interface{}{
 					"rsa": 2048,
 				},
@@ -1312,6 +1315,7 @@ func TestBackend_AllowedUserKeyLengths(t *testing.T) {
 			createRoleStep("multikey", map[string]interface{}{
 				"key_type":                "ca",
 				"allow_user_certificates": true,
+				"allowed_users":           "toor",
 				"allowed_user_key_lengths": map[string]interface{}{
 					"rsa": []int{2048, 4096},
 				},
@@ -1321,7 +1325,8 @@ func TestBackend_AllowedUserKeyLengths(t *testing.T) {
 				Operation: logical.UpdateOperation,
 				Path:      "sign/multikey",
 				Data: map[string]interface{}{
-					"public_key": testCAPublicKey,
+					"public_key":       testCAPublicKey,
+					"valid_principals": "toor",
 				},
 			},
 			// Pass with 4096-bit key
@@ -1329,7 +1334,8 @@ func TestBackend_AllowedUserKeyLengths(t *testing.T) {
 				Operation: logical.UpdateOperation,
 				Path:      "sign/multikey",
 				Data: map[string]interface{}{
-					"public_key": publicKey4096,
+					"public_key":       publicKey4096,
+					"valid_principals": "toor",
 				},
 			},
 			// Fail with 3072-bit key
@@ -1337,7 +1343,8 @@ func TestBackend_AllowedUserKeyLengths(t *testing.T) {
 				Operation: logical.UpdateOperation,
 				Path:      "sign/multikey",
 				Data: map[string]interface{}{
-					"public_key": publicKey3072,
+					"public_key":       publicKey3072,
+					"valid_principals": "toor",
 				},
 				ErrorOk: true,
 				Check: func(resp *logical.Response) error {
@@ -1352,7 +1359,8 @@ func TestBackend_AllowedUserKeyLengths(t *testing.T) {
 				Operation: logical.UpdateOperation,
 				Path:      "sign/multikey",
 				Data: map[string]interface{}{
-					"public_key": publicKeyECDSA256,
+					"public_key":       publicKeyECDSA256,
+					"valid_principals": "toor",
 				},
 				ErrorOk: true,
 				Check: func(resp *logical.Response) error {
@@ -1365,6 +1373,7 @@ func TestBackend_AllowedUserKeyLengths(t *testing.T) {
 			createRoleStep("ectypes", map[string]interface{}{
 				"key_type":                "ca",
 				"allow_user_certificates": true,
+				"allowed_users":           "toor",
 				"allowed_user_key_lengths": map[string]interface{}{
 					"ec":                  []int{256},
 					"ecdsa-sha2-nistp521": 0,
@@ -1375,7 +1384,8 @@ func TestBackend_AllowedUserKeyLengths(t *testing.T) {
 				Operation: logical.UpdateOperation,
 				Path:      "sign/ectypes",
 				Data: map[string]interface{}{
-					"public_key": publicKeyECDSA256,
+					"public_key":       publicKeyECDSA256,
+					"valid_principals": "toor",
 				},
 			},
 			// Pass with ECDSA P-521
@@ -1383,7 +1393,8 @@ func TestBackend_AllowedUserKeyLengths(t *testing.T) {
 				Operation: logical.UpdateOperation,
 				Path:      "sign/ectypes",
 				Data: map[string]interface{}{
-					"public_key": publicKeyECDSA521,
+					"public_key":       publicKeyECDSA521,
+					"valid_principals": "toor",
 				},
 			},
 			// Fail with RSA key
@@ -1391,7 +1402,8 @@ func TestBackend_AllowedUserKeyLengths(t *testing.T) {
 				Operation: logical.UpdateOperation,
 				Path:      "sign/ectypes",
 				Data: map[string]interface{}{
-					"public_key": publicKey3072,
+					"public_key":       publicKey3072,
+					"valid_principals": "toor",
 				},
 				ErrorOk: true,
 				Check: func(resp *logical.Response) error {
@@ -1840,6 +1852,7 @@ func TestSSHBackend_IssueSign(t *testing.T) {
 				"key_type":                "ca",
 				"allow_user_key_ids":      false,
 				"allow_user_certificates": true,
+				"allowed_users":           "*",
 				"allowed_user_key_lengths": map[string]interface{}{
 					"ssh-rsa":             []int{2048, 3072, 4096},
 					"ecdsa-sha2-nistp521": 0,
@@ -2117,8 +2130,9 @@ func issueSSHKeyPairStep(role, keyType string, keyBits int, expectError bool, er
 		Operation: logical.UpdateOperation,
 		Path:      "issue/" + role,
 		Data: map[string]interface{}{
-			"key_type": keyType,
-			"key_bits": keyBits,
+			"key_type":         keyType,
+			"key_bits":         keyBits,
+			"valid_principals": "toor",
 		},
 		ErrorOk: true,
 		Check: func(resp *logical.Response) error {
@@ -2675,13 +2689,31 @@ func TestProperAuthing(t *testing.T) {
 	_, err = client.Logical().WriteWithContext(ctx, "ssh/roles/test-ca", map[string]interface{}{
 		"key_type":                "ca",
 		"allow_user_certificates": true,
+		"allowed_users":           "*",
 	})
 	if err != nil {
 		t.Fatal(err)
 	}
  
 	_, err = client.Logical().WriteWithContext(ctx, "ssh/issue/test-ca", map[string]interface{}{
-		"username": "toor",
+		"valid_principals": "toor",
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+ 
+	_, err = client.Logical().WriteWithContext(ctx, "ssh/roles/test-ca-empty", map[string]interface{}{
+		"key_type":                "ca",
+		"allow_host_certificates": true,
+		"allow_empty_principals":  true,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+ 
+	_, err = client.Logical().WriteWithContext(ctx, "ssh/issue/test-ca-empty", map[string]interface{}{
+		"cert_type": "host",
+		"key_type":  "ssh-ed25519",
 	})
 	if err != nil {
 		t.Fatal(err)

builtin/logical/ssh/path_config_ca_test.go+2 −1

@@ -201,7 +201,8 @@ func createDeleteHelper(t *testing.T, b logical.Backend, config *logical.Backend
 	}
  
 	issueOptions := map[string]interface{}{
-		"public_key": testCAPublicKeyEd25519,
+		"public_key":       testCAPublicKeyEd25519,
+		"valid_principals": "toor",
 	}
 	issueReq := &logical.Request{
 		Path:      "sign/ca-issuance",

builtin/logical/ssh/path_issue_sign.go+4 −0

@@ -191,6 +191,10 @@ func (b *backend) calculateValidPrincipals(data *framework.FieldData, req *logic
  
 	switch {
 	case len(parsedPrincipals) == 0:
+		if !role.AllowEmptyPrincipals && principalsAllowedByRole != "*" {
+			return nil, fmt.Errorf("refusing to issue unsafe, globally-valid certificate with no principals specified; set valid_principals or default_user")
+		}
+ 
 		// There is nothing to process
 		return nil, nil
 	case len(allowedPrincipals) == 0:

builtin/logical/ssh/path_roles.go+20 −0

@@ -60,6 +60,7 @@ type sshRole struct {
 	AllowBareDomains           bool              `mapstructure:"allow_bare_domains" json:"allow_bare_domains"`
 	AllowSubdomains            bool              `mapstructure:"allow_subdomains" json:"allow_subdomains"`
 	AllowUserKeyIDs            bool              `mapstructure:"allow_user_key_ids" json:"allow_user_key_ids"`
+	AllowEmptyPrincipals       bool              `mapstructure:"allow_empty_principals" json:"allow_empty_principals"`
 	KeyIDFormat                string            `mapstructure:"key_id_format" json:"key_id_format"`
 	OldAllowedUserKeyLengths   map[string]int    `mapstructure:"allowed_user_key_lengths" json:"allowed_user_key_lengths,omitempty"`
 	AllowedUserKeyTypesLengths map[string][]int  `mapstructure:"allowed_user_key_types_lengths" json:"allowed_user_key_types_lengths"`
@@ -374,6 +375,23 @@ func pathRoles(b *backend) *framework.Path {
 					Value: 30,
 				},
 			},
+			"allow_empty_principals": {
+				Type: framework.TypeBool,
+				Description: `
+                [Optional for CA type]
+				If true, host and user certificates can be issued without any valid principals. For
+				host certificates, this means that any domain a host claims to be will be trusted by
+				the connecting client. For user certificates, when a CA certificate is placed in a
+				user's AuthorizedKeys file, any principal on that certificate will be allowed to
+				connect. When allowed_users or allowed_domains is set to * (corresponding to the
+				role/certificate type), allow_empty_principals=false still permits issuance.
+ 
+				It is recommend to leave this disabled.
+                `,
+				DisplayAttrs: &framework.DisplayAttributes{
+					Name: "Allow User Key IDs",
+				},
+			},
 		},
  
 		Callbacks: map[logical.Operation]framework.OperationFunc{
@@ -504,6 +522,7 @@ func (b *backend) createCARole(allowedUsers, defaultUser, signer string, data *f
 		AllowBareDomains:          data.Get("allow_bare_domains").(bool),
 		AllowSubdomains:           data.Get("allow_subdomains").(bool),
 		AllowUserKeyIDs:           data.Get("allow_user_key_ids").(bool),
+		AllowEmptyPrincipals:      data.Get("allow_empty_principals").(bool),
 		DefaultExtensionsTemplate: data.Get("default_extensions_template").(bool),
 		KeyIDFormat:               data.Get("key_id_format").(string),
 		KeyType:                   KeyTypeCA,
@@ -694,6 +713,7 @@ func (b *backend) parseRole(role *sshRole) (map[string]interface{}, error) {
 			"allow_bare_domains":          role.AllowBareDomains,
 			"allow_subdomains":            role.AllowSubdomains,
 			"allow_user_key_ids":          role.AllowUserKeyIDs,
+			"allow_empty_principals":      role.AllowEmptyPrincipals,
 			"key_id_format":               role.KeyIDFormat,
 			"key_type":                    role.KeyType,
 			"default_critical_options":    role.DefaultCriticalOptions,

website/content/api-docs/secret/ssh.mdx+9 −0

@@ -202,6 +202,15 @@ This endpoint creates or updates a named role.
 - `not_before_duration` `(duration: "30s")` – Specifies the duration by which to
   backdate the `ValidAfter` property. Uses [duration format strings](/docs/concepts/duration-format).
  
+- `allow_empty_principals` `(bool: false)` – If true, host and user
+certificates can be issued without any valid principals. For host
+certificates, this means that any domain a host claims to be will be trusted
+by the connecting client. For user certificates, when a CA certificate is
+placed in a user's AuthorizedKeys file, any principal on that certificate
+will be allowed to connect. When `allowed_users` or `allowed_domains` is set
+to `*` (corresponding to the role/certificate type),
+`allow_empty_principals=false` still permits issuance.
+ 
 ### Sample payload
  
 ```json
 changelog/561.txt | 3 +++
 1 file changed, 3 insertions(+)
 create mode 100644 changelog/561.txt

changelog/561.txt+3 −0

@@ -0,0 +1,3 @@
+```release-note:security
+secrets/ssh: Deny globally valid certificate issuance without valid_principals or allow_empty_principals override. HCSEC-2024-20 / CVE-2024-7594.
+```
 changelog/561.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

changelog/561.txt+1 −1

@@ -1,3 +1,3 @@
 ```release-note:security
-secrets/ssh: Deny globally valid certificate issuance without valid_principals or allow_empty_principals override. HCSEC-2024-20 / CVE-2024-7594.
+secrets/ssh: Deny globally valid certificate issuance without valid_principals or allow_empty_principals override. HCSEC-2024-20 / CVE-2024-7594. (**potentially breaking**)
 ```

Deny empty valid_principals during SSH issuance (#561)

Alexander Scheel· Oct 3, 2024, 10:28 PM+81−12d5b4e92246