Vault SSH: review rework: formatted and moved code

builtin/logical/ssh/backend.go

@@ -40,12 +40,12 @@ type backend struct {
 }
 
 const backendHelp = `
-The SSH backend dynamically generates SSH private keys for remote hosts.
-The generated key has a configurable lease set and are automatically 
-revoked at the end of the lease.
+The SSH backend dynamically generates SSH private keys for 
+remote hosts.The generated key has a configurable lease set
+and are automatically revoked at the end of the lease.
 
-After mounting this backend, configure the lease using the 'config/lease'
-endpoint. The shared SSH key belonging to any infrastructure should be 
-registered with the 'roles/' endpoint before dynamic keys for remote hosts 
-can be generated.
+After mounting this backend, configure the lease using the
+'config/lease' endpoint. The shared SSH key belonging to any
+infrastructure should be registered with the 'roles/' endpoint
+before dynamic keys for remote hosts can be generated.
 `

builtin/logical/ssh/path_config_lease.go

@@ -35,10 +35,10 @@ func (b *backend) pathConfigLeaseWrite(req *logical.Request, d *framework.FieldD
 	leaseRaw := d.Get("lease").(string)
 	leaseMaxRaw := d.Get("lease_max").(string)
 	if leaseRaw == "" {
-		return logical.ErrorResponse("Invalid 'lease'"), nil
+		return logical.ErrorResponse("Missing lease"), nil
 	}
 	if leaseMaxRaw == "" {
-		return logical.ErrorResponse("Invalid 'lease_max'"), nil
+		return logical.ErrorResponse("Missing lease_max"), nil
 	}
 
 	lease, err := time.ParseDuration(leaseRaw)

builtin/logical/ssh/path_keys.go

@@ -63,7 +63,7 @@ func (b *backend) pathKeysWrite(req *logical.Request, d *framework.FieldData) (*
 	keyString := d.Get("key").(string)
 
 	if keyString == "" {
-		return nil, fmt.Errorf("invalid 'key'")
+		return logical.ErrorResponse("Missing key"), nil
 	}
 
 	keyPath := fmt.Sprintf("keys/%s", keyName)

builtin/logical/ssh/path_lookup.go

@@ -3,7 +3,6 @@ package ssh
 import (
 	"fmt"
 	"net"
-	"strings"
 
 	"github.com/hashicorp/vault/logical"
 	"github.com/hashicorp/vault/logical/framework"
@@ -29,7 +28,7 @@ func pathLookup(b *backend) *framework.Path {
 func (b *backend) pathLookupWrite(req *logical.Request, d *framework.FieldData) (*logical.Response, error) {
 	ipAddr := d.Get("ip").(string)
 	if ipAddr == "" {
-		return logical.ErrorResponse("Invalid 'ip'"), nil
+		return logical.ErrorResponse("Missing ip"), nil
 	}
 	ip := net.ParseIP(ipAddr)
 	if ip == nil {
@@ -61,35 +60,6 @@ func (b *backend) pathLookupWrite(req *logical.Request, d *framework.FieldData)
 	}, nil
 }
 
-func containsIP(s logical.Storage, roleName string, ip string) (bool, error) {
-	if roleName == "" || ip == "" {
-		return false, fmt.Errorf("invalid parameters")
-	}
-	roleEntry, err := s.Get(fmt.Sprintf("policy/%s", roleName))
-	if err != nil {
-		return false, fmt.Errorf("error retrieving role '%s'", err)
-	}
-	if roleEntry == nil {
-		return false, fmt.Errorf("role '%s' not found", roleName)
-	}
-	var role sshRole
-	if err := roleEntry.DecodeJSON(&role); err != nil {
-		return false, fmt.Errorf("error decoding role '%s'", roleName)
-	}
-	ipMatched := false
-	for _, item := range strings.Split(role.CIDR, ",") {
-		_, cidrIPNet, err := net.ParseCIDR(item)
-		if err != nil {
-			return false, fmt.Errorf("invalid cidr entry '%s'", item)
-		}
-		ipMatched = cidrIPNet.Contains(net.ParseIP(ip))
-		if ipMatched {
-			break
-		}
-	}
-	return ipMatched, nil
-}
-
 const pathLookupSyn = `
 Lists 'roles' that can be used to create a dynamic key.
 `

builtin/logical/ssh/path_role_create.go

@@ -41,10 +41,10 @@ func (b *backend) pathRoleCreateWrite(
 	username := d.Get("username").(string)
 	ipRaw := d.Get("ip").(string)
 	if roleName == "" {
-		return logical.ErrorResponse("Invalid 'name'"), nil
+		return logical.ErrorResponse("Missing name"), nil
 	}
 	if ipRaw == "" {
-		return logical.ErrorResponse("Invalid 'ip'"), nil
+		return logical.ErrorResponse("Missing ip"), nil
 	}
 
 	//find the role to be used for installing dynamic key

builtin/logical/ssh/path_roles.go

@@ -55,16 +55,16 @@ func (b *backend) pathRoleWrite(req *logical.Request, d *framework.FieldData) (*
 
 	//input validations
 	if roleName == "" {
-		return logical.ErrorResponse("Invalid 'roleName'"), nil
+		return logical.ErrorResponse("Missing role name"), nil
 	}
 	if keyName == "" {
-		return logical.ErrorResponse("Invalid 'key'"), nil
+		return logical.ErrorResponse("Missing key name"), nil
 	}
 	if adminUser == "" {
-		return logical.ErrorResponse("Invalid 'admin_user'"), nil
+		return logical.ErrorResponse("Missing admin username"), nil
 	}
 	if cidr == "" {
-		return logical.ErrorResponse("Invalid 'cidr'"), nil
+		return logical.ErrorResponse("Missing cidr blocks"), nil
 	}
 	for _, item := range strings.Split(cidr, ",") {
 		_, _, err := net.ParseCIDR(item)
@@ -137,9 +137,16 @@ Manage the 'roles' that can be created with this backend.
 `
 
 const pathRoleHelpDesc = `
-This path allows you to manage the roles that are used to create dynamic keys.
-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.
+This path allows you to manage the roles that are used to create
+dynamic keys. 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' field takes comma seperated CIDR blocks. The 'admin_user' 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.
+The 'cidr' field takes comma seperated CIDR blocks. The 'admin_user'
+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.
 `

builtin/logical/ssh/util.go

@@ -8,9 +8,13 @@ import (
 	"encoding/pem"
 	"fmt"
 	"io"
+	"net"
 	"os"
 	"os/exec"
 	"path/filepath"
+	"strings"
+
+	"github.com/hashicorp/vault/logical"
 
 	"golang.org/x/crypto/ssh"
 )
@@ -69,8 +73,14 @@ Creates a SSH session object which can be used to run commands in the target mac
 The session will use public key authentication method with port 22.
 */
 func createSSHPublicKeysSession(username, ipAddr, hostKey string) (*ssh.Session, error) {
-	if username == "" || ipAddr == "" || hostKey == "" {
-		return nil, fmt.Errorf("invalid parameters")
+	if username == "" {
+		return nil, fmt.Errorf("missing username")
+	}
+	if ipAddr == "" {
+		return nil, fmt.Errorf("missing ip address")
+	}
+	if hostKey == "" {
+		return nil, fmt.Errorf("missing host key")
 	}
 	signer, err := ssh.ParsePrivateKey([]byte(hostKey))
 	if err != nil {
@@ -105,7 +115,7 @@ The parameter is just the name of the file and not a path.
 */
 func removeFile(fileName string) error {
 	if fileName == "" {
-		return fmt.Errorf("invalid file name")
+		return fmt.Errorf("missing file name")
 	}
 	wd, err := os.Getwd()
 	if err != nil {
@@ -144,3 +154,35 @@ func generateRSAKeys() (publicKeyRsa string, privateKeyRsa string, err error) {
 	publicKeyRsa = "ssh-rsa " + base64.StdEncoding.EncodeToString(sshPublicKey.Marshal())
 	return
 }
+
+func containsIP(s logical.Storage, roleName string, ip string) (bool, error) {
+	if roleName == "" {
+		return false, fmt.Errorf("missing role name")
+	}
+	if ip == "" {
+		return false, fmt.Errorf("missing ip")
+	}
+	roleEntry, err := s.Get(fmt.Sprintf("policy/%s", roleName))
+	if err != nil {
+		return false, fmt.Errorf("error retrieving role '%s'", err)
+	}
+	if roleEntry == nil {
+		return false, fmt.Errorf("role '%s' not found", roleName)
+	}
+	var role sshRole
+	if err := roleEntry.DecodeJSON(&role); err != nil {
+		return false, fmt.Errorf("error decoding role '%s'", roleName)
+	}
+	ipMatched := false
+	for _, item := range strings.Split(role.CIDR, ",") {
+		_, cidrIPNet, err := net.ParseCIDR(item)
+		if err != nil {
+			return false, fmt.Errorf("invalid cidr entry '%s'", item)
+		}
+		ipMatched = cidrIPNet.Contains(net.ParseIP(ip))
+		if ipMatched {
+			break
+		}
+	}
+	return ipMatched, nil
+}