Vault SSH: Documentation update and minor refactoring changes.

api/ssh.go

@@ -2,8 +2,6 @@ package api
 
 import "fmt"
 
-const SSHDefaultMountPoint = "ssh"
-
 // SSH is used to return a client to invoke operations on SSH backend.
 type SSH struct {
 	c          *Client
@@ -12,7 +10,7 @@ type SSH struct {
 
 // Returns the client for logical-backend API calls.
 func (c *Client) SSH() *SSH {
-	return c.SSHWithMountPoint(SSHDefaultMountPoint)
+	return c.SSHWithMountPoint(SSHAgentDefaultMountPoint)
 }
 
 // Returns the client with specific SSH mount point.

api/ssh_agent.go

@@ -17,7 +17,7 @@ import (
 )
 
 const (
-	// Default path at which SSH backend will be mounted
+	// Default path at which SSH backend will be mounted in Vault server
 	SSHAgentDefaultMountPoint = "ssh"
 
 	// Echo request message sent as OTP by the agent
@@ -38,9 +38,15 @@ type SSHAgent struct {
 // SSHVerifyResp is a structure representing the fields in Vault server's
 // response.
 type SSHVerifyResponse struct {
-	Message  string `mapstructure:"message"`
+	// Usually empty. If the request OTP is echo request message, this will
+	// be set to the corresponding echo response message.
+	Message string `mapstructure:"message"`
+
+	// Username associated with the OTP
 	Username string `mapstructure:"username"`
-	IP       string `mapstructure:"ip"`
+
+	// IP associated with the OTP
+	IP string `mapstructure:"ip"`
 }
 
 // Structure which represents the entries from the agent's configuration file.
@@ -53,7 +59,7 @@ type SSHAgentConfig struct {
 	AllowedCidrList string `hcl:"allowed_cidr_list"`
 }
 
-// Returns a HTTP client that uses TLS verification (TLS 1.2) with the given
+// Returns a HTTP client that uses TLS verification (TLS 1.2) for a given
 // certificate pool.
 func (c *SSHAgentConfig) TLSClient(certPool *x509.CertPool) *http.Client {
 	tlsConfig := &tls.Config{
@@ -113,7 +119,10 @@ func (c *SSHAgentConfig) NewClient() (*Client, error) {
 }
 
 // Load agent's configuration from the file and populate the corresponding
-// in-memory structure. Vault address and SSH mount points required parameters.
+// in-memory structure.
+//
+// Vault address is a required parameter.
+// Mount point defaults to "ssh".
 func LoadSSHAgentConfig(path string) (*SSHAgentConfig, error) {
 	var config SSHAgentConfig
 	contents, err := ioutil.ReadFile(path)
@@ -134,7 +143,7 @@ func LoadSSHAgentConfig(path string) (*SSHAgentConfig, error) {
 		return nil, fmt.Errorf("config missing vault_addr")
 	}
 	if config.SSHMountPoint == "" {
-		return nil, fmt.Errorf("config missing ssh_mount_point")
+		config.SSHMountPoint = SSHAgentDefaultMountPoint
 	}
 
 	return &config, nil
@@ -155,9 +164,11 @@ func (c *Client) SSHAgentWithMountPoint(mountPoint string) *SSHAgent {
 	}
 }
 
-// Verifies if the key provided by user is present in Vault server. If yes,
+// Verifies if the key provided by user is present in Vault server. The response
-// the response will contain the IP address and username associated with the
+// will contain the IP address and username associated with the OTP. In case the
-// key.
+// OTP matches the echo request message, instead of searching an entry for the OTP,
+// an echo response message is returned. This feature is used by agent to verify if
+// its configured correctly.
 func (c *SSHAgent) Verify(otp string) (*SSHVerifyResponse, error) {
 	data := map[string]interface{}{
 		"otp": otp,

builtin/logical/ssh/backend.go

@@ -60,31 +60,30 @@ func Backend(conf *logical.BackendConfig) (*framework.Backend, error) {
 }
 
 const backendHelp = `
-The SSH backend generates keys to eatablish SSH connection
+The SSH backend generates credentials to establish SSH connection with remote hosts.
-with remote hosts. There are two options to create the keys:
+There are two types of credentials that could be generated: Dynamic and OTP. The
-long lived dynamic key, one time password. 
+desired way of key creation should be chosen by using 'key_type' parameter of 'roles/'
+endpoint. When a credential is requested for a particular role, Vault will generate
+a credential accordingly and issue it.
 
-Long lived dynamic key is a rsa private key which can be used
+Dynamic Key: is a RSA private key which can be used to establish SSH session using
-to login to remote host using the publickey authentication.
+publickey authentication. When the client receives a key and uses it to establish
-There is no additional change required in the remote hosts to
+connections with hosts, Vault server will have no way to know when and how many 
-support this type of keys. But the keys generated will be valid
+times the key will be used. So, these login attempts will not be audited by Vault.
-as long as the lease of the key is valid. Also, logins to remote
+To create a dynamic credential, Vault will use the shared private key registered
-hosts will not be audited in vault server.
+with the role. Named key should be created using 'keys/' endpoint and used with
+'roles/' endpoint for Vault to know the shared key to use for installing the newly
+generated key. Since Vault uses the shared key to install keys for other usernames,
+shared key should have sudoer privileges in remote hosts and password prompts for
+sudoers should be disabled. Also, dynamic keys are leased keys and gets revoked
+in remote hosts by Vault after the expiry.
 
-One Time Password (OTP), on the other hand is a randomly generated
+OTP Key: is a UUID which can be used to login using keyboard-interactive authentication.
-UUID that is used to login to remote host using the keyboard-
+All the hosts that intend to support OTP should have Vault SSH Agent installed in
-interactive challenge response authentication. A vault agent
+them. This agent will receive the OTP from client and get it validated by Vault server.
-has to be installed at the remote host to support OTP. Upon 
+And since Vault server has a role to play for each successful connection, all the
-request, vault server generates and provides the key to the
+events will be audited. Vault server validates a key only once, hence it is a OTP.
-user. During login, vault agent receives the key and verifies
-the correctness with the vault server (and hence audited). The
-server after verifying the key for the first time, deletes the
-same (and hence one-time).
 
-Both type of keys have a configurable lease set and are automatically
+After mounting this backend, before generating the keys, configure the lease using
-revoked at the end of the lease.
+'congig/lease' endpoint and create roles using 'roles/' endpoint.
-
-After mounting this backend, before generating the keys, configure
-the lease using the 'config/lease' endpoint and create roles using
-the 'roles/' endpoint.
 `

builtin/logical/ssh/linux_install_script.go

@@ -1,7 +1,9 @@
 package ssh
 
 const (
-	LinuxInstallScript = `
+	// This is a constant representing a script to install and uninstall public
+	// key in remote hosts.
+	DefaultPublicKeyInstallScript = `
 #!/bin/bash
 #
 # This script file installs or uninstalls an RSA public key to/from authoried_keys
@@ -16,27 +18,29 @@ const (
 # as file name to avoid collisions with public keys generated for requests.
 #
 # $3: Absolute path of the authorized_keys file.
+# Currently, vault uses /home/<username>/.ssh/authorized_keys as the path.
 #
-# [Note: Modify the script if targt machine does not have the commands used in
+# [Note: This is a default script and is written to provide convenience.
-# this script]
+# If the host platform differs, or if the binaries used in this script are not
+# available, write a new script that takes the above parameters and does the
+# same task as this script, and register it Vault while role creation using
+# 'install_script' parameter.
 
 if [ $1 != "install" && $1 != "uninstall" ]; then
 	exit 1
 fi
 
-# If the key being installed is already present in the authorized_keys file, it is
+# Remove the key from authorized_key file if it is already present.
-# removed and the result is stored in a temporary file.
+# This step is common for both installing and uninstalling the key.
 grep -vFf $2 $3 > temp_$2
-
-# Contents of temporary file will be the contents of authorized_keys file.
 cat temp_$2 | sudo tee $3
 
 if [ $1 == "install" ]; then
-# New public key is appended to authorized_keys file
+# Append the new public key to authorized_keys file
 cat $2 | sudo tee --append $3
 fi
 
-# Auxiliary files are deleted
+# Delete the auxiliary files
 rm -f $2 temp_$2
 `
 )

builtin/logical/ssh/path_config_lease.go

@@ -99,10 +99,9 @@ Configure the default lease information for SSH dynamic keys.
 `
 
 const pathConfigLeaseHelpDesc = `
-This configures the default lease information used for SSH keys 
+This configures the default lease information used for SSH keys generated by
-generated by this backend. The lease specifies the duration that a
+this backend. The lease specifies the duration that a credential will be valid
-credential will be valid for, as well as the maximum session for
+for, as well as the maximum session for a set of credentials.
-a set of credentials.
 
 The format for the lease is "1h" or integer and then unit. The longest
 unit is hour.

builtin/logical/ssh/path_creds_create.go

@@ -41,18 +41,6 @@ func pathCredsCreate(b *backend) *framework.Path {
 	}
 }
 
-// Checks if the username supplied by the user is present in the list of
-// allowed users registered which creation of role.
-func validateUsername(username, allowedUsers string) error {
-	userList := strings.Split(allowedUsers, ",")
-	for _, user := range userList {
-		if user == username {
-			return nil
-		}
-	}
-	return fmt.Errorf("username not in allowed users list")
-}
-
 func (b *backend) pathCredsCreateWrite(
 	req *logical.Request, d *framework.FieldData) (*logical.Response, error) {
 	roleName := d.Get("role").(string)
@@ -73,7 +61,9 @@ func (b *backend) pathCredsCreateWrite(
 		return logical.ErrorResponse(fmt.Sprintf("Role '%s' not found", roleName)), nil
 	}
 
+	// username is an optional parameter.
 	username := d.Get("username").(string)
+
 	// Set the default username
 	if username == "" {
 		if role.DefaultUser == "" {
@@ -112,10 +102,16 @@ func (b *backend) pathCredsCreateWrite(
 
 	var result *logical.Response
 	if role.KeyType == KeyTypeOTP {
+		// Generate an OTP
 		otp, err := b.GenerateOTPCredential(req, username, ip)
 		if err != nil {
 			return nil, err
 		}
+
+		// Return the information relevant to user of OTP type and save
+		// the data required for later use in the internal section of secret.
+		// In this case, saving just the OTP is sufficient since there is
+		// no need to establish connection with the remote host.
 		result = b.Secret(SecretOTPType).Response(map[string]interface{}{
 			"key_type": role.KeyType,
 			"key":      otp,
@@ -126,10 +122,15 @@ func (b *backend) pathCredsCreateWrite(
 			"otp": otp,
 		})
 	} else if role.KeyType == KeyTypeDynamic {
+		// Generate an RSA key pair. This also installs the newly generated
+		// public key in the remote host.
 		dynamicPublicKey, dynamicPrivateKey, err := b.GenerateDynamicCredential(req, role, username, ip)
 		if err != nil {
 			return nil, err
 		}
+
+		// Return the information relevant to user of dynamic type and save
+		// information required for later use in internal section of secret.
 		result = b.Secret(SecretDynamicKeyType).Response(map[string]interface{}{
 			"key":      dynamicPrivateKey,
 			"key_type": role.KeyType,
@@ -152,11 +153,13 @@ func (b *backend) pathCredsCreateWrite(
 	// Change the lease information to reflect user's choice
 	lease, _ := b.Lease(req.Storage)
 
+	// If the lease information is set, update it in secret.
 	if lease != nil {
 		result.Secret.Lease = lease.Lease
 		result.Secret.LeaseGracePeriod = lease.LeaseMax
 	}
 
+	// If lease information is not set, set it to 10 minutes.
 	if lease == nil {
 		result.Secret.Lease = 10 * time.Minute
 		result.Secret.LeaseGracePeriod = 2 * time.Minute
@@ -170,11 +173,11 @@ func (b *backend) GenerateDynamicCredential(req *logical.Request, role *sshRole,
 	// Fetch the host key to be used for dynamic key installation
 	keyEntry, err := req.Storage.Get(fmt.Sprintf("keys/%s", role.KeyName))
 	if err != nil {
-		return "", "", fmt.Errorf("key '%s' not found error:%s", role.KeyName, err)
+		return "", "", fmt.Errorf("key '%s' not found. err:%s", role.KeyName, err)
 	}
 
 	if keyEntry == nil {
-		return "", "", fmt.Errorf("key '%s' not found", role.KeyName, err)
+		return "", "", fmt.Errorf("key '%s' not found", role.KeyName)
 	}
 
 	var hostKey sshHostKey
@@ -182,26 +185,14 @@ func (b *backend) GenerateDynamicCredential(req *logical.Request, role *sshRole,
 		return "", "", fmt.Errorf("error reading the host key: %s", err)
 	}
 
+	// Generate a new RSA key pair with the given key length.
 	dynamicPublicKey, dynamicPrivateKey, err := generateRSAKeys(role.KeyBits)
 	if err != nil {
 		return "", "", fmt.Errorf("error generating key: %s", err)
 	}
 
-	// Transfer the public key to target machine
-	_, publicKeyFileName := b.GenerateSaltedOTP()
-	err = scpUpload(role.AdminUser, ip, role.Port, hostKey.Key, publicKeyFileName, dynamicPublicKey)
-	if err != nil {
-		return "", "", fmt.Errorf("error uploading public key: %s", err)
-	}
-
-	scriptFileName := fmt.Sprintf("%s.sh", publicKeyFileName)
-	err = scpUpload(role.AdminUser, ip, role.Port, hostKey.Key, scriptFileName, role.InstallScript)
-	if err != nil {
-		return "", "", fmt.Errorf("error uploading install script: %s", err)
-	}
-
 	// Add the public key to authorized_keys file in target machine
-	err = installPublicKeyInTarget(role.AdminUser, publicKeyFileName, username, ip, role.Port, hostKey.Key, true)
+	err = b.installPublicKeyInTarget(role.AdminUser, username, ip, role.Port, hostKey.Key, dynamicPublicKey, role.InstallScript, true)
 	if err != nil {
 		return "", "", fmt.Errorf("error adding public key to authorized_keys file in target")
 	}
@@ -217,8 +208,14 @@ func (b *backend) GenerateSaltedOTP() (string, string) {
 // Generates an UUID OTP and creates an entry for the same in storage backend with its salted string.
 func (b *backend) GenerateOTPCredential(req *logical.Request, username, ip string) (string, error) {
 	otp, otpSalted := b.GenerateSaltedOTP()
+
+	// Check if there is an entry already created for the newly generated OTP.
 	entry, err := b.getOTP(req.Storage, otpSalted)
-	// Make sure that new OTP is not replacing an existing one
+
+	// If entry already exists for the OTP, make sure that new OTP is not
+	// replacing an existing one by recreating new ones until an unused
+	// OTP is generated. It is very unlikely that this is the case and this
+	// code is just for safety.
 	for err == nil && entry != nil {
 		otp, otpSalted = b.GenerateSaltedOTP()
 		entry, err = b.getOTP(req.Storage, otpSalted)
@@ -226,6 +223,8 @@ func (b *backend) GenerateOTPCredential(req *logical.Request, username, ip strin
 			return "", err
 		}
 	}
+
+	// Store an entry for the salt of OTP.
 	newEntry, err := logical.StorageEntryJSON("otp/"+otpSalted, sshOTP{
 		Username: username,
 		IP:       ip,
@@ -239,6 +238,18 @@ func (b *backend) GenerateOTPCredential(req *logical.Request, username, ip strin
 	return otp, nil
 }
 
+// Checks if the username supplied by the user is present in the list of
+// allowed users registered which creation of role.
+func validateUsername(username, allowedUsers string) error {
+	userList := strings.Split(allowedUsers, ",")
+	for _, user := range userList {
+		if user == username {
+			return nil
+		}
+	}
+	return fmt.Errorf("username not in allowed users list")
+}
+
 const pathCredsCreateHelpSyn = `
 Creates a credential for establishing SSH connection with the remote host.
 `

builtin/logical/ssh/path_keys.go

@@ -86,6 +86,7 @@ func (b *backend) pathKeysWrite(req *logical.Request, d *framework.FieldData) (*
 
 	keyString := d.Get("key").(string)
 
+	// Check if the key provided is infact a private key
 	signer, err := ssh.ParsePrivateKey([]byte(keyString))
 	if err != nil || signer == nil {
 		return logical.ErrorResponse("Invalid key"), nil
@@ -97,6 +98,7 @@ func (b *backend) pathKeysWrite(req *logical.Request, d *framework.FieldData) (*
 
 	keyPath := fmt.Sprintf("keys/%s", keyName)
 
+	// Store the key
 	entry, err := logical.StorageEntryJSON(keyPath, map[string]interface{}{
 		"key": keyString,
 	})
@@ -110,18 +112,15 @@ func (b *backend) pathKeysWrite(req *logical.Request, d *framework.FieldData) (*
 }
 
 const pathKeysSyn = `
-Register a shared key which can be used to install dynamic key
+Register a shared private key with Vault.
-in remote machine.
 `
 
 const pathKeysDesc = `
-The shared key registered will be used to install and uninstall
+Vault uses this key to install and uninstall dynamic keys in remote hosts. This
-long lived dynamic keys in remote machine. This key should have
+key should have sudoer privileges in remote hosts. This enables installing keys
-"root" privileges at target machine. This enables installing keys
 for unprivileged usernames.
 
-If this backend is mounted as "ssh", then the endpoint for registering
+If this backend is mounted as "ssh", then the endpoint for registering shared key
-shared key is "ssh/keys/webrack", if "webrack" is the user coined 
+is "ssh/keys/webrack", if "webrack" is the user coined name for the key. The name
-name for the key. The name given here can be associated with any
+given here can be associated with any number of roles via the endpoint "ssh/roles/".
-number of roles via the endpoint "ssh/roles/".
 `

builtin/logical/ssh/path_lookup.go

@@ -35,11 +35,14 @@ func (b *backend) pathLookupWrite(req *logical.Request, d *framework.FieldData)
 		return logical.ErrorResponse(fmt.Sprintf("Invalid IP '%s'", ip.String())), nil
 	}
 
+	// Get all the roles created in the backend.
 	keys, err := req.Storage.List("roles/")
 	if err != nil {
 		return nil, err
 	}
 
+	// Look for roles which has CIDR blocks that encompasses the given IP
+	// and create a list out of it.
 	var matchingRoles []string
 	for _, role := range keys {
 		if contains, _ := roleContainsIP(req.Storage, role, ip.String()); contains {
@@ -47,13 +50,14 @@ func (b *backend) pathLookupWrite(req *logical.Request, d *framework.FieldData)
 		}
 	}
 
-	// This result may potentially reveal more information than it is supposed to.
+	// This list may potentially reveal more information than it is supposed to.
 	// The roles for which the client is not authorized to will also be displayed.
 	// However, if the client tries to use the role for which the client is not
-	// authenticated, it will fail. There are no problems there. In a way this can
+	// authenticated, it will fail. It is not a problem. In a way this can be
-	// be viewed as a feature. The client can ask for permissions to be given for
+	// viewed as a feature. The client can ask for permissions to be given for
 	// a specific role if things are not working!
-	// Going forward, the role names should be filtered and only the roles which
+	//
+	// Ideally, the role names should be filtered and only the roles which
 	// the client is authorized to see, should be returned.
 	return &logical.Response{
 		Data: map[string]interface{}{
@@ -63,16 +67,15 @@ func (b *backend) pathLookupWrite(req *logical.Request, d *framework.FieldData)
 }
 
 const pathLookupSyn = `
-Lists 'roles' that can be used to create a dynamic key.
+List all the roles associated with the given IP address.
 `
 
 const pathLookupDesc = `
-The IP address for which the key is requested, is searched in the
+The IP address for which the key is requested, is searched in the CIDR blocks
-CIDR blocks registered with vault using the 'roles' endpoint. Keys
+registered with vault using the 'roles' endpoint. Keys can be generated only by
-can be generated only by specifying the 'role' name. The roles that
+specifying the 'role' name. The roles that can be used to generate the key for
-can be used to generate the key for a particular IP, are listed via
+a particular IP, are listed via this endpoint. For example, if this backend is
-this endpoint. For example, if this backend is mounted at "ssh", then
+mounted at "ssh", then "ssh/lookup" lists the roles associated with keys can be
-"ssh/lookup" lists the roles associated with keys can be generated
+generated for a target IP, if the CIDR block encompassing the IP is registered
-for a target IP, if the CIDR block encompassing the IP, is registered
 with vault.
 `

builtin/logical/ssh/path_roles.go

@@ -2,7 +2,6 @@ package ssh
 
 import (
 	"fmt"
-	"net"
 	"strings"
 
 	"github.com/hashicorp/vault/logical"
@@ -14,6 +13,9 @@ const (
 	KeyTypeDynamic = "dynamic"
 )
 
+// Structure that represents a role in SSH backend. This is a common role structure
+// for both OTP and Dynamic roles. Not all the fields are mandatory for both type.
+// Some are applicable for one and not for other. It doesn't matter.
 type sshRole struct {
 	KeyType       string `mapstructure:"key_type" json:"key_type"`
 	KeyName       string `mapstructure:"key" json:"key"`
@@ -140,11 +142,11 @@ func (b *backend) pathRoleWrite(req *logical.Request, d *framework.FieldData) (*
 	if cidrList == "" {
 		return logical.ErrorResponse("Missing CIDR blocks"), nil
 	}
-	for _, item := range strings.Split(cidrList, ",") {
+
-		_, _, err := net.ParseCIDR(item)
+	// Check if all the CIDR entries are infact valid entries
-		if err != nil {
+	err := validateCIDRList(cidrList)
-			return logical.ErrorResponse(fmt.Sprintf("Invalid CIDR list entry '%s'", item)), nil
+	if err != nil {
-		}
+		return logical.ErrorResponse(fmt.Sprintf("Invalid cidr_list entry. %s", err)), nil
 	}
 
 	port := d.Get("port").(int)
@@ -158,14 +160,16 @@ func (b *backend) pathRoleWrite(req *logical.Request, d *framework.FieldData) (*
 	}
 	keyType = strings.ToLower(keyType)
 
-	var err error
 	var roleEntry sshRole
 	if keyType == KeyTypeOTP {
+		// Admin user is not used if OTP key type is used because there is
+		// no need to login to remote machine.
 		adminUser := d.Get("admin_user").(string)
 		if adminUser != "" {
 			return logical.ErrorResponse("Admin user not required for OTP type"), nil
 		}
 
+		// Below are the only fields used from the role structure for OTP type.
 		roleEntry = sshRole{
 			DefaultUser:  defaultUser,
 			CIDRList:     cidrList,
@@ -174,6 +178,7 @@ func (b *backend) pathRoleWrite(req *logical.Request, d *framework.FieldData) (*
 			AllowedUsers: allowedUsers,
 		}
 	} else if keyType == KeyTypeDynamic {
+		// Key name is required by dynamic type and not by OTP type.
 		keyName := d.Get("key").(string)
 		if keyName == "" {
 			return logical.ErrorResponse("Missing key name"), nil
@@ -184,10 +189,11 @@ func (b *backend) pathRoleWrite(req *logical.Request, d *framework.FieldData) (*
 		}
 
 		installScript := d.Get("install_script").(string)
+
+		// Setting the default script here. The script will install the
+		// generated public key in the authorized_keys file of linux host.
 		if installScript == "" {
-			// Setting the default script here. The script will install the generated public key in
+			installScript = DefaultPublicKeyInstallScript
-			// the authorized_keys file of linux host.
-			installScript = LinuxInstallScript
 		}
 
 		adminUser := d.Get("admin_user").(string)
@@ -195,14 +201,18 @@ func (b *backend) pathRoleWrite(req *logical.Request, d *framework.FieldData) (*
 			return logical.ErrorResponse("Missing admin username"), nil
 		}
 
+		// Key bits can only be 1024, 2048 or 4096.
 		keyBits := d.Get("key_bits").(int)
 		if keyBits != 0 && keyBits != 1024 && keyBits != 2048 && keyBits != 4096 {
 			return logical.ErrorResponse("Invalid key_bits field"), nil
 		}
+
+		// If user has not set this field, default it to 2048
 		if keyBits == 0 {
 			keyBits = 2048
 		}
 
+		// Store all the fields required by dynamic key type
 		roleEntry = sshRole{
 			KeyName:       keyName,
 			AdminUser:     adminUser,
@@ -255,24 +265,33 @@ func (b *backend) pathRoleRead(req *logical.Request, d *framework.FieldData) (*l
 		return nil, nil
 	}
 
+	// Return information should be based on the key type of the role
 	if role.KeyType == KeyTypeOTP {
 		return &logical.Response{
 			Data: map[string]interface{}{
-				"default_user": role.DefaultUser,
+				"default_user":  role.DefaultUser,
-				"cidr_list":    role.CIDRList,
+				"cidr_list":     role.CIDRList,
-				"port":         role.Port,
+				"key_type":      role.KeyType,
-				"key_type":     role.KeyType,
+				"port":          role.Port,
+				"allowed_users": role.AllowedUsers,
 			},
 		}, nil
 	} else {
 		return &logical.Response{
 			Data: map[string]interface{}{
-				"key":          role.KeyName,
+				"key":           role.KeyName,
-				"admin_user":   role.AdminUser,
+				"admin_user":    role.AdminUser,
-				"default_user": role.DefaultUser,
+				"default_user":  role.DefaultUser,
-				"cidr_list":    role.CIDRList,
+				"cidr_list":     role.CIDRList,
-				"port":         role.Port,
+				"port":          role.Port,
-				"key_type":     role.KeyType,
+				"key_type":      role.KeyType,
+				"key_bits":      role.KeyBits,
+				"allowed_users": role.AllowedUsers,
+				// Returning install script will make the output look messy.
+				// But this is one way for clients to see the script that is
+				// being used to install the key. If there is some problem,
+				// the script can be modified and configured by clients.
+				"install_script": role.InstallScript,
 			},
 		}, nil
 	}
@@ -292,16 +311,14 @@ Manage the 'roles' that can be created with this backend.
 `
 
 const pathRoleHelpDesc = `
-This path allows you to manage the roles that are used to generate 
+This path allows you to manage the roles that are used to generate credentials.
-credentials. These roles will be having privileged access to all
-the hosts mentioned by CIDR blocks. For example, if the backend
-is mounted at "ssh" and the role is created at "ssh/roles/web",
-then a user could request for a new key at "ssh/creds/web" for the
-supplied username and IP address.
 
-The 'cidr_list' field takes comma seperated CIDR blocks. The 'admin_user'
+Role takes a 'key_type' parameter that decides what type of credential this role
-should have root access in all the hosts represented by the 'cidr_list'
+can generate. If remote hosts have Vault SSH Agent installed, an 'otp' type can
-field. When the user requests key for an IP, the key will be installed
+be used, otherwise 'dynamic' type can be used.
-for the user mentioned by 'default_user' field. The 'key' field takes
+
-a named key which can be configured by 'ssh/keys/' endpoint.
+If the backend is mounted at "ssh" and the role is created at "ssh/roles/web",
+then a user could request for a credential at "ssh/creds/web" for an IP that
+belongs to the role. The credential will be for the 'default_user' registered
+with the role. There is also an optional parameter 'username' for 'creds/' endpoint.
 `

builtin/logical/ssh/path_verify.go

@@ -54,6 +54,9 @@ func (b *backend) pathVerifyWrite(req *logical.Request, d *framework.FieldData)
 		}, nil
 	}
 
+	// Create the salt of OTP because entry would have been create with the
+	// salt and not directly of the OTP. Salt will yield the same value which
+	// because the seed is the same, the backend salt.
 	otpSalted := b.salt.SaltID(otp)
 
 	// Return nil if there is no entry found for the OTP
@@ -81,14 +84,13 @@ func (b *backend) pathVerifyWrite(req *logical.Request, d *framework.FieldData)
 }
 
 const pathVerifyHelpSyn = `
-Tells if the key provided by the client is valid or not.
+Validate the OTP provided by Vault SSH Agent.
 `
 
 const pathVerifyHelpDesc = `
-This path will be used by the vault agent running in the
+This path will be used by Vault SSH Agent runnin in the remote hosts. The OTP
-target machine to check if the key provided by the client
+provided by the client is sent to Vault for validation by the agent. If Vault
-to establish the SSH connection is valid or not.
+finds an entry for the OTP, it responds with the username and IP it is associated
-
+with. Agent uses this information to authenticate the client. Vault deletes the
-This key will be a one-time-password. The vault server responds
+OTP after validating it once.
-that the key is valid and then deletes it, hence the key is OTP. 
 `

builtin/logical/ssh/secret_dynamic_key.go

@@ -112,22 +112,9 @@ func (b *backend) secretDynamicKeyRevoke(req *logical.Request, d *framework.Fiel
 		return nil, fmt.Errorf("key '%s' not found", hostKeyName)
 	}
 
-	// Transfer the dynamic public key to target machine and use it to remove the entry from authorized_keys file
-	_, dynamicPublicKeyFileName := b.GenerateSaltedOTP()
-	err = scpUpload(adminUser, ip, port, hostKey.Key, dynamicPublicKeyFileName, dynamicPublicKey)
-	if err != nil {
-		return nil, fmt.Errorf("error uploading pubic key: %s", err)
-	}
-
-	scriptFileName := fmt.Sprintf("%s.sh", dynamicPublicKeyFileName)
-	err = scpUpload(adminUser, ip, port, hostKey.Key, scriptFileName, installScript)
-	if err != nil {
-		return nil, fmt.Errorf("error uploading script file: %s", err)
-	}
-
 	// Remove the public key from authorized_keys file in target machine
 	// The last param 'false' indicates that the key should be uninstalled.
-	err = installPublicKeyInTarget(adminUser, dynamicPublicKeyFileName, username, ip, port, hostKey.Key, false)
+	err = b.installPublicKeyInTarget(adminUser, username, ip, port, hostKey.Key, dynamicPublicKey, installScript, false)
 	if err != nil {
 		return nil, fmt.Errorf("error removing public key from authorized_keys file in target")
 	}

builtin/logical/ssh/util.go

@@ -57,9 +57,8 @@ func createSSHPublicKeysSession(username, ipAddr string, port int, hostKey strin
 	return session, nil
 }
 
-// Creates a new RSA key pair with key length of 2048.
+// Creates a new RSA key pair with the given key length. The private key will be
-// The private key will be of pem format and the public key will be
+// of pem format and the public key will be of OpenSSH format.
-// of OpenSSH format.
 func generateRSAKeys(keyBits int) (publicKeyRsa string, privateKeyRsa string, err error) {
 	privateKey, err := rsa.GenerateKey(rand.Reader, keyBits)
 	if err != nil {
@@ -79,13 +78,32 @@ func generateRSAKeys(keyBits int) (publicKeyRsa string, privateKeyRsa string, er
 	return
 }
 
-// Installs or uninstalls the dynamic key in the remote host. The parameterized script
+// Public key and the script to install the key are uploaded to remote machine.
-// will install or uninstall the key. The remote host is assumed to be Linux,
+// Public key is either added or removed from authorized_keys file using the
-// and hence the path of the authorized_keys file is hard coded to resemble Linux.
+// script. Default script is for a Linux machine and hence the path of the
-// Installing and uninstalling the keys means that the public key is appended or
+// authorized_keys file is hard coded to resemble Linux.
-// removed from authorized_keys file.
+//
-// The param 'install' if false, uninstalls the key.
+// The last param 'install' if false, uninstalls the key.
-func installPublicKeyInTarget(adminUser, publicKeyFileName, username, ip string, port int, hostkey string, install bool) error {
+func (b *backend) installPublicKeyInTarget(adminUser, username, ip string, port int, hostkey, dynamicPublicKey, installScript string, install bool) error {
+	// Transfer the newly generated public key to remote host under a random
+	// file name. This is to avoid name collisions from other requests.
+	_, publicKeyFileName := b.GenerateSaltedOTP()
+	err := scpUpload(adminUser, ip, port, hostkey, publicKeyFileName, dynamicPublicKey)
+	if err != nil {
+		return fmt.Errorf("error uploading public key: %s", err)
+	}
+
+	// Transfer the script required to install or uninstall the key to the remote
+	// host under a random file name as well. This is to avoid name collisions
+	// from other requests.
+	scriptFileName := fmt.Sprintf("%s.sh", publicKeyFileName)
+	err = scpUpload(adminUser, ip, port, hostkey, scriptFileName, installScript)
+	if err != nil {
+		return fmt.Errorf("error uploading install script: %s", err)
+	}
+
+	// Create a session to run remote command that triggers the script to install
+	// or uninstall the key.
 	session, err := createSSHPublicKeysSession(adminUser, ip, port, hostkey)
 	if err != nil {
 		return fmt.Errorf("unable to create SSH Session using public keys: %s", err)
@@ -96,7 +114,6 @@ func installPublicKeyInTarget(adminUser, publicKeyFileName, username, ip string,
 	defer session.Close()
 
 	authKeysFileName := fmt.Sprintf("/home/%s/.ssh/authorized_keys", username)
-	scriptFileName := fmt.Sprintf("%s.sh", publicKeyFileName)
 
 	var installOption string
 	if install {
@@ -104,6 +121,7 @@ func installPublicKeyInTarget(adminUser, publicKeyFileName, username, ip string,
 	} else {
 		installOption = "uninstall"
 	}
+
 	// Give execute permissions to install script, run and delete it.
 	chmodCmd := fmt.Sprintf("chmod +x %s", scriptFileName)
 	scriptCmd := fmt.Sprintf("./%s %s %s %s", scriptFileName, installOption, publicKeyFileName, authKeysFileName)
@@ -145,6 +163,17 @@ func roleContainsIP(s logical.Storage, roleName string, ip string) (bool, error)
 	}
 }
 
+// Checks if the comma separated list of CIDR blocks are all valid.
+func validateCIDRList(cidrList string) error {
+	for _, item := range strings.Split(cidrList, ",") {
+		_, _, err := net.ParseCIDR(item)
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 // Returns true if the IP supplied by the user is part of the comma
 // separated CIDR blocks
 func cidrContainsIP(ip, cidrList string) (bool, error) {
@@ -160,6 +189,7 @@ func cidrContainsIP(ip, cidrList string) (bool, error) {
 	return false, nil
 }
 
+// Uploads the file to the remote machine
 func scpUpload(username, ip string, port int, hostkey, fileName, fileContent string) error {
 	signer, err := ssh.ParsePrivateKey([]byte(hostkey))
 	clientConfig := &ssh.ClientConfig{

command/ssh.go

@@ -11,6 +11,7 @@ import (
 	"strings"
 
 	"github.com/hashicorp/vault/builtin/logical/ssh"
+	"github.com/mitchellh/mapstructure"
 )
 
 // SSHCommand is a Command that establishes a SSH connection
@@ -19,8 +20,16 @@ type SSHCommand struct {
 	Meta
 }
 
+// Structure to hold the fields returned when asked for a credential from SSHh backend.
+type SSHCredentialResp struct {
+	KeyType  string `mapstructure:"key_type"`
+	Key      string `mapstructure:"key"`
+	Username string `mapstructure:"username"`
+	IP       string `mapstructure:"ip"`
+	Port     int    `mapstructure:"port"`
+}
+
 func (c *SSHCommand) Run(args []string) int {
-	var portNum int
 	var role, mountPoint, format string
 	var noExec bool
 	var sshCmdArgs []string
@@ -28,7 +37,6 @@ func (c *SSHCommand) Run(args []string) int {
 	flags := c.Meta.FlagSet("ssh", FlagSetDefault)
 	flags.StringVar(&format, "format", "table", "")
 	flags.StringVar(&role, "role", "", "")
-	flags.IntVar(&portNum, "port", 22, "")
 	flags.StringVar(&mountPoint, "mount-point", "ssh", "")
 	flags.BoolVar(&noExec, "no-exec", false, "")
 
@@ -42,8 +50,6 @@ func (c *SSHCommand) Run(args []string) int {
 		return 2
 	}
 
-	port := strconv.Itoa(portNum)
-
 	client, err := c.Client()
 	if err != nil {
 		c.Ui.Error(fmt.Sprintf("Error initializing client: %s", err))
@@ -115,17 +121,24 @@ func (c *SSHCommand) Run(args []string) int {
 		return OutputSecret(c.Ui, format, keySecret)
 	}
 
-	if keySecret.Data["key_type"].(string) == ssh.KeyTypeDynamic {
+	var resp SSHCredentialResp
-		sshDynamicKey := string(keySecret.Data["key"].(string))
+	if err := mapstructure.Decode(keySecret.Data, &resp); err != nil {
-		if len(sshDynamicKey) == 0 {
+		c.Ui.Error(fmt.Sprintf("Error parsing the credential response:%s", err))
+		return 1
+	}
+
+	port := strconv.Itoa(resp.Port)
+
+	if resp.KeyType == ssh.KeyTypeDynamic {
+		if len(resp.Key) == 0 {
 			c.Ui.Error(fmt.Sprintf("Invalid key"))
 			return 2
 		}
 		sshDynamicKeyFileName = fmt.Sprintf("vault_ssh_%s_%s", username, ip.String())
-		err = ioutil.WriteFile(sshDynamicKeyFileName, []byte(sshDynamicKey), 0600)
+		err = ioutil.WriteFile(sshDynamicKeyFileName, []byte(resp.Key), 0600)
 		sshCmdArgs = append(sshCmdArgs, []string{"-i", sshDynamicKeyFileName}...)
 
-	} else if keySecret.Data["key_type"].(string) == ssh.KeyTypeOTP {
+	} else if resp.KeyType == ssh.KeyTypeOTP {
 		// Check if the application 'sshpass' is installed in the client machine.
 		// If it is then, use it to automate typing in OTP to the prompt. Unfortunately,
 		// it was not possible to automate it without a third-party application, with
@@ -133,7 +146,7 @@ func (c *SSHCommand) Run(args []string) int {
 		// Feel free to try and remove this dependency.
 		sshpassPath, err := exec.LookPath("sshpass")
 		if err == nil {
-			sshCmdArgs = append(sshCmdArgs, []string{"-p", string(keySecret.Data["key"].(string)), "ssh", "-p", port}...)
+			sshCmdArgs = append(sshCmdArgs, []string{"-p", string(resp.Key), "ssh", "-p", port}...)
 			sshCmdArgs = append(sshCmdArgs, args...)
 			sshCmd := exec.Command(sshpassPath, sshCmdArgs...)
 			sshCmd.Stdin = os.Stdin
@@ -144,7 +157,7 @@ func (c *SSHCommand) Run(args []string) int {
 			}
 			return 0
 		}
-		c.Ui.Output("OTP for the session is " + string(keySecret.Data["key"].(string)))
+		c.Ui.Output("OTP for the session is " + resp.Key)
 		c.Ui.Output("[Note: Install 'sshpass' to automate typing in OTP]")
 	}
 	sshCmdArgs = append(sshCmdArgs, []string{"-p", port}...)
@@ -164,7 +177,7 @@ func (c *SSHCommand) Run(args []string) int {
 	}
 
 	// Delete the temporary key file generated by the command.
-	if keySecret.Data["key_type"].(string) == ssh.KeyTypeDynamic {
+	if resp.KeyType == ssh.KeyTypeDynamic {
 		// Ignoring the error from the below call since it is not a security
 		// issue if the deletion of file is not successful. User is authorized
 		// to have this secret.
@@ -256,8 +269,6 @@ SSH Options:
 			If there are no roles associated with the IP, register
 			the CIDR block of that IP using the "roles/" endpoint.
 
-  -port                 Port number to use for SSH connection. This defaults to port 22.
-
   -no-exec		Shows the credentials but does not establish connection.
 
   -mount-point		Mount point of SSH backend. If the backend is mounted at

website/source/docs/secrets/ssh/index.html.md

@@ -10,11 +10,19 @@ description: |-
 
 Name: `ssh`
 
-The SSH secret backend for Vault generates SSH credentials dynamically. This backend
+Vault SSH backend generates SSH credentials for remote hosts dynamically. This
-increases the security and solves the problem of management and distribution of keys
+backend increases the security by removing the need to share the private key to
-belonging to remote hosts. This backend provides two ways of credential creation.
+everyone who needs access to infrastructures. It also solves the problem of
-Both of them addresses the problem in different ways. Understand both of them and
+management and distribution of keys belonging to remote hosts.
-choose the one which best suits your needs.
+
+This backend supports two types of credential creation: Dynamic and OTP. Both of
+them addresses the problems in different ways.
+
+Read and carefully understand both of them and choose the one which best suits
+your needs.
+
+This page will show a quick start for this backend. For detailed documentation
+on every path, use `vault path-help` after mounting the backend.
 
 ----------------------------------------------------
 ## I. Dynamic Type
@@ -23,13 +31,12 @@ Register the shared secret key (having super user privileges) with Vault and let
 Vault take care of issuing a dynamic secret key every time a client wants to SSH
 into the remote host.
 
-When a Vault authenticated client requests for a credential, Vault server creates
+When a Vault authenticated client requests for a dynamic credential, Vault server
-a key-pair, uses the previously shared secret key to login to the remote host and
+creates a key-pair, uses the previously shared secret key to login to the remote
-appends the newly generated public key to ~/.ssh/authorized_keys file of the desired
+host and appends the newly generated public key to ~/.ssh/authorized_keys file for 
-username. Vault uses a install script (configurable) to achieve this.
+the desired username. Vault uses an install script (configurable) to achieve this.
-
+To run this script in super user mode without password prompts, `NOPASSWD` option
-To run the script without prompts, password requests for sudoers should be disabled at
+for sudoers should be enabled at all remote hosts.
-remote hosts.
 
 File: `/etc/sudoers`
 
@@ -38,9 +45,10 @@ File: `/etc/sudoers`
 ```
 
 The private key returned to the user will be leased and can be renewed if desired.
-Once the key is given to the user, Vault has no control on how and when the keys
+Once the key is given to the user, Vault will not know when the user it or how many
-will be used. Therefore, Vault **WILL NOT** and cannot audit the SSH session establishments. 
+time it gets used. Therefore, Vault **WILL NOT** and cannot audit the SSH session
-OTP type audits every SSH request (see below).
+establishments. An alternative is to use OTP type, which audits every SSH request
+(see below).
 
 ### Mounting SSH
 
@@ -74,8 +82,9 @@ Success! Data written to: ssh/roles/dynamic_key_role
 ```
 
 Use the `install_script` option to provide an install script if hosts does not
-resemble typical Linux machine. The default script is very straight forward and
+resemble typical Linux machine. The default script is compiled into the binary.
-is shown below. The script takes three arguments which are explained in the comments.
+It is straight forward and is shown below. The script takes three arguments which
+are explained in the comments.
 
 ```shell
 # This script file installs or uninstalls an RSA public key to/from authoried_keys
@@ -276,13 +285,15 @@ username@ip:~$
       <li>
         <span class="param">lease</span>
         <span class="param-flags">required</span>
-        (String) The lease value provided as a duration
+        (String)
+	The lease value provided as a duration
         with time suffix. Hour is the largest suffix.
       </li>
       <li>
         <span class="param">lease_max</span>
         <span class="param-flags">required</span>
-        (String) The maximum lease value provided as a duration
+        (String)
+	The maximum lease value provided as a duration
         with time suffix. Hour is the largest suffix.
       </li>
     </ul>
@@ -315,7 +326,8 @@ username@ip:~$
       <li>
         <span class="param">key</span>
         <span class="param-flags">required</span>
-        (String) SSH private key with super user privileges in host 
+        (String)
+	SSH private key with super user privileges in host 
       </li>
     </ul>
   </dd>
@@ -396,12 +408,14 @@ username@ip:~$
       <li>
         <span class="param">key</span>
         <span class="param-flags">required for dynamic type, NA for otp type</span>
-	Name of the registered key in Vault. Before creating the role, use the `keys/`
+	(String)
-        endpoint to create a named key.
+        Name of the registered key in Vault. Before creating the role, use
+        the `keys/` endpoint to create a named key.
       </li>
       <li>
         <span class="param">admin_user</span>
         <span class="param-flags">required for dynamic type, NA for otp type</span>
+	(String)
 	Admin user at remote host. The shared key being registered should be
 	for this user and should have root privileges. Everytime a dynamic 
 	credential is being generated for other users, Vault uses this admin
@@ -411,6 +425,7 @@ username@ip:~$
       <li>
         <span class="param">default_user</span>
         <span class="param-flags">required for both types</span>
+	(String)
 	Default username for which a credential will be generated.
 	When the endpoint 'creds/' is used without a username, this
 	value will be used as default username.
@@ -418,12 +433,14 @@ username@ip:~$
       <li>
         <span class="param">cidr_list</span>
         <span class="param-flags">required for both types</span>
+	(String)
 	Comma separated list of CIDR blocks for which the role is applicable for.
 	CIDR blocks can belong to more than one role.
       </li>
       <li>
         <span class="param">port</span>
         <span class="param-flags">optional for both types</span>
+	(Integer)
 	Port number for SSH connection. Default is '22'. Port number does not
 	play any role in creation of OTP. For 'otp' type, this is just a way
 	to inform client about the port number to use. Port number will be
@@ -432,23 +449,27 @@ username@ip:~$
       <li>
         <span class="param">key_type</span>
         <span class="param-flags">required for both types</span>
+	(String)
 	Type of key used to login to hosts. It can be either `otp` or `dynamic`.
 	`otp` type requires agent to be installed in remote hosts.
       </li>
       <li>
         <span class="param">key_bits</span>
         <span class="param-flags">optional for dynamic type, NA for otp type</span>
+	(Integer)
 	Length of the RSA dynamic key in bits. It can be one of 1024, 2048 or 4096.
       </li>
       <li>
         <span class="param">install_script</span>
         <span class="param-flags">optional for dynamic type, NA for otp type</span>
+	(String)
 	Script used to install and uninstall public keys in the target machine.
 	The inbuilt default install script will be for Linux hosts.
       </li>
       <li>
         <span class="param">allowed_users</span>
         <span class="param-flags">optional for both types</span>
+	(String)
 	If this option is not specified, client can request for a credential for
 	any valid user at the remote host, including the admin user. If only certain
 	usernames are to be allowed, then this list enforces it. If this field is
@@ -550,11 +571,13 @@ username@ip:~$
       <li>
         <span class="param">username</span>
         <span class="param-flags">optional</span>
+	(String)
         Username in remote host.
       </li>
       <li>
         <span class="param">ip</span>
         <span class="param-flags">required</span>
+	(String)
         IP of the remote host.
       </li>
     </ul>
@@ -586,6 +609,7 @@ username@ip:~$
       <li>
         <span class="param">ip</span>
         <span class="param-flags">required</span>
+	(String)
         IP of the remote host.
       </li>
     </ul>
@@ -617,6 +641,7 @@ username@ip:~$
       <li>
         <span class="param">otp</span>
         <span class="param-flags">required</span>
+	(String)
         One-Time-Key that needs to be validated.
       </li>
     </ul>