Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show HCL parsing errors and typos #1200

Merged
merged 12 commits into from Mar 11, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion Godeps/Godeps.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

92 changes: 71 additions & 21 deletions api/ssh_agent.go
Expand Up @@ -8,18 +8,22 @@ import (
"os"

"github.com/hashicorp/go-cleanhttp"
"github.com/hashicorp/go-multierror"
"github.com/hashicorp/hcl"
"github.com/hashicorp/hcl/hcl/ast"
"github.com/mitchellh/mapstructure"
)

const (
// Default path at which SSH backend will be mounted in Vault server
// SSHHelperDefaultMountPoint is the default path at which SSH backend will be
// mounted in the Vault server.
SSHHelperDefaultMountPoint = "ssh"

// Echo request message sent as OTP by the vault-ssh-helper
// VerifyEchoRequest is the echo request message sent as OTP by the helper.
VerifyEchoRequest = "verify-echo-request"

// Echo response message sent as a response to OTP matching echo request
// VerifyEchoResponse is the echo response message sent as a response to OTP
// matching echo request.
VerifyEchoResponse = "verify-echo-response"
)

Expand Down Expand Up @@ -55,8 +59,7 @@ type SSHHelperConfig struct {
TLSSkipVerify bool `hcl:"tls_skip_verify"`
}

// TLSClient returns a HTTP client that uses TLS verification (TLS 1.2) for a given
// certificate pool.
// SetTLSParameters sets the TLS parameters for this SSH agent.
func (c *SSHHelperConfig) SetTLSParameters(clientConfig *Config, certPool *x509.CertPool) {
tlsConfig := &tls.Config{
InsecureSkipVerify: c.TLSSkipVerify,
Expand Down Expand Up @@ -112,29 +115,48 @@ func (c *SSHHelperConfig) NewClient() (*Client, error) {
// Vault address is a required parameter.
// Mount point defaults to "ssh".
func LoadSSHHelperConfig(path string) (*SSHHelperConfig, error) {
var config SSHHelperConfig
contents, err := ioutil.ReadFile(path)
if !os.IsNotExist(err) {
obj, err := hcl.Parse(string(contents))
if err != nil {
return nil, err
}
if err != nil && !os.IsNotExist(err) {
return nil, multierror.Prefix(err, "ssh_helper:")
}
return ParseSSHHelperConfig(string(contents))
}

if err := hcl.DecodeObject(&config, obj); err != nil {
return nil, err
}
} else {
return nil, err
// ParseSSHHelperConfig parses the given contents as a string for the SSHHelper
// configuration.
func ParseSSHHelperConfig(contents string) (*SSHHelperConfig, error) {
root, err := hcl.Parse(string(contents))
if err != nil {
return nil, fmt.Errorf("ssh_helper: error parsing config: %s", err)
}

if config.VaultAddr == "" {
return nil, fmt.Errorf("config missing vault_addr")
list, ok := root.Node.(*ast.ObjectList)
if !ok {
return nil, fmt.Errorf("ssh_helper: error parsing config: file doesn't contain a root object")
}
if config.SSHMountPoint == "" {
config.SSHMountPoint = SSHHelperDefaultMountPoint

valid := []string{
"vault_addr",
"ssh_mount_point",
"ca_cert",
"ca_path",
"allowed_cidr_list",
"tls_skip_verify",
}
if err := checkHCLKeys(list, valid); err != nil {
return nil, multierror.Prefix(err, "ssh_helper:")
}

return &config, nil
var c SSHHelperConfig
c.SSHMountPoint = SSHHelperDefaultMountPoint
if err := hcl.DecodeObject(&c, list); err != nil {
return nil, multierror.Prefix(err, "ssh_helper:")
}

if c.VaultAddr == "" {
return nil, fmt.Errorf("ssh_helper: missing config 'vault_addr'")
}
return &c, nil
}

// SSHHelper creates an SSHHelper object which can talk to Vault server with SSH backend
Expand Down Expand Up @@ -189,3 +211,31 @@ func (c *SSHHelper) Verify(otp string) (*SSHVerifyResponse, error) {
}
return &verifyResp, nil
}

func checkHCLKeys(node ast.Node, valid []string) error {
var list *ast.ObjectList
switch n := node.(type) {
case *ast.ObjectList:
list = n
case *ast.ObjectType:
list = n.List
default:
return fmt.Errorf("cannot check HCL keys of type %T", n)
}

validMap := make(map[string]struct{}, len(valid))
for _, v := range valid {
validMap[v] = struct{}{}
}

var result error
for _, item := range list.Items {
key := item.Keys[0].Token.Value().(string)
if _, ok := validMap[key]; !ok {
result = multierror.Append(result, fmt.Errorf(
"invalid key '%s' on line %d", key, item.Assign.Line))
}
}

return result
}
39 changes: 39 additions & 0 deletions api/ssh_agent_test.go
Expand Up @@ -2,6 +2,7 @@ package api

import (
"fmt"
"strings"
"testing"
)

Expand All @@ -28,3 +29,41 @@ func TestSSH_CreateTLSClient(t *testing.T) {
panic(fmt.Sprintf("error creating client with TLS transport"))
}
}

func TestParseSSHHelperConfig(t *testing.T) {
config, err := ParseSSHHelperConfig(`
vault_addr = "1.2.3.4"
`)
if err != nil {
t.Fatal(err)
}

if config.SSHMountPoint != SSHHelperDefaultMountPoint {
t.Errorf("expected %q to be %q", config.SSHMountPoint, SSHHelperDefaultMountPoint)
}
}

func TestParseSSHHelperConfig_missingVaultAddr(t *testing.T) {
_, err := ParseSSHHelperConfig("")
if err == nil {
t.Fatal("expected error")
}

if !strings.Contains(err.Error(), "ssh_helper: missing config 'vault_addr'") {
t.Errorf("bad error: %s", err)
}
}

func TestParseSSHHelperConfig_badKeys(t *testing.T) {
_, err := ParseSSHHelperConfig(`
vault_addr = "1.2.3.4"
nope = "bad"
`)
if err == nil {
t.Fatal("expected error")
}

if !strings.Contains(err.Error(), "ssh_helper: invalid key 'nope' on line 3") {
t.Errorf("bad error: %s", err)
}
}
69 changes: 57 additions & 12 deletions command/config.go
Expand Up @@ -5,7 +5,9 @@ import (
"io/ioutil"
"os"

"github.com/hashicorp/go-multierror"
"github.com/hashicorp/hcl"
"github.com/hashicorp/hcl/hcl/ast"
"github.com/mitchellh/go-homedir"
)

Expand Down Expand Up @@ -44,22 +46,65 @@ func LoadConfig(path string) (*Config, error) {
return nil, fmt.Errorf("Error expanding config path: %s", err)
}

var config Config
contents, err := ioutil.ReadFile(path)
if !os.IsNotExist(err) {
if err != nil {
return nil, err
}
if err != nil && !os.IsNotExist(err) {
return nil, err
}

obj, err := hcl.Parse(string(contents))
if err != nil {
return nil, err
}
return ParseConfig(string(contents))
}

// ParseConfig parses the given configuration as a string.
func ParseConfig(contents string) (*Config, error) {
root, err := hcl.Parse(contents)
if err != nil {
return nil, err
}

// Top-level item should be the object list
list, ok := root.Node.(*ast.ObjectList)
if !ok {
return nil, fmt.Errorf("Failed to parse config: does not contain a root object")
}

valid := []string{
"token_helper",
}
if err := checkHCLKeys(list, valid); err != nil {
return nil, err
}

var c Config
if err := hcl.DecodeObject(&c, list); err != nil {
return nil, err
}
return &c, nil
}

func checkHCLKeys(node ast.Node, valid []string) error {
var list *ast.ObjectList
switch n := node.(type) {
case *ast.ObjectList:
list = n
case *ast.ObjectType:
list = n.List
default:
return fmt.Errorf("cannot check HCL keys of type %T", n)
}

validMap := make(map[string]struct{}, len(valid))
for _, v := range valid {
validMap[v] = struct{}{}
}

if err := hcl.DecodeObject(&config, obj); err != nil {
return nil, err
var result error
for _, item := range list.Items {
key := item.Keys[0].Token.Value().(string)
if _, ok := validMap[key]; !ok {
result = multierror.Append(result, fmt.Errorf(
"invalid key '%s' on line %d", key, item.Assign.Line))
}
}

return &config, nil
return result
}
26 changes: 26 additions & 0 deletions command/config_test.go
Expand Up @@ -3,6 +3,7 @@ package command
import (
"path/filepath"
"reflect"
"strings"
"testing"
)

Expand All @@ -19,3 +20,28 @@ func TestLoadConfig(t *testing.T) {
t.Fatalf("bad: %#v", config)
}
}

func TestLoadConfig_noExist(t *testing.T) {
config, err := LoadConfig("nope/not-once/.never")
if err != nil {
t.Fatal(err)
}

if config.TokenHelper != "" {
t.Errorf("expected %q to be %q", config.TokenHelper, "")
}
}

func TestParseConfig_badKeys(t *testing.T) {
_, err := ParseConfig(`
token_helper = "/token"
nope = "true"
`)
if err == nil {
t.Fatal("expected error")
}

if !strings.Contains(err.Error(), "invalid key 'nope' on line 3") {
t.Errorf("bad error: %s", err.Error())
}
}
2 changes: 1 addition & 1 deletion command/policy_write_test.go
Expand Up @@ -24,7 +24,7 @@ func TestPolicyWrite(t *testing.T) {
args := []string{
"-address", addr,
"foo",
"./test-fixtures/config.hcl",
"./test-fixtures/policy.hcl",
}
if code := c.Run(args); code != 0 {
t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String())
Expand Down