From 13ab7fc40b87ac40f10cfbe1dd659b901ea24756 Mon Sep 17 00:00:00 2001 From: Vishal Nayak Date: Wed, 1 Jul 2015 11:58:49 -0400 Subject: [PATCH] Vault SSH: PR review rework - 1 --- api/ssh.go | 16 ++++++++-------- builtin/logical/ssh/backend.go | 4 ++-- builtin/logical/ssh/path_config_lease.go | 10 +++++----- builtin/logical/ssh/path_lookup.go | 2 +- builtin/logical/ssh/path_role_create.go | 8 ++------ builtin/logical/ssh/path_roles.go | 7 ++++++- builtin/logical/ssh/secret_ssh_key.go | 10 +++++----- cli/commands.go | 2 +- command/ssh.go | 12 ++++++------ 9 files changed, 36 insertions(+), 35 deletions(-) diff --git a/api/ssh.go b/api/ssh.go index ee0544430c..ee6c959551 100644 --- a/api/ssh.go +++ b/api/ssh.go @@ -5,16 +5,16 @@ import ( "fmt" ) -type Ssh struct { +type SSH struct { c *Client } -func (c *Client) Ssh() *Ssh { - return &Ssh{c: c} +func (c *Client) SSH() *SSH { + return &SSH{c: c} } -func (c *Ssh) KeyCreate(role string, data map[string]interface{}) (*Secret, error) { - r := c.c.NewRequest("PUT", fmt.Sprintf("/v1/ssh/creds/"+role)) +func (c *SSH) KeyCreate(role string, data map[string]interface{}) (*Secret, error) { + r := c.c.NewRequest("PUT", fmt.Sprintf("/v1/ssh/creds/%s", role)) if err := r.SetJSONBody(data); err != nil { return nil, err } @@ -28,7 +28,7 @@ func (c *Ssh) KeyCreate(role string, data map[string]interface{}) (*Secret, erro return ParseSecret(resp.Body) } -func (c *Ssh) Lookup(data map[string]interface{}) (*SshRoles, error) { +func (c *SSH) Lookup(data map[string]interface{}) (*SSHRoles, error) { r := c.c.NewRequest("PUT", "/v1/ssh/lookup") if err := r.SetJSONBody(data); err != nil { return nil, err @@ -40,7 +40,7 @@ func (c *Ssh) Lookup(data map[string]interface{}) (*SshRoles, error) { } defer resp.Body.Close() - var roles SshRoles + var roles SSHRoles dec := json.NewDecoder(resp.Body) if err := dec.Decode(&roles); err != nil { return nil, err @@ -48,6 +48,6 @@ func (c *Ssh) Lookup(data map[string]interface{}) (*SshRoles, error) { return &roles, nil } -type SshRoles struct { +type SSHRoles struct { Data map[string]interface{} `json:"data"` } diff --git a/builtin/logical/ssh/backend.go b/builtin/logical/ssh/backend.go index f15650c77e..4443870658 100644 --- a/builtin/logical/ssh/backend.go +++ b/builtin/logical/ssh/backend.go @@ -29,7 +29,7 @@ func Backend() *framework.Backend { }, Secrets: []*framework.Secret{ - secretSshKey(&b), + secretSSHKey(&b), }, } return b.Backend @@ -41,7 +41,7 @@ type backend struct { const backendHelp = ` The SSH backend dynamically generates SSH private keys for remote hosts. -The key generated has a configurable lease set and are automatically +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' diff --git a/builtin/logical/ssh/path_config_lease.go b/builtin/logical/ssh/path_config_lease.go index a50b85aa3f..3ccaea68e2 100644 --- a/builtin/logical/ssh/path_config_lease.go +++ b/builtin/logical/ssh/path_config_lease.go @@ -44,12 +44,12 @@ func (b *backend) pathConfigLeaseWrite(req *logical.Request, d *framework.FieldD lease, err := time.ParseDuration(leaseRaw) if err != nil { return logical.ErrorResponse(fmt.Sprintf( - "Invalid lease: %s", err)), nil + "Invalid 'lease': %s", err)), nil } leaseMax, err := time.ParseDuration(leaseMaxRaw) if err != nil { return logical.ErrorResponse(fmt.Sprintf( - "Invalid lease: %s", err)), nil + "Invalid 'lease_max': %s", err)), nil } entry, err := logical.StorageEntryJSON("config/lease", &configLease{ @@ -57,10 +57,10 @@ func (b *backend) pathConfigLeaseWrite(req *logical.Request, d *framework.FieldD LeaseMax: leaseMax, }) if err != nil { - return nil, err + return nil, fmt.Errorf("Could not create storage entry JSON: %s", err) } if err := req.Storage.Put(entry); err != nil { - return nil, err + return nil, fmt.Errorf("Could not store JSON: %s", err) } return nil, nil @@ -89,7 +89,7 @@ type configLease struct { } const pathConfigLeaseHelpSyn = ` -Configure the default lease information for SSH one time keys. +Configure the default lease information for SSH dynamic keys. ` const pathConfigLeaseHelpDesc = ` diff --git a/builtin/logical/ssh/path_lookup.go b/builtin/logical/ssh/path_lookup.go index 20084a8091..d56f1e7467 100644 --- a/builtin/logical/ssh/path_lookup.go +++ b/builtin/logical/ssh/path_lookup.go @@ -80,7 +80,7 @@ func containsIP(s logical.Storage, roleName string, ip string) (bool, error) { for _, item := range strings.Split(role.CIDR, ",") { _, cidrIPNet, err := net.ParseCIDR(item) if err != nil { - return false, fmt.Errorf(fmt.Sprintf("Invalid cidr entry '%s'", item)) + return false, fmt.Errorf("Invalid cidr entry '%s'", item) } ipMatched = cidrIPNet.Contains(net.ParseIP(ip)) if ipMatched { diff --git a/builtin/logical/ssh/path_role_create.go b/builtin/logical/ssh/path_role_create.go index 8ab9d46ad6..0081857b7b 100644 --- a/builtin/logical/ssh/path_role_create.go +++ b/builtin/logical/ssh/path_role_create.go @@ -1,7 +1,6 @@ package ssh import ( - "bytes" "fmt" "io/ioutil" "net" @@ -109,11 +108,11 @@ func (b *backend) pathRoleCreateWrite( //delete the temporary files if they are already present err = removeFile(dynamicPrivateKeyFileName) if err != nil { - return nil, fmt.Errorf(fmt.Sprintf("Error removing dynamic private key file: '%s'", err)) + return nil, fmt.Errorf("Error removing dynamic private key file: '%s'", err) } err = removeFile(dynamicPublicKeyFileName) if err != nil { - return nil, fmt.Errorf(fmt.Sprintf("Error removing dynamic private key file: '%s'", err)) + return nil, fmt.Errorf("Error removing dynamic private key file: '%s'", err) } //generate RSA key pair @@ -136,8 +135,6 @@ func (b *backend) pathRoleCreateWrite( if session == nil { return nil, fmt.Errorf("Invalid session object") } - var buf bytes.Buffer - session.Stdout = &buf authKeysFileName := "/home/" + username + "/.ssh/authorized_keys" tempKeysFileName := "/home/" + username + "/temp_authorized_keys" @@ -159,7 +156,6 @@ func (b *backend) pathRoleCreateWrite( return nil, err } session.Close() - fmt.Println(buf.String()) result := b.Secret(SecretOneTimeKeyType).Response(map[string]interface{}{ "key": dynamicPrivateKey, diff --git a/builtin/logical/ssh/path_roles.go b/builtin/logical/ssh/path_roles.go index e34c0d1459..89ea2f3d79 100644 --- a/builtin/logical/ssh/path_roles.go +++ b/builtin/logical/ssh/path_roles.go @@ -73,12 +73,17 @@ func (b *backend) pathRoleWrite(req *logical.Request, d *framework.FieldData) (* } } - rolePath := "policy/" + roleName + keyPath := "keys/" + keyName + keyEntry, err := req.Storage.Get(keyPath) + if err != nil || keyEntry == nil { + return logical.ErrorResponse(fmt.Sprintf("Invalid 'key': '%s'", keyName)), nil + } if defaultUser == "" { defaultUser = adminUser } + rolePath := "policy/" + roleName entry, err := logical.StorageEntryJSON(rolePath, sshRole{ KeyName: keyName, AdminUser: adminUser, diff --git a/builtin/logical/ssh/secret_ssh_key.go b/builtin/logical/ssh/secret_ssh_key.go index 2033dba077..6b588f56e1 100644 --- a/builtin/logical/ssh/secret_ssh_key.go +++ b/builtin/logical/ssh/secret_ssh_key.go @@ -12,7 +12,7 @@ import ( const SecretOneTimeKeyType = "secret_one_type_key_type" -func secretSshKey(b *backend) *framework.Secret { +func secretSSHKey(b *backend) *framework.Secret { return &framework.Secret{ Type: SecretOneTimeKeyType, Fields: map[string]*framework.FieldSchema{ @@ -27,12 +27,12 @@ func secretSshKey(b *backend) *framework.Secret { }, DefaultDuration: 1 * time.Hour, DefaultGracePeriod: 10 * time.Minute, - Renew: b.secretSshKeyRenew, - Revoke: b.secretSshKeyRevoke, + Renew: b.secretSSHKeyRenew, + Revoke: b.secretSSHKeyRevoke, } } -func (b *backend) secretSshKeyRenew(req *logical.Request, d *framework.FieldData) (*logical.Response, error) { +func (b *backend) secretSSHKeyRenew(req *logical.Request, d *framework.FieldData) (*logical.Response, error) { lease, err := b.Lease(req.Storage) if err != nil { return nil, err @@ -44,7 +44,7 @@ func (b *backend) secretSshKeyRenew(req *logical.Request, d *framework.FieldData return f(req, d) } -func (b *backend) secretSshKeyRevoke(req *logical.Request, d *framework.FieldData) (*logical.Response, error) { +func (b *backend) secretSSHKeyRevoke(req *logical.Request, d *framework.FieldData) (*logical.Response, error) { usernameRaw, ok := req.Secret.InternalData["username"] if !ok { return nil, fmt.Errorf("secret is missing internal data") diff --git a/cli/commands.go b/cli/commands.go index 66d6567349..94aa937870 100644 --- a/cli/commands.go +++ b/cli/commands.go @@ -81,7 +81,7 @@ func Commands(metaPtr *command.Meta) map[string]cli.CommandFactory { }, "ssh": func() (cli.Command, error) { - return &command.SshCommand{ + return &command.SSHCommand{ Meta: meta, }, nil }, diff --git a/command/ssh.go b/command/ssh.go index b2cf7cc24a..b06258ec3a 100644 --- a/command/ssh.go +++ b/command/ssh.go @@ -11,11 +11,11 @@ import ( "syscall" ) -type SshCommand struct { +type SSHCommand struct { Meta } -func (c *SshCommand) Run(args []string) int { +func (c *SSHCommand) Run(args []string) int { var role string flags := c.Meta.FlagSet("ssh", FlagSetDefault) flags.StringVar(&role, "role", "", "") @@ -70,7 +70,7 @@ func (c *SshCommand) Run(args []string) int { "username": username, "ip": ip.String(), } - keySecret, err := client.Ssh().KeyCreate(role, data) + keySecret, err := client.SSH().KeyCreate(role, data) if err != nil { c.Ui.Error(fmt.Sprintf("Error getting key for SSH session:%s", err)) return 2 @@ -103,13 +103,13 @@ type OneTimeKey struct { Key string } -func (c *SshCommand) Synopsis() string { +func (c *SSHCommand) Synopsis() string { return "Initiate a SSH session" } -func (c *SshCommand) Help() string { +func (c *SSHCommand) Help() string { helpText := ` - SshCommand Help String + SSHCommand Help String ` return strings.TrimSpace(helpText) }