Merge pull request #71 from smlx/ecdh-gpg-decrypt

Implement ECDH for gpg-agent

README.md

@@ -90,7 +90,7 @@ If you have a Mac, I'd love to add support for `launchd` socket activation. See
 | ---                     | ---          | ---     |
 | ECDSA Sign (NIST P-256) | ✅           | ✅      |
 | EDDSA Sign (Curve25519) | ⏳           | ⏳      |
-| ECDH Decrypt            | ⏳           | ❌      |
+| ECDH Decrypt            | ✅           | ✅      |
 | RSA Sign                | ❌           | ✅      |
 | RSA Decrypt             | ❌           | ✅      |
 

cmd/piv-agent/list.go

@@ -39,7 +39,6 @@ func (cmd *ListCmd) Run(l *zap.Logger) error {
 		}
 	}
 	if keyformats["gpg"] {
-		fmt.Println("\nGPG keys:")
 		for _, k := range securityKeys {
 			ss, err := k.StringsGPG(cmd.PGPName, cmd.PGPEmail)
 			if err != nil {

cmd/piv-agent/main.go

@@ -14,8 +14,8 @@ var (
 type CLI struct {
 	Debug bool     `kong:"help='Enable debug logging'"`
 	Serve ServeCmd `kong:"cmd,default=1,help='(default) Listen for signing requests'"`
-	Setup SetupCmd `kong:"cmd,help='Set up the security key for use with SSH'"`
-	List  ListCmd  `kong:"cmd,help='List signing keys available on each security key'"`
+	Setup SetupCmd `kong:"cmd,help='Set up the hardware security key for use with piv-agent'"`
+	List  ListCmd  `kong:"cmd,help='List cryptographic keys available on each hardware security key'"`
 }
 
 func main() {

cmd/piv-agent/setup.go

@@ -13,10 +13,11 @@ import (
 
 // SetupCmd represents the setup command.
 type SetupCmd struct {
-	Card             string `kong:"help='Specify a smart card device'"`
-	ResetSecurityKey bool   `kong:"help='Overwrite any existing keys'"`
-	PIN              uint64 `kong:"help='Set the PIN/PUK of the device (6-8 digits). Will be prompted interactively if not provided.'"`
-	AllTouchPolicies bool   `kong:"default='true',help='Create two additional keys with touch policies always and never (default true)'"`
+	Card             string   `kong:"help='Specify a smart card device'"`
+	ResetSecurityKey bool     `kong:"help='Overwrite any existing keys'"`
+	PIN              uint64   `kong:"help='Set the PIN/PUK of the device (6-8 digits). Will be prompted interactively if not provided.'"`
+	SigningKeys      []string `kong:"default='cached,always,never',enum='cached,always,never',help='Generate signing keys with various touch policies (cached,always,never)'"`
+	DecryptingKey    bool     `kong:"default='true',help='Generate a decrypting key (default true)'"`
 }
 
 func interactivePIN() (uint64, error) {
@@ -60,7 +61,7 @@ func (cmd *SetupCmd) Run() error {
 		return fmt.Errorf("couldn't get security key: %v", err)
 	}
 	err = k.Setup(strconv.FormatUint(cmd.PIN, 10), version,
-		cmd.ResetSecurityKey)
+		cmd.ResetSecurityKey, cmd.SigningKeys, cmd.DecryptingKey)
 	if errors.Is(err, securitykey.ErrNotReset) {
 		return fmt.Errorf("--reset-security-key not specified: %w", err)
 	}

internal/assuan/assuan.go

@@ -9,7 +9,6 @@ import (
 	"encoding/hex"
 	"fmt"
 	"io"
-	"regexp"
 	"strconv"
 	"strings"
 
@@ -22,13 +21,11 @@ import (
 type KeyService interface {
 	Name() string
 	HaveKey([][]byte) (bool, []byte, error)
+	Keygrips() ([][]byte, error)
 	GetSigner([]byte) (crypto.Signer, error)
 	GetDecrypter([]byte) (crypto.Decrypter, error)
 }
 
-var ciphertextRegex = regexp.MustCompile(
-	`^D \(7:enc-val\(3:rsa\(1:a(\d+):(.+)\)\)\)$`)
-
 // New initialises a new gpg-agent server assuan FSM.
 // It returns a *fsm.Machine configured in the ready state.
 func New(rw io.ReadWriter, log *zap.Logger, ks ...KeyService) *Assuan {
@@ -68,16 +65,31 @@ func New(rw io.ReadWriter, log *zap.Logger, ks ...KeyService) *Assuan {
 						err = fmt.Errorf("unknown getinfo command: %q", assuan.data[0])
 					}
 				case havekey:
-					// HAVEKEY arguments are a list of keygrips
-					// if _any_ key is available, we return OK, otherwise
-					// No_Secret_Key.
+					// HAVEKEY arguments are either:
+					// * a list of keygrips; or
+					// * --list=1000
+					// if _any_ key is available, we return OK, otherwise No_Secret_Key.
+					// handle --list
+					if bytes.HasPrefix(assuan.data[0], []byte("--list")) {
+						var grips []byte
+						grips, err = allKeygrips(ks)
+						if err != nil {
+							_, _ = io.WriteString(rw, "ERR 1 couldn't list keygrips\n")
+							return err
+						}
+						// apply buggy libgcrypt encoding
+						_, err = io.WriteString(rw, fmt.Sprintf("D %s\nOK\n",
+							PercentEncodeSExp(grips)))
+						return err
+					}
+					// handle list of keygrips
 					keygrips, err = hexDecode(assuan.data...)
 					if err != nil {
 						return fmt.Errorf("couldn't decode keygrips: %v", err)
 					}
 					keyFound, _, err = haveKey(ks, keygrips)
 					if err != nil {
-						_, err = io.WriteString(rw, "ERR 1 couldn't check for keygrip\n")
+						_, _ = io.WriteString(rw, "ERR 1 couldn't check for keygrip\n")
 						return err
 					}
 					if keyFound {
@@ -91,7 +103,10 @@ func New(rw io.ReadWriter, log *zap.Logger, ks ...KeyService) *Assuan {
 					// ignore scdaemon requests
 					_, err = io.WriteString(rw, "ERR 100696144 No such device <SCD>\n")
 				case readkey:
-					// READKEY argument is a keygrip
+					// READKEY argument is a keygrip, optionally prefixed by "--".
+					if bytes.Equal(assuan.data[0], []byte("--")) {
+						assuan.data = assuan.data[1:]
+					}
 					// return information about the given key
 					keygrips, err = hexDecode(assuan.data...)
 					if err != nil {
@@ -265,27 +280,13 @@ func New(rw io.ReadWriter, log *zap.Logger, ks ...KeyService) *Assuan {
 					if len(chunks) < 1 {
 						return fmt.Errorf("invalid ciphertext format")
 					}
-					sexp := bytes.Join(chunks[:], []byte("\n"))
-					matches := ciphertextRegex.FindAllSubmatch(sexp, -1)
 					var plaintext, ciphertext []byte
-					ciphertext = matches[0][2]
-					log.Debug("raw ciphertext",
-						zap.Binary("sexp", sexp), zap.Binary("ciphertext", ciphertext))
-					// undo the buggy encoding sent by gpg
-					ciphertext = percentDecodeSExp(ciphertext)
-					log.Debug("normalised ciphertext",
-						zap.Binary("ciphertext", ciphertext))
+					ciphertext = bytes.Join(chunks[:], []byte("\n"))
 					plaintext, err = assuan.decrypter.Decrypt(nil, ciphertext, nil)
 					if err != nil {
 						return fmt.Errorf("couldn't decrypt: %v", err)
 					}
-					// gnupg uses the pre-buggy-encoding length in the sexp
-					plaintextLen := len(plaintext)
-					// apply the buggy encoding as expected by gpg
-					plaintext = percentEncodeSExp(plaintext)
-					plaintextSexp := fmt.Sprintf("D (5:value%d:%s)\x00\nOK\n",
-						plaintextLen, plaintext)
-					_, err = io.WriteString(rw, plaintextSexp)
+					_, err = rw.Write(plaintext)
 				case setkeydesc:
 					// ignore this event since we don't currently use the client's
 					// description in the prompt
@@ -345,6 +346,22 @@ func haveKey(ks []KeyService, keygrips [][]byte) (bool, []byte, error) {
 	return false, nil, nil
 }
 
+// allKeygrips returns all keygrips available for any keyservice, concatenated
+// into a single byte slice.
+func allKeygrips(ks []KeyService) ([]byte, error) {
+	var grips []byte
+	for _, k := range ks {
+		kgs, err := k.Keygrips()
+		if err != nil {
+			return nil, fmt.Errorf("couldn't get keygrips for %s: %v", k.Name(), err)
+		}
+		for _, kg := range kgs {
+			grips = append(grips, kg...)
+		}
+	}
+	return grips, nil
+}
+
 // hexDecode take a list of hex-encoded bytestring values and converts them to
 // their raw byte representation.
 func hexDecode(data ...[]byte) ([][]byte, error) {
@@ -359,29 +376,3 @@ func hexDecode(data ...[]byte) ([][]byte, error) {
 	}
 	return decoded, nil
 }
-
-// Work around bug(?) in gnupg where some byte sequences are
-// percent-encoded in the sexp. Yes, really. NFI what to do if the
-// percent-encoded byte sequences themselves are part of the
-// ciphertext. Yikes.
-//
-// These two functions represent over a week of late nights stepping through
-// debug builds of libcrypt in gdb :-(
-
-// percentDecodeSExp replaces the percent-encoded byte sequences with their raw
-// byte values.
-func percentDecodeSExp(data []byte) []byte {
-	data = bytes.ReplaceAll(data, []byte{0x25, 0x32, 0x35}, []byte{0x25}) // %
-	data = bytes.ReplaceAll(data, []byte{0x25, 0x30, 0x41}, []byte{0x0a}) // \n
-	data = bytes.ReplaceAll(data, []byte{0x25, 0x30, 0x44}, []byte{0x0d}) // \r
-	return data
-}
-
-// percentEncodeSExp replaces the raw byte values with their percent-encoded
-// byte sequences.
-func percentEncodeSExp(data []byte) []byte {
-	data = bytes.ReplaceAll(data, []byte{0x25}, []byte{0x25, 0x32, 0x35})
-	data = bytes.ReplaceAll(data, []byte{0x0a}, []byte{0x25, 0x30, 0x41})
-	data = bytes.ReplaceAll(data, []byte{0x0d}, []byte{0x25, 0x30, 0x44})
-	return data
-}

internal/assuan/assuan_test.go

@@ -208,7 +208,9 @@ func TestKeyinfo(t *testing.T) {
 			defer ctrl.Finish()
 			var mockSecurityKey = mock.NewMockSecurityKey(ctrl)
 			mockSecurityKey.EXPECT().SigningKeys().AnyTimes().Return(
-				[]securitykey.SigningKey{{Public: pubKey}})
+				[]securitykey.SigningKey{
+					{CryptoKey: securitykey.CryptoKey{Public: pubKey}},
+				})
 			keyService := mock.NewMockKeyService(ctrl)
 			keyService.EXPECT().HaveKey(gomock.Any()).AnyTimes().Return(
 				true, keygrip, nil)
@@ -335,14 +337,10 @@ func TestDecryptRSAKeyfile(t *testing.T) {
 			if err != nil {
 				tt.Fatal(err)
 			}
-			keyfileService, err := gpg.New(log, mockPES, tc.keyPath)
-			if err != nil {
-				tt.Fatal(err)
-			}
 			// mockConn is a pair of buffers that the assuan statemachine reads/write
 			// to/from.
 			mockConn := MockConn{}
-			a := assuan.New(&mockConn, log, keyfileService)
+			a := assuan.New(&mockConn, log, gpg.New(log, mockPES, tc.keyPath))
 			// write all the lines into the statemachine
 			for _, in := range tc.input {
 				if _, err := mockConn.ReadBuf.WriteString(in); err != nil {
@@ -433,14 +431,10 @@ func TestSignRSAKeyfile(t *testing.T) {
 			if err != nil {
 				tt.Fatal(err)
 			}
-			keyfileService, err := gpg.New(log, mockPES, tc.keyPath)
-			if err != nil {
-				tt.Fatal(err)
-			}
 			// mockConn is a pair of buffers that the assuan statemachine reads/write
 			// to/from.
 			mockConn := MockConn{}
-			a := assuan.New(&mockConn, log, keyfileService)
+			a := assuan.New(&mockConn, log, gpg.New(log, mockPES, tc.keyPath))
 			// write all the lines into the statemachine
 			for _, in := range tc.input {
 				if _, err := mockConn.ReadBuf.WriteString(in); err != nil {
@@ -519,14 +513,122 @@ func TestReadKey(t *testing.T) {
 			if err != nil {
 				tt.Fatal(err)
 			}
-			keyfileService, err := gpg.New(log, mockPES, tc.keyPath)
+			// mockConn is a pair of buffers that the assuan statemachine reads/write
+			// to/from.
+			mockConn := MockConn{}
+			a := assuan.New(&mockConn, log, gpg.New(log, mockPES, tc.keyPath))
+			// write all the lines into the statemachine
+			for _, in := range tc.input {
+				if _, err := mockConn.ReadBuf.WriteString(in); err != nil {
+					tt.Fatal(err)
+				}
+			}
+			// start the state machine
+			if err := a.Run(context.Background()); err != nil {
+				tt.Fatal(err)
+			}
+			// check the responses
+			for _, expected := range tc.expect {
+				//spew.Dump(mockConn.WriteBuf.String())
+				line, err := mockConn.WriteBuf.ReadString(byte('\n'))
+				if err != nil && err != io.EOF {
+					tt.Fatal(err)
+				}
+				if line != expected {
+					fmt.Println("got")
+					spew.Dump(line)
+					fmt.Println("expected")
+					spew.Dump(expected)
+					tt.Fatalf("error")
+				}
+			}
+		})
+	}
+}
+
+func TestDecryptECDHKeyfile(t *testing.T) {
+	var testCases = map[string]struct {
+		keyPath string
+		input   []string
+		expect  []string
+	}{
+		// test data is taken from a successful decrypt by gpg-agent
+		"decrypt file": {
+			keyPath: "testdata/private/test-assuan2@example.com.gpg",
+			input: []string{
+				"RESET\n",
+				"OPTION ttyname=/dev/pts/12\n",
+				"OPTION ttytype=xterm-256color\n",
+				"OPTION display=:0\n",
+				"OPTION xauthority=/run/user/1000/.mutter-Xwaylandauth.PAZSA1\n",
+				"OPTION putenv=XMODIFIERS=@im=ibus\n",
+				"OPTION putenv=WAYLAND_DISPLAY=wayland-0\n",
+				"OPTION putenv=XDG_SESSION_TYPE=wayland\n",
+				"OPTION putenv=DBUS_SESSION_BUS_ADDRESS=unix:path=/run/user/1000/bus\n",
+				"OPTION putenv=QT_IM_MODULE=ibus\n",
+				"OPTION lc-ctype=en_AU.UTF-8\n",
+				"OPTION lc-messages=en_AU.UTF-8\n",
+				"GETINFO version\n",
+				"OPTION allow-pinentry-notify\n",
+				"OPTION agent-awareness=2.1.0\n",
+				"SCD SERIALNO\n",
+				"SCD SERIALNO\n",
+				"SCD KEYINFO --list=encr\n",
+				"HAVEKEY --list=1000\n",
+				"RESET\n",
+				"SETKEY 98E3311ADC66E078D1A4BEBEBBC498D1E5765A8D\n",
+				"SETKEYDESC Please+enter+the+passphrase+to+unlock+the+OpenPGP+secret+key:%0A%22test-assuan@example.com%22%0A256-bit+ECDH+key,+ID+0x419969CE7D167442,%0Acreated+2021-10-10+(main+key+ID+0xFDB0A7FF92431C37).%0A\n",
+				"PKDECRYPT\n",
+				"\x44\x20\x28\x37\x3a\x65\x6e\x63\x2d\x76\x61\x6c\x28\x34\x3a\x65\x63\x64\x68\x28\x31\x3a\x73\x34\x39\x3a\x30\xc0\xc4\x09\xb5\x8a\x36\xb8\x09\xa6\xcc\xaf\x9c\x46\x65\x92\xaa\xef\xe8\xae\x67\xb5\x28\x65\xfa\x8a\x8f\x11\x38\xed\xcc\xa5\xe6\x7a\xcf\xcb\x82\xc3\x51\xe9\xa8\x8d\xbd\xb1\x43\x49\x50\x8e\x82\x29\x28\x31\x3a\x65\x36\x35\x3a\x04\xcb\x0c\x10\x45\xaf\x3b\xfa\x3e\x44\x3c\x35\xe0\xf8\xa8\x11\xa9\xd0\x3f\x50\xc0\x93\xea\x71\x99\x81\x39\x51\xa1\x2e\x7f\xd8\x90\xd4\x1d\x89\x9f\x62\x1d\x08\xfa\x15\x81\x45\x10\x42\x92\x17\xd7\x97\xf0\x8d\x86\x9a\x74\x3d\x8a\x5e\xfb\xa3\xc3\x98\x06\xbd\x50\x29\x29\x29\x0a",
+				"END\n",
+			},
+			expect: []string{
+				"OK Pleased to meet you, process 123456789\n",
+				"OK\n",
+				"OK\n",
+				"OK\n",
+				"OK\n",
+				"OK\n",
+				"OK\n",
+				"OK\n",
+				"OK\n",
+				"OK\n",
+				"OK\n",
+				"OK\n",
+				"OK\n",
+				"D 2.2.27\n",
+				"OK\n",
+				"OK\n",
+				"OK\n",
+				"ERR 100696144 No such device <SCD>\n",
+				"ERR 100696144 No such device <SCD>\n",
+				"ERR 100696144 No such device <SCD>\n",
+				"\x44\x20\x98\xe3\x31\x1a\xdc\x66\xe0\x78\xd1\xa4\xbe\xbe\xbb\xc4\x98\xd1\xe5\x76\x5a\x8d\x0a",
+				"OK\n",
+				"OK\n",
+				"OK\n",
+				"OK\n",
+				"S INQUIRE_MAXLEN 4096\n",
+				"INQUIRE CIPHERTEXT\n",
+				"\x44\x20\x28\x35\x3a\x76\x61\x6c\x75\x65\x36\x35\x3a\x04\xc8\x50\x0c\x67\x98\x95\x86\x1b\x6c\xa4\x4f\x9f\x8d\x17\xf2\xf8\x71\xbc\xe5\xa3\xe5\xe6\xc4\xae\x01\xfa\x04\x6c\xc9\xc4\x2c\x9a\x56\x52\x2b\xab\x62\xa6\x29\xdb\x12\xc0\xc2\x62\xa0\x36\xd0\x93\x46\x99\xe5\x35\xca\xc4\xbe\xe6\x05\xa5\xae\x7f\xb2\x3c\xbb\x2f\x29\x0a",
+				"OK\n",
+			},
+		},
+	}
+	for name, tc := range testCases {
+		t.Run(name, func(tt *testing.T) {
+			ctrl := gomock.NewController(tt)
+			defer ctrl.Finish()
+			// no securityKeys available
+			mockPES := mock.NewMockPINEntryService(ctrl)
+			log, err := zap.NewDevelopment()
 			if err != nil {
 				tt.Fatal(err)
 			}
 			// mockConn is a pair of buffers that the assuan statemachine reads/write
 			// to/from.
 			mockConn := MockConn{}
-			a := assuan.New(&mockConn, log, keyfileService)
+			a := assuan.New(&mockConn, log, gpg.New(log, mockPES, tc.keyPath))
 			// write all the lines into the statemachine
 			for _, in := range tc.input {
 				if _, err := mockConn.ReadBuf.WriteString(in); err != nil {
@@ -539,7 +641,6 @@ func TestReadKey(t *testing.T) {
 			}
 			// check the responses
 			for _, expected := range tc.expect {
-				//spew.Dump(mockConn.WriteBuf.String())
 				line, err := mockConn.WriteBuf.ReadString(byte('\n'))
 				if err != nil && err != io.EOF {
 					tt.Fatal(err)