Update SSH CA logic/tests

builtin/logical/ssh/backend_test.go

@@ -8,14 +8,15 @@ import (
 
 	"golang.org/x/crypto/ssh"
 
+	"encoding/base64"
+	"errors"
+	"strings"
+
 	"github.com/hashicorp/vault/api"
 	"github.com/hashicorp/vault/logical"
 	logicaltest "github.com/hashicorp/vault/logical/testing"
 	"github.com/hashicorp/vault/vault"
 	"github.com/mitchellh/mapstructure"
-	"errors"
-	"strings"
-	"encoding/base64"
 )
 
 // Before the following tests are run, a username going by the name 'vaultssh' has
@@ -536,8 +537,7 @@ func TestBackend_AbleToAutoGenerateSigningKeys(t *testing.T) {
 			logicaltest.TestStep{
 				Operation: logical.UpdateOperation,
 				Path:      "config/ca",
-				Data: map[string]interface{}{
+				Data:      map[string]interface{}{},
-				},
 			},
 
 			logicaltest.TestStep{
@@ -576,10 +576,10 @@ func TestBackend_ValidPrincipalsValidatedForHostCertificates(t *testing.T) {
 			configCaStep(),
 
 			createRoleStep("testing", map[string]interface{}{
-				"key_type": "ca",
+				"key_type":                "ca",
 				"allow_host_certificates": true,
-				"allowed_domains": "example.com,example.org",
+				"allowed_domains":         "example.com,example.org",
-				"allow_subdomains":         true,
+				"allow_subdomains":        true,
 				"default_critical_options": map[string]interface{}{
 					"option": "value",
 				},
@@ -619,8 +619,10 @@ func TestBackend_OptionsOverrideDefaults(t *testing.T) {
 			configCaStep(),
 
 			createRoleStep("testing", map[string]interface{}{
-				"key_type": "ca",
+				"key_type":                 "ca",
-				"allow_user_certificates": true,
+				"allowed_users":            "tuber",
+				"default_user":             "tuber",
+				"allow_user_certificates":  true,
 				"allowed_critical_options": "option,secondary",
 				"allowed_extensions":       "extension,additional",
 				"default_critical_options": map[string]interface{}{
@@ -631,7 +633,7 @@ func TestBackend_OptionsOverrideDefaults(t *testing.T) {
 				},
 			}),
 
-			signCertificateStep("testing", "root", ssh.UserCert, nil, map[string]string{
+			signCertificateStep("testing", "root", ssh.UserCert, []string{"tuber"}, map[string]string{
 				"secondary": "value",
 			}, map[string]string{
 				"additional": "value",
@@ -670,8 +672,11 @@ func createRoleStep(name string, parameters map[string]interface{}) logicaltest.
 	}
 }
 
-func signCertificateStep(role, keyId string, certType int, validPrincipals []string, criticalOptionPermissions, extensionPermissions map[string]string, ttl time.Duration,
+func signCertificateStep(
-requestParameters map[string]interface{}) logicaltest.TestStep {
+	role, keyId string, certType int, validPrincipals []string,
+	criticalOptionPermissions, extensionPermissions map[string]string,
+	ttl time.Duration,
+	requestParameters map[string]interface{}) logicaltest.TestStep {
 	return logicaltest.TestStep{
 		Operation: logical.UpdateOperation,
 		Path:      "sign/" + role,
@@ -702,7 +707,7 @@ requestParameters map[string]interface{}) logicaltest.TestStep {
 }
 
 func validateSSHCertificate(cert *ssh.Certificate, keyId string, certType int, validPrincipals []string, criticalOptionPermissions, extensionPermissions map[string]string,
-ttl time.Duration) error {
+	ttl time.Duration) error {
 
 	if cert.KeyId != keyId {
 		return fmt.Errorf("Incorrect KeyId: %v", cert.KeyId)

builtin/logical/ssh/path_config_ca.go

@@ -48,23 +48,20 @@ func (b *backend) pathCAWrite(req *logical.Request, data *framework.FieldData) (
 	publicKey := data.Get("public_key").(string)
 	privateKey := data.Get("private_key").(string)
 
-	signingKeyGenerated := false
+	var generateSigningKey bool
+
 	generateSigningKeyRaw, ok := data.GetOk("generate_signing_key")
-	if ok {
+	switch {
-		if generateSigningKeyRaw.(bool) {
+	// explicitly set true
-			if publicKey != "" || privateKey != "" {
+	case ok && generateSigningKeyRaw.(bool):
-				return logical.ErrorResponse("public_key and private_key is not required when generate_signing_key is set to true"), nil
+		if publicKey != "" || privateKey != "" {
-			}
+			return logical.ErrorResponse("public_key and private_key must not be set when generate_signing_key is set to true"), nil
-
-			publicKey, privateKey, err = generateSSHKeyPair()
-			if err != nil {
-				return nil, err
-			}
-			signingKeyGenerated = true
 		}
-	}
 
-	if !signingKeyGenerated {
+		generateSigningKey = true
+
+		// explicitly set to false, or not set and we have both a public and private key
+	case ok, publicKey != "" && privateKey != "":
 		if publicKey == "" {
 			return logical.ErrorResponse("missing public_key"), nil
 		}
@@ -82,6 +79,25 @@ func (b *backend) pathCAWrite(req *logical.Request, data *framework.FieldData) (
 		if err != nil {
 			return logical.ErrorResponse(fmt.Sprintf("Unable to parse public_key as an SSH public key: %v", err)), nil
 		}
+
+		// not set and no public/private key provided so generate
+	case publicKey == "" && privateKey == "":
+		publicKey, privateKey, err = generateSSHKeyPair()
+		if err != nil {
+			return nil, err
+		}
+
+		generateSigningKey = true
+
+	default: // not set, but one or the other supplied
+		return logical.ErrorResponse("only one of public_key and private_key set; both must be set to use, or both must be blank to auto-generate"), nil
+	}
+
+	if generateSigningKey {
+		publicKey, privateKey, err = generateSSHKeyPair()
+		if err != nil {
+			return nil, err
+		}
 	}
 
 	if publicKey == "" || privateKey == "" {