Skip to content

Commit

Permalink
Merge pull request #82 from slok/slok/age-public-keys-new-lines
Browse files Browse the repository at this point in the history
Improve age public keys loading by sanitizing the input data
  • Loading branch information
slok authored May 22, 2021
2 parents 24c5bd6 + 796941d commit 19939ea
Show file tree
Hide file tree
Showing 4 changed files with 152 additions and 88 deletions.
7 changes: 6 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,16 @@

## [Unreleased]

### Changed

- Fixed bug that wouldn't allow loading `X25519` (Age) public keys with comments or newlines.
- Allow loading `X25519` (Age) public keys in the form of `Public key: {PUBLIC_KEY}` (e.g: Using `age-keygen -o ./priv.key 2> ./pub.key`).

## [v0.5.1] - 2021-05-15

### Changed

- Fixed bug that wouldn't allow loading `X25519` (Age) keys with comments or newlines.
- Fixed bug that wouldn't allow loading `X25519` (Age) private keys with comments or newlines.

## [v0.5.0] - 2021-05-03

Expand Down
100 changes: 13 additions & 87 deletions internal/key/age/age.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,8 @@ import (
"context"
"fmt"
"io"
"os"
"regexp"
"strings"

"filippo.io/age"
"filippo.io/age/agessh"
"golang.org/x/crypto/ssh"
"golang.org/x/term"

"github.com/slok/agebox/internal/key"
"github.com/slok/agebox/internal/log"
Expand Down Expand Up @@ -52,15 +46,18 @@ func (p PrivateKey) AgeIdentity() age.Identity { return p.identity }

var _ model.PrivateKey = &PrivateKey{}

type publicKeyParser func(ctx context.Context, key string) (age.Recipient, error)
type privateKeyParser func(ctx context.Context, key string) (age.Identity, error)

type factory struct {
// These are the key parsers used to load keys, they will work in
// brute force mode being used as a chain, if one fails we continue
// until one is correct.
//
// TODO(slok): We could optimize this as age does, checking
// the keys headers and selecting the correct one.
publicKeyParsers []func(string) (age.Recipient, error)
privateKeyParsers []func(string) (age.Identity, error)
publicKeyParsers []publicKeyParser
privateKeyParsers []privateKeyParser
}

// Factory is the key.Factory implementation for age supported keys.
Expand All @@ -72,13 +69,13 @@ func NewFactory(passphraseReader io.Reader, logger log.Logger) key.Factory {
logger = logger.WithValues(log.Kv{"svc": "key.age.Factory"})

return factory{
publicKeyParsers: []func(string) (age.Recipient, error){
agessh.ParseRecipient,
func(d string) (age.Recipient, error) { return age.ParseX25519Recipient(d) },
publicKeyParsers: []publicKeyParser{
parseSSHPublic(),
parseAgePublic(),
},
privateKeyParsers: []func(string) (age.Identity, error){
parseSSHIdentityFunc(passphraseReader, logger),
parseAgeIdentityFunc(),
privateKeyParsers: []privateKeyParser{
parseSSHPrivateFunc(passphraseReader, logger),
parseAgePrivateFunc(),
},
}
}
Expand All @@ -88,7 +85,7 @@ var _ key.Factory = factory{}
func (f factory) GetPublicKey(ctx context.Context, data []byte) (model.PublicKey, error) {
sdata := string(data)
for _, f := range f.publicKeyParsers {
recipient, err := f(sdata)
recipient, err := f(ctx, sdata)
// If no error, we have our public key.
if err == nil {
return PublicKey{
Expand All @@ -104,7 +101,7 @@ func (f factory) GetPublicKey(ctx context.Context, data []byte) (model.PublicKey
func (f factory) GetPrivateKey(ctx context.Context, data []byte) (model.PrivateKey, error) {
sdata := string(data)
for _, f := range f.privateKeyParsers {
identity, err := f(sdata)
identity, err := f(ctx, sdata)
// If no error, we have our private key.
if err == nil {
return PrivateKey{
Expand All @@ -116,74 +113,3 @@ func (f factory) GetPrivateKey(ctx context.Context, data []byte) (model.PrivateK

return nil, fmt.Errorf("invalid private key")
}

func parseSSHIdentityFunc(passphraseR io.Reader, logger log.Logger) func(string) (age.Identity, error) {
return func(d string) (age.Identity, error) {
// Get the SSH private key.
secretData := []byte(d)
id, err := agessh.ParseIdentity(secretData)
if err == nil {
return id, nil
}

// If passphrase required, ask for it.
sshErr, ok := err.(*ssh.PassphraseMissingError)
if !ok {
return nil, err
}

if sshErr.PublicKey == nil {
return nil, fmt.Errorf("passphrase required and public key can't be obtained from private key")
}

// Ask for passphrase and get identity.
i, err := agessh.NewEncryptedSSHIdentity(sshErr.PublicKey, secretData, askPasswordStdin(passphraseR, logger))
if err != nil {
return nil, err
}

return i, nil
}
}

func askPasswordStdin(r io.Reader, logger log.Logger) func() ([]byte, error) {
return func() ([]byte, error) {
// If not stdin just return the passphrase.
if r != os.Stdin {
return io.ReadAll(r)
}

// Check if is a valid terminal and try getting it.
fd := int(os.Stdin.Fd())
if !term.IsTerminal(fd) {
tty, err := os.Open("/dev/tty")
if err != nil {
return nil, fmt.Errorf("standard input is not available or not a terminal, and opening /dev/tty failed: %v", err)
}
defer tty.Close()
fd = int(tty.Fd())
}

// Ask for password.
logger.Warningf("SSH key passphrase required")
logger.Infof("Enter passphrase for ssh key: ")

p, err := term.ReadPassword(fd)
if err != nil {
return nil, err
}

return p, nil
}
}

var removeCommentRegexp = regexp.MustCompile("(?m)(^#.*$)")

func parseAgeIdentityFunc() func(s string) (age.Identity, error) {
return func(d string) (age.Identity, error) {
d = removeCommentRegexp.ReplaceAllString(d, "")
d = strings.TrimSpace(d)

return age.ParseX25519Identity(d)
}
}
22 changes: 22 additions & 0 deletions internal/key/age/age_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,28 @@ func TestKeyFactoryPublicKey(t *testing.T) {
key: `age1dsnalzl92c076vh54s3xwqet87de2qde60gcfrpwnm9t3ghc6s7qadhjay`,
expErr: false,
},

"X25519 keys with newlines should be valid.": {
key: ` age1dsnalzl92c076vh54s3xwqet87de2qde60gcfrpwnm9t3ghc6s7qadhjay
`,
expErr: false,
},

"X25519 keys with default age-keygen comment should be valid.": {
key: `
Public key: age1dsnalzl92c076vh54s3xwqet87de2qde60gcfrpwnm9t3ghc6s7qadhjay
`,
expErr: false,
},
"X25519 keys with comments should be valid.": {
key: `
# Public key
age1dsnalzl92c076vh54s3xwqet87de2qde60gcfrpwnm9t3ghc6s7qadhjay
`,
expErr: false,
},
}

for name, test := range tests {
Expand Down
111 changes: 111 additions & 0 deletions internal/key/age/parsers.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
package age

import (
"context"
"fmt"
"io"
"os"
"regexp"
"strings"

"filippo.io/age"
"filippo.io/age/agessh"
"golang.org/x/crypto/ssh"
"golang.org/x/term"

"github.com/slok/agebox/internal/log"
)

func parseSSHPublic() publicKeyParser {
return func(ctx context.Context, key string) (age.Recipient, error) {
return agessh.ParseRecipient(key)
}
}

var (
// Some users could make this directly `age-keygen -o ./priv.key 2> ./pub.key`
// This will create a public key in the form of: `Public key: {KEY}`, so we help the user
// by removing this so we can load keys of this kind directly, anyway, if the key is invalid
// for any other reason than this, age library will not load an return an error.
removeAgeDefPhraseRegexp = regexp.MustCompile("(?m)(^Public key:)")
removeCommentRegexp = regexp.MustCompile("(?m)(^#.*$)")
)

func parseAgePublic() publicKeyParser {
return func(ctx context.Context, key string) (age.Recipient, error) {
key = removeCommentRegexp.ReplaceAllString(key, "")
key = removeAgeDefPhraseRegexp.ReplaceAllString(key, "")
key = strings.TrimSpace(key)

return age.ParseX25519Recipient(key)
}
}

func parseAgePrivateFunc() privateKeyParser {
return func(ctx context.Context, key string) (age.Identity, error) {
key = removeCommentRegexp.ReplaceAllString(key, "")
key = strings.TrimSpace(key)

return age.ParseX25519Identity(key)
}
}

func parseSSHPrivateFunc(passphraseR io.Reader, logger log.Logger) privateKeyParser {
return func(ctx context.Context, key string) (age.Identity, error) {
// Get the SSH private key.
secretData := []byte(key)
id, err := agessh.ParseIdentity(secretData)
if err == nil {
return id, nil
}

// If passphrase required, ask for it.
sshErr, ok := err.(*ssh.PassphraseMissingError)
if !ok {
return nil, err
}

if sshErr.PublicKey == nil {
return nil, fmt.Errorf("passphrase required and public key can't be obtained from private key")
}

// Ask for passphrase and get identity.
i, err := agessh.NewEncryptedSSHIdentity(sshErr.PublicKey, secretData, askPasswordStdin(passphraseR, logger))
if err != nil {
return nil, err
}

return i, nil
}
}

func askPasswordStdin(r io.Reader, logger log.Logger) func() ([]byte, error) {
return func() ([]byte, error) {
// If not stdin just return the passphrase.
if r != os.Stdin {
return io.ReadAll(r)
}

// Check if is a valid terminal and try getting it.
fd := int(os.Stdin.Fd())
if !term.IsTerminal(fd) {
tty, err := os.Open("/dev/tty")
if err != nil {
return nil, fmt.Errorf("standard input is not available or not a terminal, and opening /dev/tty failed: %v", err)
}
defer tty.Close()
fd = int(tty.Fd())
}

// Ask for password.
logger.Warningf("SSH key passphrase required")
logger.Infof("Enter passphrase for ssh key: ")

p, err := term.ReadPassword(fd)
if err != nil {
return nil, err
}

return p, nil
}
}

0 comments on commit 19939ea

Please sign in to comment.