Vault SSH: CRUD test case for OTP Role

builtin/logical/ssh/backend_test.go

@@ -2,7 +2,6 @@ package ssh
 
 import (
 	"fmt"
-	"log"
 	"os/user"
 	"strings"
 	"testing"
@@ -16,8 +15,11 @@ import (
 )
 
 const (
+	testOTPKeyType       = "otp"
+	testDynamicKeyType   = "dynamic"
 	testCidr             = "127.0.0.1/32"
-	testRoleName         = "testRoleName"
+	testDynamicRoleName  = "testDynamicRoleName"
+	testOTPRoleName      = "testOTPRoleName"
 	testKey              = "testKey"
 	testSharedPrivateKey = `
 -----BEGIN RSA PRIVATE KEY-----
@@ -85,6 +87,66 @@ func TestSSHDynamicKeyBackend(t *testing.T) {
 	})
 }
 
+func TestSSHBackend_OTPRoleCrud(t *testing.T) {
+	data := map[string]interface{}{
+		"key_type":     testOTPKeyType,
+		"cidr":         testCidr,
+		"default_user": testUserName,
+	}
+	logicaltest.Test(t, logicaltest.TestCase{
+		Factory: Factory,
+		Steps: []logicaltest.TestStep{
+			testOTPRoleWrite(t, data),
+			testOTPRoleRead(t, data),
+			testOTPRoleDelete(t),
+			testOTPRoleRead(t, nil),
+		},
+	})
+}
+
+func testOTPRoleWrite(t *testing.T, data map[string]interface{}) logicaltest.TestStep {
+	return logicaltest.TestStep{
+		Operation: logical.WriteOperation,
+		Path:      "roles/" + testOTPRoleName,
+		Data:      data,
+	}
+}
+
+func testOTPRoleRead(t *testing.T, data map[string]interface{}) logicaltest.TestStep {
+	return logicaltest.TestStep{
+		Operation: logical.ReadOperation,
+		Path:      "roles/" + testOTPRoleName,
+		Check: func(resp *logical.Response) error {
+			if resp == nil {
+				if data == nil {
+					return nil
+				}
+				return fmt.Errorf("bad: %#v", resp)
+			}
+			var d struct {
+				KeyType     string `mapstructure:"key_type"`
+				DefaultUser string `mapstructure:"default_user"`
+				Port        string `mapstructure:"port"`
+				Cidr        string `mapstructure:"cidr"`
+			}
+			if err := mapstructure.Decode(resp.Data, &d); err != nil {
+				return err
+			}
+			if d.KeyType != data["key_type"] || d.DefaultUser != data["default_user"] || d.Cidr != data["cidr"] {
+				return fmt.Errorf("bad: %#v", resp)
+			}
+			return nil
+		},
+	}
+}
+
+func testOTPRoleDelete(t *testing.T) logicaltest.TestStep {
+	return logicaltest.TestStep{
+		Operation: logical.DeleteOperation,
+		Path:      "roles/" + testOTPRoleName,
+	}
+}
+
 func testNamedKeys(t *testing.T) logicaltest.TestStep {
 	return logicaltest.TestStep{
 		Operation: logical.WriteOperation,
@@ -98,7 +160,7 @@ func testNamedKeys(t *testing.T) logicaltest.TestStep {
 func testNewDynamicKeyRole(t *testing.T) logicaltest.TestStep {
 	return logicaltest.TestStep{
 		Operation: logical.WriteOperation,
-		Path:      fmt.Sprintf("roles/%s", testRoleName),
+		Path:      fmt.Sprintf("roles/%s", testDynamicRoleName),
 		Data: map[string]interface{}{
 			"key_type":   "dynamic",
 			"key":        testKey,
@@ -112,7 +174,7 @@ func testNewDynamicKeyRole(t *testing.T) logicaltest.TestStep {
 func testDynamicKeyCredsCreate(t *testing.T) logicaltest.TestStep {
 	return logicaltest.TestStep{
 		Operation: logical.WriteOperation,
-		Path:      fmt.Sprintf("creds/%s", testRoleName),
+		Path:      fmt.Sprintf("creds/%s", testDynamicRoleName),
 		Data: map[string]interface{}{
 			"username": testUserName,
 			"ip":       testIP,
@@ -124,7 +186,6 @@ func testDynamicKeyCredsCreate(t *testing.T) logicaltest.TestStep {
 			if err := mapstructure.Decode(resp.Data, &d); err != nil {
 				return err
 			}
-			log.Printf("[WARN] Generated Key:%s\n", d.Key)
 			if d.Key == "" {
 				return fmt.Errorf("Generated key is an empty string")
 			}

builtin/logical/ssh/path_creds_create.go

@@ -94,7 +94,6 @@ func (b *backend) pathCredsCreateWrite(
 
 	var result *logical.Response
 	if role.KeyType == KeyTypeOTP {
-		// Generate salted OTP
 		otp, err := b.GenerateOTPCredential(req, username, ip)
 		if err != nil {
 			return nil, err
@@ -127,10 +126,12 @@ func (b *backend) pathCredsCreateWrite(
 
 	// Change the lease information to reflect user's choice
 	lease, _ := b.Lease(req.Storage)
+
 	if lease != nil {
 		result.Secret.Lease = lease.Lease
 		result.Secret.LeaseGracePeriod = lease.LeaseMax
 	}
+
 	return result, nil
 }
 
@@ -177,7 +178,7 @@ func (b *backend) GenerateDynamicCredential(req *logical.Request, role *sshRole,
 	return dynamicPublicKey, dynamicPrivateKey, nil
 }
 
-// Generates an OTP and creates an entry for the same in storage backend.
+// Generates a salted OTP and creates an entry for the same in storage backend.
 func (b *backend) GenerateOTPCredential(req *logical.Request, username, ip string) (string, error) {
 	otp := uuid.GenerateUUID()
 	otpSalted := b.salt.SaltID(otp)

builtin/logical/ssh/path_roles.go

@@ -108,6 +108,7 @@ func (b *backend) pathRoleWrite(req *logical.Request, d *framework.FieldData) (*
 			DefaultUser: defaultUser,
 			CIDR:        cidr,
 			KeyType:     KeyTypeOTP,
+			Port:        port,
 		})
 	} else if keyType == KeyTypeDynamic {
 		keyName := d.Get("key").(string)
@@ -178,16 +179,27 @@ func (b *backend) pathRoleRead(req *logical.Request, d *framework.FieldData) (*l
 		return nil, err
 	}
 
-	return &logical.Response{
-		Data: map[string]interface{}{
-			"key":          role.KeyName,
-			"admin_user":   role.AdminUser,
-			"default_user": role.DefaultUser,
-			"cidr":         role.CIDR,
-			"port":         role.Port,
-			"key_type":     role.KeyType,
-		},
-	}, nil
+	if role.KeyType == KeyTypeOTP {
+		return &logical.Response{
+			Data: map[string]interface{}{
+				"default_user": role.DefaultUser,
+				"cidr":         role.CIDR,
+				"port":         role.Port,
+				"key_type":     role.KeyType,
+			},
+		}, nil
+	} else {
+		return &logical.Response{
+			Data: map[string]interface{}{
+				"key":          role.KeyName,
+				"admin_user":   role.AdminUser,
+				"default_user": role.DefaultUser,
+				"cidr":         role.CIDR,
+				"port":         role.Port,
+				"key_type":     role.KeyType,
+			},
+		}, nil
+	}
 }
 
 func (b *backend) pathRoleDelete(req *logical.Request, d *framework.FieldData) (*logical.Response, error) {
@@ -226,4 +238,31 @@ should have root access in all the hosts represented by the 'cidr'
 field. When the user requests key for an IP, the key will be installed
 for the user mentioned by 'default_user' field. The 'key' field takes
 a named key which can be configured by 'ssh/keys/' endpoint.
+
+Role Options:
+
+  -key_type		This can be either 'otp' or 'dynamic'. 'otp' key requires
+  			agent to be installed in target machine. Required field for
+			both types.
+
+  -key			Name of the key registered using 'keys/' endpoint. Required
+  			field for 'dynamic' type. Not applicable for 'otp' type.
+
+  -admin_user		Username at the target which is having root privileges. This
+  			username will be used to install keys for other unprivileged
+			users. Required field for 'dynamic' type. Not applicable for
+			'otp' type.
+
+  -default_user		When keys are created using '/creds' endpoint with only the
+  			IP address, by default, this username is used to create the
+			credentials. Required for 'otp' type. Optional for 'dynamic' type.
+
+  -cidr			CIDR block for which is role is applicable for. Required field
+  			for both types.
+
+  -port			Port number for SSH connections. Default is '22'. Optional for
+  			both types.
+
+  -key_bits		Length of RSa dynamic key in bits. Optional for 'dynamic' type.
+  			Not applicable for 'otp' type.
 `

builtin/logical/ssh/util.go

@@ -36,10 +36,7 @@ func uploadPublicKeyScp(publicKey, publicKeyFileName, username, ip, port, key st
 		fmt.Fprint(w, "\x00")
 		w.Close()
 	}()
-	err = session.Run(fmt.Sprintf("scp -vt %s", publicKeyFileName))
-	if err != nil {
-		return fmt.Errorf("public key upload failed")
-	}
+	session.Run(fmt.Sprintf("scp -vt %s", publicKeyFileName))
 	return nil
 }
 

command/server/config.go

@@ -17,9 +17,9 @@ type Config struct {
 	Listeners []*Listener `hcl:"-"`
 	Backend   *Backend    `hcl:"-"`
 
-	DisableMlock bool   `hcl:"disable_mlock"`
+	DisableMlock bool `hcl:"disable_mlock"`
 
-	Telemetry    *Telemetry `hcl:"telemetry"`
+	Telemetry *Telemetry `hcl:"telemetry"`
 }
 
 // DevConfig is a Config that is used for dev mode of Vault.
@@ -155,7 +155,7 @@ func LoadConfigFile(path string) (*Config, error) {
 
 		if statsdAddr != nil || statsiteAddr != nil {
 			result.Telemetry = &Telemetry{
-				StatsdAddr: getString(statsdAddr),
+				StatsdAddr:   getString(statsdAddr),
 				StatsiteAddr: getString(statsiteAddr),
 			}
 		}