Merge 12f03b0 into backport/sgm/allow_empty_principals_flag/nationall…

…y-assuring-rabbit

builtin/logical/ssh/backend_test.go

@@ -1298,6 +1298,80 @@ func TestBackend_OptionsOverrideDefaults(t *testing.T) {
 	logicaltest.Test(t, testCase)
 }
 
+func TestBackend_EmptyPrincipals(t *testing.T) {
+	config := logical.TestBackendConfig()
+
+	b, err := Factory(context.Background(), config)
+	if err != nil {
+		t.Fatalf("Cannot create backend: %s", err)
+	}
+	testCase := logicaltest.TestCase{
+		LogicalBackend: b,
+		Steps: []logicaltest.TestStep{
+			configCaStep(testCAPublicKey, testCAPrivateKey),
+			createRoleStep("no_user_principals", map[string]interface{}{
+				"key_type":                "ca",
+				"allow_user_certificates": true,
+				"allowed_user_key_lengths": map[string]interface{}{
+					"rsa": 2048,
+				},
+				"allowed_users": "no_principals",
+			}),
+			{
+				Operation: logical.UpdateOperation,
+				Path:      "sign/no_user_principals",
+				Data: map[string]interface{}{
+					"public_key": testCAPublicKey,
+				},
+				ErrorOk: true,
+				Check: func(resp *logical.Response) error {
+					if resp.Data["error"] != "empty valid principals not allowed by role" {
+						return errors.New("expected empty valid principals not allowed by role")
+					}
+					return nil
+				},
+			},
+			createRoleStep("no_host_principals", map[string]interface{}{
+				"key_type":                "ca",
+				"allow_host_certificates": true,
+				"allowed_domains":         "*",
+			}),
+			{
+				Operation: logical.UpdateOperation,
+				Path:      "sign/no_host_principals",
+				Data: map[string]interface{}{
+					"cert_type":  "host",
+					"public_key": testCAPublicKeyEd25519,
+				},
+				ErrorOk: true,
+				Check: func(resp *logical.Response) error {
+					if resp.Data["error"] != "empty valid principals not allowed by role" {
+						return errors.New("expected empty valid principals not allowed by role")
+					}
+					return nil
+				},
+			},
+			{
+				Operation: logical.UpdateOperation,
+				Path:      "sign/no_host_principals",
+				Data: map[string]interface{}{
+					"cert_type":        "host",
+					"public_key":       testCAPublicKeyEd25519,
+					"valid_principals": "example.com",
+				},
+				ErrorOk: true,
+				Check: func(resp *logical.Response) error {
+					if resp.Data["error"] != nil {
+						return errors.New("expected no error")
+					}
+					return nil
+				},
+			},
+		},
+	}
+	logicaltest.Test(t, testCase)
+}
+
 func TestBackend_AllowedUserKeyLengths(t *testing.T) {
 	config := logical.TestBackendConfig()
 
@@ -1315,6 +1389,7 @@ func TestBackend_AllowedUserKeyLengths(t *testing.T) {
 				"allowed_user_key_lengths": map[string]interface{}{
 					"rsa": 4096,
 				},
+				"allowed_users": "guest",
 			}),
 			{
 				Operation: logical.UpdateOperation,
@@ -1336,21 +1411,24 @@ func TestBackend_AllowedUserKeyLengths(t *testing.T) {
 				"allowed_user_key_lengths": map[string]interface{}{
 					"rsa": 2048,
 				},
+				"allowed_users": "guest",
 			}),
 			// Pass with 2048 key
 			{
 				Operation: logical.UpdateOperation,
 				Path:      "sign/stdkey",
 				Data: map[string]interface{}{
-					"public_key": testCAPublicKey,
+					"public_key":       testCAPublicKey,
+					"valid_principals": "guest",
 				},
 			},
 			// Fail with 4096 key
 			{
 				Operation: logical.UpdateOperation,
 				Path:      "sign/stdkey",
 				Data: map[string]interface{}{
-					"public_key": publicKey4096,
+					"public_key":       publicKey4096,
+					"valid_principals": "guest",
 				},
 				ErrorOk: true,
 				Check: func(resp *logical.Response) error {
@@ -1366,21 +1444,24 @@ func TestBackend_AllowedUserKeyLengths(t *testing.T) {
 				"allowed_user_key_lengths": map[string]interface{}{
 					"rsa": []int{2048, 4096},
 				},
+				"allowed_users": "guest",
 			}),
 			// Pass with 2048-bit key
 			{
 				Operation: logical.UpdateOperation,
 				Path:      "sign/multikey",
 				Data: map[string]interface{}{
-					"public_key": testCAPublicKey,
+					"public_key":       testCAPublicKey,
+					"valid_principals": "guest",
 				},
 			},
 			// Pass with 4096-bit key
 			{
 				Operation: logical.UpdateOperation,
 				Path:      "sign/multikey",
 				Data: map[string]interface{}{
-					"public_key": publicKey4096,
+					"public_key":       publicKey4096,
+					"valid_principals": "guest",
 				},
 			},
 			// Fail with 3072-bit key
@@ -1403,7 +1484,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": "guest",
 				},
 				ErrorOk: true,
 				Check: func(resp *logical.Response) error {
@@ -1420,29 +1502,33 @@ func TestBackend_AllowedUserKeyLengths(t *testing.T) {
 					"ec":                  []int{256},
 					"ecdsa-sha2-nistp521": 0,
 				},
+				"allowed_users": "guest",
 			}),
 			// Pass with ECDSA P-256
 			{
 				Operation: logical.UpdateOperation,
 				Path:      "sign/ectypes",
 				Data: map[string]interface{}{
-					"public_key": publicKeyECDSA256,
+					"public_key":       publicKeyECDSA256,
+					"valid_principals": "guest",
 				},
 			},
 			// Pass with ECDSA P-521
 			{
 				Operation: logical.UpdateOperation,
 				Path:      "sign/ectypes",
 				Data: map[string]interface{}{
-					"public_key": publicKeyECDSA521,
+					"public_key":       publicKeyECDSA521,
+					"valid_principals": "guest",
 				},
 			},
 			// Fail with RSA key
 			{
 				Operation: logical.UpdateOperation,
 				Path:      "sign/ectypes",
 				Data: map[string]interface{}{
-					"public_key": publicKey3072,
+					"public_key":       publicKey3072,
+					"valid_principals": "guest",
 				},
 				ErrorOk: true,
 				Check: func(resp *logical.Response) error {
@@ -1896,6 +1982,7 @@ func TestSSHBackend_IssueSign(t *testing.T) {
 					"ecdsa-sha2-nistp521": 0,
 					"ed25519":             0,
 				},
+				"allow_empty_principals": true,
 			}),
 			// Key_type not in allowed_user_key_types_lengths
 			issueSSHKeyPairStep("testing", "ec", 256, true, "provided key_type value not in allowed_user_key_types"),
@@ -2726,13 +2813,14 @@ 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":           "toor",
 	})
 	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)

builtin/logical/ssh/path_config_ca_test.go

@@ -259,6 +259,7 @@ func TestSSH_ConfigCAKeyTypes(t *testing.T) {
 		"key_type":                "ca",
 		"ttl":                     "30s",
 		"not_before_duration":     "2h",
+		"allow_empty_principals":  true,
 	}
 	roleReq := &logical.Request{
 		Operation: logical.UpdateOperation,

builtin/logical/ssh/path_issue.go

@@ -58,7 +58,7 @@ be later than the role max TTL.`,
 			},
 			"valid_principals": {
 				Type:        framework.TypeString,
-				Description: `Valid principals, either usernames or hostnames, that the certificate should be signed for.`,
+				Description: `Valid principals, either usernames or hostnames, that the certificate should be signed for.  Must be non-empty unless allow_empty_principals=true (not recommended) or a value for DefaultUser has been set in the role`,
 			},
 			"cert_type": {
 				Type:        framework.TypeString,

builtin/logical/ssh/path_issue_sign.go

@@ -189,10 +189,19 @@ func (b *backend) calculateValidPrincipals(data *framework.FieldData, req *logic
 		allowedPrincipals = strutil.RemoveDuplicates(strutil.ParseStringSlice(principalsAllowedByRole, ","), false)
 	}
 
+	if len(parsedPrincipals) == 0 && defaultPrincipal != "" {
+		// defaultPrincipal will either be the defaultUser or a rendered defaultUserTemplate
+		parsedPrincipals = []string{defaultPrincipal}
+	}
+
 	switch {
 	case len(parsedPrincipals) == 0:
-		// There is nothing to process
-		return nil, nil
+		if role.AllowEmptyPrincipals {
+			// There is nothing to process
+			return nil, nil
+		} else {
+			return nil, fmt.Errorf("empty valid principals not allowed by role")
+		}
 	case len(allowedPrincipals) == 0:
 		// User has requested principals to be set, but role is not configured
 		// with any principals

builtin/logical/ssh/path_roles.go

@@ -66,6 +66,7 @@ type sshRole struct {
 	AlgorithmSigner            string            `mapstructure:"algorithm_signer" json:"algorithm_signer"`
 	Version                    int               `mapstructure:"role_version" json:"role_version"`
 	NotBeforeDuration          time.Duration     `mapstructure:"not_before_duration" json:"not_before_duration"`
+	AllowEmptyPrincipals       bool              `mapstructure:"allow_empty_principals" json:"allow_empty_principals"`
 }
 
 func pathListRoles(b *backend) *framework.Path {
@@ -363,6 +364,11 @@ func pathRoles(b *backend) *framework.Path {
 					Value: 30,
 				},
 			},
+			"allow_empty_principals": {
+				Type:        framework.TypeBool,
+				Description: `Whether to allow issuing certificates with no valid principals (meaning any valid principal).  Exists for backwards compatibility only, the default of false is highly recommended.`,
+				Default:     false,
+			},
 		},
 
 		Callbacks: map[logical.Operation]framework.OperationFunc{
@@ -499,6 +505,7 @@ func (b *backend) createCARole(allowedUsers, defaultUser, signer string, data *f
 		AlgorithmSigner:           signer,
 		Version:                   roleEntryVersion,
 		NotBeforeDuration:         time.Duration(data.Get("not_before_duration").(int)) * time.Second,
+		AllowEmptyPrincipals:      data.Get("allow_empty_principals").(bool),
 	}
 
 	if !role.AllowUserCertificates && !role.AllowHostCertificates {

changelog/28466.txt

@@ -0,0 +1,3 @@
+```release-note:change
+secrets/ssh: Add a flag, `allow_empty_principals` to allow keys or certs to apply to any user/principal.
+```

website/content/api-docs/secret/ssh.mdx

@@ -171,6 +171,10 @@ This endpoint creates or updates a named role.
   ~> **Note**: In FIPS 140-2 mode, the following algorithms are not certified
      and thus should not be used: `ed25519`.
 
+- `allow_empty_principals` `(bool: false)` - Allow signing certificates with
+  no valid principals (e.g. any valid principal).  For backwards
+  compatibility only.  The default of false is highly recommended.
+
 - `algorithm_signer` `(string: "default")` - Algorithm to sign keys with. Valid
   values are `ssh-rsa`, `rsa-sha2-256`, `rsa-sha2-512`, or `default`. This
   value may also be left blank to use the signer's default algorithm, and must
@@ -190,6 +194,10 @@ 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](/vault/docs/concepts/duration-format).
 
+- `allow_empty_principals` `(bool: false)` - If true, allows certificates
+  to be issued against all principals.  Highly recommended to use the default of
+  false.
+
 ### Sample payload
 
 ```json
@@ -744,7 +752,8 @@ parameters of the issued certificate can be further customized in this API call.
   set.
 
 - `valid_principals` `(string: "")` – Specifies valid principals, either
-  usernames or hostnames, that the certificate should be signed for.
+  usernames or hostnames, that the certificate should be signed for.  Required
+  unless the role has specified allow_empty_principals.
 
 - `cert_type` `(string: "user")` – Specifies the type of certificate to be
   created; either "user" or "host".
@@ -830,7 +839,8 @@ parameters of the issued certificate can be further customized in this API call.
   set.
 
 - `valid_principals` `(string: "")` – Specifies valid principals, either
-  usernames or hostnames, that the certificate should be signed for.
+  usernames or hostnames, that the certificate should be signed for.  Required
+  unless the role has specified allow_empty_principals.
 
 - `cert_type` `(string: "user")` – Specifies the type of certificate to be
   created; either "user" or "host".
@@ -926,4 +936,4 @@ $ curl \
   "warnings": null,
   "auth": null
 }
-```
+```