Repository: google/glome (language: python)

Issue:
Add Go GLOME config file parsing
We have an internal user of GLOME that's written their own config parser. Let's provide an in-tree config parser in Go, so we can include it in test suites publically and keep it updated in lockstep with the C one.


Discussion / hints:

Fix patch:
diff --git a/go/config/config.go b/go/config/config.go
new file mode 100644
index 0000000..de2f6a2
--- /dev/null
+++ b/go/config/config.go
@@ -0,0 +1,347 @@
+// Copyright 2023 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package config
+
+import (
+	"bufio"
+	"encoding/base64"
+	"encoding/hex"
+	"fmt"
+	"io"
+	"strconv"
+	"strings"
+	"unicode"
+
+	"github.com/google/glome/go/glome"
+)
+
+// Config represents the supported GLOME login settings.
+type Config struct {
+	AuthDelay      int
+	InputTimeout   int
+	ConfigPath     string
+	EphemeralKey   glome.PrivateKey
+	MinAuthcodeLen int
+	HostID         string
+	HostIDType     string
+	LoginPath      string
+	DisableSyslog  bool
+	PrintSecrets   bool
+	Timeout        int
+	Verbose        bool
+
+	ServiceConfig ServiceConfig
+}
+
+// ServiceConfig contains GLOME settings from the [service] configuration section.
+type ServiceConfig struct {
+	PublicKey  glome.PublicKey
+	KeyVersion int
+	Prompt     string
+}
+
+// ParseErrorType represents different classes of things that can happen during parsing a GLOME configuration.
+type ParseErrorType string
+
+const (
+	// BadSectionName indicates that a line could not be parsed as a configuration section header.
+	BadSectionName ParseErrorType = "bad section header line"
+
+	// BadKeyValue indicates that a line could not be parsed as a key=value.
+	BadKeyValue ParseErrorType = "bad key/value line"
+
+	// UnknownSection indicates that the section name is unknown.
+	UnknownSection ParseErrorType = "unknown section name"
+
+	// UnknownKeyInDefault indicates that the configuration key in the default section is unknown.
+	UnknownKeyInDefault ParseErrorType = "unknown key in default section"
+
+	// UnknownKeyInService indicates that the configuration key in the service section is unknown.
+	UnknownKeyInService ParseErrorType = "unknown key in 'service' section"
+
+	// InvalidValueForKey indicates that parsing the configuration value failed.
+	InvalidValueForKey ParseErrorType = "invalid value for key"
+
+	// InsecureOptionsProhibited indicates that the configuration specifies a key marked as "insecure", which is not allowed without AllowInsecureOptions.
+	InsecureOptionsProhibited ParseErrorType = "insecure option prohibited"
+)
+
+// ParseError represents an error that happened while parsing a GLOME configuration.
+type ParseError struct {
+	LineNum     int
+	ErrorType   ParseErrorType
+	Description string
+}
+
+// Error satisfies the Go `error` interface.
+func (e ParseError) Error() string {
+	descriptionSeparator := ""
+	if e.Description != "" {
+		descriptionSeparator = ": "
+	}
+	return fmt.Sprintf("config file parsing failed in line %d (%s%s%s)", e.LineNum, e.ErrorType, descriptionSeparator, e.Description)
+}
+
+var (
+	sectionAssigners = map[string]func(cfg *Config, lineNum int, key, value string, o *options) error{
+		"default": assignDefaultSection,
+		"service": assignServiceSection,
+	}
+)
+
+type options struct {
+	AllowInsecureOptions bool
+}
+
+// OptionFunc modifies the available options.
+type OptionFunc func(o *options)
+
+// AllowInsecureOptions enables the parsed config file to include options that are intended for testing only and should not be used in production.
+func AllowInsecureOptions(o *options) {
+	o.AllowInsecureOptions = true
+}
+
+// Parse parses a GLOME ini-style configuration file to a Config struct.
+func Parse(r io.Reader, opts ...OptionFunc) (*Config, error) {
+	o := &options{}
+	for _, opt := range opts {
+		opt(o)
+	}
+
+	s := bufio.NewScanner(r)
+	currentSection := "default"
+	lineNum := 0
+	cfg := new(Config)
+	for s.Scan() {
+		lineNum++
+		txt := strings.TrimSpace(s.Text())
+		switch {
+		case len(txt) == 0, txt[0] == '#', txt[0] == ';':
+			// Purely whitespace, or a comment.
+			continue
+		case txt[0] == '[':
+			// Section header
+			end := strings.IndexByte(txt, ']')
+			if end == -1 {
+				return nil, ParseError{lineNum, BadSectionName, "couldn't find closing ]"}
+			}
+			currentSection = txt[1:end]
+			if len(currentSection) == 0 {
+				return nil, ParseError{lineNum, BadSectionName, "section name was empty"}
+			}
+			if _, ok := sectionAssigners[currentSection]; !ok {
+				return nil, ParseError{lineNum, UnknownSection, currentSection}
+			}
+		default:
+			// Key value config option.
+			key, value, err := parseKeyValue(txt)
+			if err != nil {
+				return nil, ParseError{lineNum, BadKeyValue, err.Error()}
+			}
+
+			assignValue, ok := sectionAssigners[currentSection]
+			if !ok {
+				// We shouldn't end up here since we validate section names as we assign them.
+				// However, just in case...
+				return nil, ParseError{lineNum, UnknownSection, currentSection}
+			}
+			if err := assignValue(cfg, lineNum, key, value, o); err != nil {
+				return nil, err
+			}
+		}
+
+	}
+	return cfg, nil
+}
+
+func assignDefaultSection(cfg *Config, lineNum int, key, value string, o *options) error {
+	var err error
+	switch key {
+	case "auth-delay":
+		err = interpretPositiveInt(value, &cfg.AuthDelay)
+	case "input-timeout":
+		err = interpretPositiveInt(value, &cfg.InputTimeout)
+	case "config-path":
+		cfg.ConfigPath = value
+	case "ephemeral-key":
+		if !o.AllowInsecureOptions {
+			return ParseError{lineNum, InsecureOptionsProhibited, key}
+		}
+		err = interpretPrivateKey(value, hex.DecodeString, &cfg.EphemeralKey)
+	case "min-authcode-len":
+		err = interpretPositiveInt(value, &cfg.MinAuthcodeLen)
+	case "host-id":
+		cfg.HostID = value
+	case "host-id-type":
+		cfg.HostIDType = value
+	case "login-path":
+		cfg.LoginPath = value
+	case "disable-syslog":
+		err = interpretBool(value, &cfg.DisableSyslog)
+	case "print-secrets":
+		err = interpretBool(value, &cfg.PrintSecrets)
+		if !o.AllowInsecureOptions && cfg.PrintSecrets {
+			// We only judge print-secrets as insecure if it's true.
+			return ParseError{lineNum, InsecureOptionsProhibited, key}
+		}
+	case "timeout":
+		err = interpretPositiveInt(value, &cfg.Timeout)
+	case "verbose":
+		err = interpretBool(value, &cfg.Verbose)
+	default:
+		return ParseError{lineNum, UnknownKeyInDefault, key}
+	}
+	if err != nil {
+		return ParseError{lineNum, InvalidValueForKey, fmt.Sprintf("section: default; key: %s; provided value: %s; %s", key, value, err.Error())}
+	}
+	return nil
+}
+
+func assignServiceSection(cfg *Config, lineNum int, key, value string, o *options) error {
+	var err error
+	switch key {
+	case "key":
+		// Provided for backwards-compatibility only.
+		// TODO: to be removed in 1.0.
+		err = interpretPublicKey(value, hex.DecodeString, &cfg.ServiceConfig.PublicKey)
+	case "url-prefix":
+		// Provided for backwards-compatibility only.
+		// TODO: to be removed in 1.0.
+		cfg.ServiceConfig.Prompt = value + "/"
+	case "key-version":
+		err = interpretKeyVersion(value, &cfg.ServiceConfig.KeyVersion)
+	case "prompt":
+		cfg.ServiceConfig.Prompt = value
+	case "public-key":
+		err = interpretPublicKey(value, decodeGLOMEPublicKey, &cfg.ServiceConfig.PublicKey)
+	default:
+		return ParseError{lineNum, UnknownKeyInService, key}
+	}
+	if err != nil {
+		return ParseError{lineNum, InvalidValueForKey, fmt.Sprintf("section: service; key: %s; provided value: %s; %s", key, value, err.Error())}
+	}
+	return nil
+}
+
+// parseKeyValue parses a `key = value` string, where whitespace has been pre-removed from the head and tail.
+func parseKeyValue(line string) (key, value string, err error) {
+	// Key is the line up to the first space or =.
+	keyEnd := strings.IndexFunc(line, func(r rune) bool {
+		return unicode.IsSpace(r) || r == '='
+	})
+	if keyEnd == -1 {
+		return "", "", fmt.Errorf("couldn't find = key/value separator")
+	}
+	key = line[:keyEnd]
+	if key == "" {
+		return "", "", fmt.Errorf("empty key is invalid")
+	}
+	line = line[keyEnd:]
+
+	// Value is the line starting from the first non-space after =.
+	valueStart := strings.IndexFunc(line, func(r rune) bool {
+		return !unicode.IsSpace(r) && r != '='
+	})
+	if valueStart == -1 {
+		// Possibly an empty value.
+		valueStart = len(line)
+	}
+	separator := line[:valueStart]
+	value = line[valueStart:]
+
+	if strings.IndexByte(separator, '=') == -1 {
+		return "", "", fmt.Errorf("couldn't find = key/value separator")
+	}
+
+	return key, value, nil
+}
+
+// interpretBool parses a boolean value in the same manner as GLOME's C implementation.
+func interpretBool(value string, b *bool) error {
+	switch value {
+	case "true", "yes", "on", "1":
+		*b = true
+		return nil
+	case "false", "no", "off", "0":
+		*b = false
+		return nil
+	}
+	return fmt.Errorf("invalid boolean value %q", value)
+}
+
+// interpretPositiveInt parses a positive integer.
+func interpretPositiveInt(value string, i *int) error {
+	v, err := strconv.Atoi(value)
+	if err != nil {
+		return err
+	}
+	if v < 0 {
+		return fmt.Errorf("expected positive int, got %d", v)
+	}
+	*i = v
+	return nil
+}
+
+// interpretPrivateKey parses a encoded private key.
+func interpretPrivateKey(value string, decoder func(s string) ([]byte, error), k *glome.PrivateKey) error {
+	bs, err := decoder(value)
+	if err != nil {
+		return err
+	}
+	pk, err := glome.PrivateKeyFromSlice(bs)
+	if err != nil {
+		return err
+	}
+	copy(k[:], pk[:])
+	return nil
+}
+
+// interpretPublicKey parses a encoded public key.
+func interpretPublicKey(value string, decoder func(s string) ([]byte, error), k *glome.PublicKey) error {
+	bs, err := decoder(value)
+	if err != nil {
+		return err
+	}
+	pk, err := glome.PublicKeyFromSlice(bs)
+	if err != nil {
+		return err
+	}
+	copy(k[:], pk[:])
+	return nil
+}
+
+// interpretKeyVersion parses a key version.
+func interpretKeyVersion(value string, i *int) error {
+	v, err := strconv.Atoi(value)
+	if err != nil {
+		return err
+	}
+	if v < 0 || v > 127 {
+		return fmt.Errorf("expected int in range [0..127], got %d", v)
+	}
+	*i = v
+	return nil
+}
+
+const glomeV1PublicKeyPrefix = "glome-v1 "
+
+// decodeGLOMEPublicKey decodes an RFD002-encoded GLOME public key to a byte slice.
+func decodeGLOMEPublicKey(value string) ([]byte, error) {
+	if !strings.HasPrefix(value, glomeV1PublicKeyPrefix) {
+		return nil, fmt.Errorf("missing %q prefix", glomeV1PublicKeyPrefix)
+	}
+	value = value[len(glomeV1PublicKeyPrefix):]
+	return base64.URLEncoding.DecodeString(value)
+}
diff --git a/go/go.mod b/go/go.mod
index f774ea6..79007fd 100644
--- a/go/go.mod
+++ b/go/go.mod
@@ -2,4 +2,7 @@ module github.com/google/glome/go
 
 go 1.15
 
-require golang.org/x/crypto v0.1.0
+require (
+	github.com/google/go-cmp v0.5.9 // indirect
+	golang.org/x/crypto v0.1.0
+)
diff --git a/go/go.sum b/go/go.sum
index ae56425..b21ef6b 100644
--- a/go/go.sum
+++ b/go/go.sum
@@ -1,3 +1,5 @@
+github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=
+github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
 github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY=
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
 golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=


Test patch:
diff --git a/go/config/config_test.go b/go/config/config_test.go
new file mode 100644
index 0000000..bf0123d
--- /dev/null
+++ b/go/config/config_test.go
@@ -0,0 +1,366 @@
+// Copyright 2023 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package config
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/google/glome/go/glome"
+	"github.com/google/go-cmp/cmp"
+)
+
+const (
+	sampleConfigPath = "../../login"
+)
+
+func TestParseKeyValue(t *testing.T) {
+	tcs := []struct {
+		line      string
+		wantKey   string
+		wantValue string
+		wantErr   bool
+	}{{
+		line:      "a = b",
+		wantKey:   "a",
+		wantValue: "b",
+	}, {
+		line:      "a=b",
+		wantKey:   "a",
+		wantValue: "b",
+	}, {
+		line:      "some-hyphenated-key\t\t=some value with spaces",
+		wantKey:   "some-hyphenated-key",
+		wantValue: "some value with spaces",
+	}}
+	for _, tc := range tcs {
+		t.Run(tc.line, func(t *testing.T) {
+			gotKey, gotValue, err := parseKeyValue(tc.line)
+			if tc.wantErr != (err != nil) {
+				t.Fatalf("parseKeyValue: %v (want err? %v)", err, tc.wantErr)
+			}
+
+			if gotKey != tc.wantKey {
+				t.Errorf("parseKeyValue: key = %q; want %q", gotKey, tc.wantKey)
+			}
+			if gotValue != tc.wantValue {
+				t.Errorf("parseKeyValue: key = %q; want %q", gotValue, tc.wantValue)
+			}
+		})
+	}
+}
+
+func TestParseError(t *testing.T) {
+	tcs := []struct {
+		name       string
+		err        ParseError
+		wantString string
+	}{{
+		name:       "descriptionless",
+		err:        ParseError{1337, BadSectionName, ""},
+		wantString: "config file parsing failed in line 1337 (bad section header line)",
+	}, {
+		name:       "with description",
+		err:        ParseError{1337, BadSectionName, "something went wrong"},
+		wantString: "config file parsing failed in line 1337 (bad section header line: something went wrong)",
+	}}
+	for _, tc := range tcs {
+		t.Run(tc.name, func(t *testing.T) {
+			gotString := tc.err.Error()
+			if gotString != tc.wantString {
+				t.Errorf("tc.err.Error() = %q; want %q", gotString, tc.wantString)
+			}
+		})
+	}
+}
+
+func TestDefaultSectionUndefined(t *testing.T) {
+	oldSectionAssigners := sectionAssigners
+	defer func() { sectionAssigners = oldSectionAssigners }()
+
+	sectionAssigners = nil
+
+	wantErr := ParseError{1, UnknownSection, "default"}
+	_, err := Parse(strings.NewReader("test = a\n"))
+	if diff := cmp.Diff(wantErr, err); diff != "" {
+		t.Errorf("Parse: got diff (-want +got)\n%v", diff)
+	}
+}
+
+func TestParseConfig(t *testing.T) {
+	for _, tc := range []struct {
+		name    string
+		config  string
+		options []OptionFunc
+		want    *Config
+	}{{
+		name: "insecure config",
+		config: `
+; Semicolon comments are allowed
+# As are hash comments
+
+auth-delay = 20
+input-timeout = 10
+config-path = /etc/glome/glome.cfg
+ephemeral-key = 77076d0a7318a57d3c16c17251b26645df4c2f87ebc0992ab177fba51db92c2a
+min-authcode-len = 5
+host-id = myhost.corp.big.example
+host-id-type = bigcorp-machine-identifier
+login-path = /opt/bigcorp/bin/login
+disable-syslog = 0
+print-secrets = yes
+timeout = 60
+verbose = true
+
+[service]
+prompt = glome://
+key-version = 27
+public-key = glome-v1 aqA9yqe1RXoOT6HrmCbF40wVUhYp50FYZR9q8_X5KF4=
+`,
+		options: []OptionFunc{AllowInsecureOptions},
+		want: &Config{
+			AuthDelay:    20,
+			InputTimeout: 10,
+			ConfigPath:   "/etc/glome/glome.cfg",
+			EphemeralKey: glome.PrivateKey{
+				0x77, 0x07, 0x6d, 0x0a, 0x73, 0x18, 0xa5, 0x7d, 0x3c, 0x16, 0xc1, 0x72, 0x51, 0xb2, 0x66, 0x45,
+				0xdf, 0x4c, 0x2f, 0x87, 0xeb, 0xc0, 0x99, 0x2a, 0xb1, 0x77, 0xfb, 0xa5, 0x1d, 0xb9, 0x2c, 0x2a,
+			},
+			MinAuthcodeLen: 5,
+			HostID:         "myhost.corp.big.example",
+			HostIDType:     "bigcorp-machine-identifier",
+			LoginPath:      "/opt/bigcorp/bin/login",
+			DisableSyslog:  false,
+			PrintSecrets:   true,
+			Timeout:        60,
+			Verbose:        true,
+
+			ServiceConfig: ServiceConfig{
+				Prompt:     "glome://",
+				KeyVersion: 27,
+				PublicKey: glome.PublicKey{
+					0x6a, 0xa0, 0x3d, 0xca, 0xa7, 0xb5, 0x45, 0x7a, 0x0e, 0x4f, 0xa1,
+					0xeb, 0x98, 0x26, 0xc5, 0xe3, 0x4c, 0x15, 0x52, 0x16, 0x29, 0xe7,
+					0x41, 0x58, 0x65, 0x1f, 0x6a, 0xf3, 0xf5, 0xf9, 0x28, 0x5e,
+				},
+			},
+		},
+	}, {
+		name: "config",
+		config: `
+; Semicolon comments are allowed
+# As are hash comments
+
+auth-delay = 20
+input-timeout = 10
+config-path = /etc/glome/glome.cfg
+min-authcode-len = 5
+host-id = myhost.corp.big.example
+host-id-type = bigcorp-machine-identifier
+login-path = /opt/bigcorp/bin/login
+disable-syslog = 0
+print-secrets = no
+timeout = 60
+verbose = true
+
+[service]
+prompt = glome://
+key-version = 27
+public-key = glome-v1 aqA9yqe1RXoOT6HrmCbF40wVUhYp50FYZR9q8_X5KF4=
+`,
+		want: &Config{
+			AuthDelay:      20,
+			InputTimeout:   10,
+			ConfigPath:     "/etc/glome/glome.cfg",
+			MinAuthcodeLen: 5,
+			HostID:         "myhost.corp.big.example",
+			HostIDType:     "bigcorp-machine-identifier",
+			LoginPath:      "/opt/bigcorp/bin/login",
+			DisableSyslog:  false,
+			PrintSecrets:   false,
+			Timeout:        60,
+			Verbose:        true,
+
+			ServiceConfig: ServiceConfig{
+				Prompt:     "glome://",
+				KeyVersion: 27,
+				PublicKey: glome.PublicKey{
+					0x6a, 0xa0, 0x3d, 0xca, 0xa7, 0xb5, 0x45, 0x7a, 0x0e, 0x4f, 0xa1,
+					0xeb, 0x98, 0x26, 0xc5, 0xe3, 0x4c, 0x15, 0x52, 0x16, 0x29, 0xe7,
+					0x41, 0x58, 0x65, 0x1f, 0x6a, 0xf3, 0xf5, 0xf9, 0x28, 0x5e,
+				},
+			},
+		},
+	}} {
+		got, err := Parse(strings.NewReader(tc.config), tc.options...)
+		if err != nil {
+			t.Errorf("%v: Parse: %v", tc.name, err)
+			continue
+		}
+
+		if diff := cmp.Diff(tc.want, got); diff != "" {
+			t.Errorf("%v: Parse: got diff (-want, +got):\n%s", tc.name, diff)
+		}
+	}
+}
+
+func TestParseConfig_Errors(t *testing.T) {
+	tcs := []struct {
+		name      string
+		config    string
+		options   []OptionFunc
+		wantError ParseErrorType
+	}{{
+		name:      "invalid section header",
+		config:    "[",
+		wantError: BadSectionName,
+	}, {
+		name:      "empty section name",
+		config:    "[]",
+		wantError: BadSectionName,
+	}, {
+		name:      "unknown section",
+		config:    "[this-section-does-not-exist]",
+		wantError: UnknownSection,
+	}, {
+		name:      "invalid config line",
+		config:    "hello",
+		wantError: BadKeyValue,
+	}, {
+		name:      "invalid config line with spaces",
+		config:    "hello a b c",
+		wantError: BadKeyValue,
+	}, {
+		name:      "missing key",
+		config:    "= true",
+		wantError: BadKeyValue,
+	}, {
+		name:      "unknown key in default section",
+		config:    "unknown-key = true",
+		wantError: UnknownKeyInDefault,
+	}, {
+		name: "unknown key in service section",
+		config: `
+[service]
+unknown-key = true
+`,
+		wantError: UnknownKeyInService,
+	}, {
+		name:      "missing value for boolean",
+		config:    "verbose =",
+		wantError: InvalidValueForKey,
+	}, {
+		name:      "invalid value for boolean",
+		config:    "verbose = invalid",
+		wantError: InvalidValueForKey,
+	}, {
+		name:      "invalid value for positive int (negative)",
+		config:    "auth-delay = -1",
+		wantError: InvalidValueForKey,
+	}, {
+		name:      "invalid value for positive int (garbage)",
+		config:    "auth-delay = invalid",
+		wantError: InvalidValueForKey,
+	}, {
+		name:      "invalid value for key version (negative)",
+		config:    "[service]\nkey-version = -1",
+		wantError: InvalidValueForKey,
+	}, {
+		name:      "invalid value for key version (garbage)",
+		config:    "[service]\nkey-version = invalid",
+		wantError: InvalidValueForKey,
+	}, {
+		name:      "invalid value for key version (too big)",
+		config:    "[service]\nkey-version = 128",
+		wantError: InvalidValueForKey,
+	}, {
+		name:      "insecure option specified without AllowInsecureOptions",
+		config:    "ephemeral-key = anything",
+		wantError: InsecureOptionsProhibited,
+	}, {
+		name:      "print-secrets specified without AllowInsecureOptions",
+		config:    "print-secrets = true",
+		wantError: InsecureOptionsProhibited,
+	}, {
+		name:      "invalid value for private key (garbage)",
+		options:   []OptionFunc{AllowInsecureOptions},
+		config:    "ephemeral-key = invalid",
+		wantError: InvalidValueForKey,
+	}, {
+		name:      "invalid value for private key (too short)",
+		config:    "ephemeral-key = aa",
+		options:   []OptionFunc{AllowInsecureOptions},
+		wantError: InvalidValueForKey,
+	}, {
+		name:      "invalid value for legacy public key (garbage)",
+		config:    "[service]\nkey = invalid",
+		wantError: InvalidValueForKey,
+	}, {
+		name:      "invalid value for legacy public key (too short)",
+		config:    "[service]\nkey = aa",
+		wantError: InvalidValueForKey,
+	}, {
+		name:      "invalid value for public key (garbage)",
+		config:    "[service]\npublic-key = invalid",
+		wantError: InvalidValueForKey,
+	}, {
+		name:      "invalid value for public key (too short)",
+		config:    "[service]\npublic-key = glome-v1 aGkK",
+		wantError: InvalidValueForKey,
+	}}
+	for _, tc := range tcs {
+		t.Run(tc.name, func(t *testing.T) {
+			_, err := Parse(strings.NewReader(tc.config), tc.options...)
+			if err == nil {
+				t.Fatalf("Parse didn't return an error; I expected one")
+			}
+
+			cpe, ok := err.(ParseError)
+			if !ok {
+				t.Fatalf("Parse: %v (wanted a ParseError)", err)
+			}
+
+			if cpe.ErrorType != tc.wantError {
+				t.Errorf("Parse: %v (error type was %q; want %q)", err, cpe.ErrorType, tc.wantError)
+			}
+		})
+	}
+}
+
+func TestParseConfig_InTreeSamples(t *testing.T) {
+	names, err := filepath.Glob(filepath.Join(sampleConfigPath, "*.cfg"))
+	if err != nil {
+		t.Fatalf("finding sample config files: %v", err)
+	}
+	if len(names) == 0 {
+		t.Fatal("no sample config files found in //login/*.cfg")
+	}
+
+	for _, name := range names {
+		t.Run(filepath.Base(name), func(t *testing.T) {
+			f, err := os.Open(name)
+			if err != nil {
+				t.Fatalf("os.Open(%q): %v", name, err)
+			}
+			defer f.Close()
+
+			if _, err := Parse(f, AllowInsecureOptions); err != nil {
+				t.Errorf("Parse: %v", err)
+			}
+		})
+	}
+}