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

@@ -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,23 +1325,26 @@ 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
 			{
 				Operation: logical.UpdateOperation,
 				Path:      "sign/multikey",
 				Data: map[string]interface{}{
-					"public_key": publicKey4096,
+					"public_key":       publicKey4096,
+					"valid_principals": "toor",
 				},
 			},
 			// Fail with 3072-bit key
 			{
 				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,23 +1384,26 @@ 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
 			{
 				Operation: logical.UpdateOperation,
 				Path:      "sign/ectypes",
 				Data: map[string]interface{}{
-					"public_key": publicKeyECDSA521,
+					"public_key":       publicKeyECDSA521,
+					"valid_principals": "toor",
 				},
 			},
 			// Fail with RSA key
 			{
 				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)

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

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

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

@@ -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. (**potentially breaking**)
+```

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