From a67e820dfa8a92108efb7a0b5fa5d0966bfb19da Mon Sep 17 00:00:00 2001 From: Dovydas Bartkevicius Date: Tue, 12 Feb 2019 12:20:58 +0000 Subject: [PATCH 1/4] Updated keyScan to ignore trailing spaces in KEYINFO attributes I've recently ran into a rather odd issue where some keys have a trailing space at the end of the card ID: > keyinfo --show-fpr --list --ssh-fpr S KEYINFO 61D048F46EE1DCE... T D2760001240102010006064000390000 OPENPGP.3 - - MD5:9b:c8:3c:7b:44:... - - This breaks key scanning as the scanner thinks there are 11 parts to it instead of 10. This change should be safe as gpg-agent returns a dash when the value is actually empty. --- agent/conn.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/agent/conn.go b/agent/conn.go index c7a0586..62cb587 100644 --- a/agent/conn.go +++ b/agent/conn.go @@ -115,7 +115,14 @@ func (conn *Conn) Close() error { } func keyScan(key *Key, line string) error { - parts := strings.Split(line, " ") + unfilteredParts := strings.Split(line, " ") + parts := []string{} + for _, p := range unfilteredParts { + if p != "" { + parts = append(parts, p) + } + } + if len(parts) != 10 { return fmt.Errorf("illegal format for KEYINFO line") } From 837a8f4fcfb57034bfd306944dbc9a1d18d5cba6 Mon Sep 17 00:00:00 2001 From: Jack Kleeman Date: Wed, 27 Feb 2019 15:46:45 +0000 Subject: [PATCH 2/4] Add a function to return a list of keygrips indexed by card ID, by solely querying the yubikey, thus ignoring any cached keys. --- agent/conn.go | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/agent/conn.go b/agent/conn.go index 62cb587..9d1c64a 100644 --- a/agent/conn.go +++ b/agent/conn.go @@ -233,6 +233,53 @@ func (conn *Conn) Keys() ([]Key, error) { return keyList, nil } +// KeyGrips returns a list of available keysgrips, indexed by CardID, by querying the card +func (conn *Conn) KeyGrips() (map[string]string, error) { + grips := map[string]string{} + + scan := func(key *Key, line string) error { + unfilteredParts := strings.Split(line, " ") + parts := []string{} + for _, p := range unfilteredParts { + if p != "" { + parts = append(parts, p) + } + } + + if len(parts) != 3 { + return fmt.Errorf("illegal format for KEYINFO line") + } + + // grips[cardID] = keygrip + grips[parts[2]] = parts[1] + + return nil + } + + respFunc := func(respType, data string) error { + if respType != "S" || !strings.HasPrefix(data, "KEYPAIRINFO ") { + return nil + } + + var key Key + if err := scan(&key, data); err != nil { + return err + } + + return nil + } + + conn.mu.Lock() + defer conn.mu.Unlock() + + err := conn.Raw(respFunc, "scd LEARN --force") + if err != nil { + return nil, err + } + + return grips, nil +} + // Raw executes a command and pipes its results to the specified ResponseFunc // parameter. func (conn *Conn) Raw(f ResponseFunc, format string, a ...interface{}) error { From e090864d4c1af97b0c208ff9f361567d40abefb9 Mon Sep 17 00:00:00 2001 From: Jack Kleeman Date: Thu, 30 May 2019 09:17:18 +0100 Subject: [PATCH 3/4] Trim leading signature zeros if needed --- agent/key.go | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/agent/key.go b/agent/key.go index 38ed34d..7210ccf 100644 --- a/agent/key.go +++ b/agent/key.go @@ -1,6 +1,7 @@ package agent import ( + "bytes" "crypto" "crypto/rsa" "encoding/hex" @@ -119,8 +120,22 @@ func (key Key) Sign(rand io.Reader, msg []byte, opts crypto.SignerOpts) (signatu return internalrsa.SignPSS(rand, priv, pssOpts.Hash, msg, pssOpts) } - return key.signPKCS1v15(msg, opts.HashFunc()) + sig, err := key.signPKCS1v15(msg, opts.HashFunc()) + if err != nil { + return nil, err + } + + if diff := len(sig) - pub.Size(); diff > 0 { + // It seems sometimes the signature can have leading zero bytes such that its over the keysize + leadingZeros := bytes.Repeat([]byte{00}, diff) + if !bytes.HasPrefix(sig, leadingZeros) { + return nil, errors.New("github.com/prep/gpg/agent: signature length too large with nonzero leading bytes") + } + // We trim the leading zeros + sig = bytes.TrimPrefix(sig, leadingZeros) + } + return sig, nil default: return nil, errors.New("github.com/prep/gpg/agent: unknown public key") } From 67e9687b465f2ad7c19f9b5215bc2022fadc8728 Mon Sep 17 00:00:00 2001 From: Alex Ciminian Date: Wed, 18 Dec 2019 12:18:50 +0000 Subject: [PATCH 4/4] Improve KEYPAIRINFO validation, tolerate new format MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Newer versions of GPG can add an extra field to the KEYPAIRINFO lines, for example: S KEYPAIRINFO F877E5110AE0878B14C75747397DA0755EC51613 OPENPGP.3 sa vs S KEYPAIRINFO A6D4D888546D31DA593F65246BB13DB2F98DB42E OPENPGP.3 This change makes our parser tolerate this format. It adds extra validation on the card ID and keygrip fields, ignoring ones that are invalid. I could not run the tests and gave up after five minutes. 🙈 --- agent/conn.go | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/agent/conn.go b/agent/conn.go index 9d1c64a..2c308f9 100644 --- a/agent/conn.go +++ b/agent/conn.go @@ -233,25 +233,32 @@ func (conn *Conn) Keys() ([]Key, error) { return keyList, nil } -// KeyGrips returns a list of available keysgrips, indexed by CardID, by querying the card +// KeyGrips returns a list of available keygrips, indexed by CardID, by querying the card func (conn *Conn) KeyGrips() (map[string]string, error) { grips := map[string]string{} scan := func(key *Key, line string) error { unfilteredParts := strings.Split(line, " ") - parts := []string{} + var parts []string for _, p := range unfilteredParts { if p != "" { parts = append(parts, p) } } - if len(parts) != 3 { - return fmt.Errorf("illegal format for KEYINFO line") + if len(parts) < 3 { + return fmt.Errorf("illegal format for KEYINFO line: %s", line) + } + + keygrip := parts[1] + cardID := parts[2] + if strings.HasPrefix(cardID, "OPENPGP.") && len(keygrip) == 40 { + // grips[cardID] = keygrip + grips[cardID] = keygrip + } else { + fmt.Printf("Warning: card ID or keygrip invalid, skipping: %s", line) } - // grips[cardID] = keygrip - grips[parts[2]] = parts[1] return nil }