From 0861193d0be542a5d6270eb794be9365ad327d1c Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Tue, 27 Jul 2021 21:38:34 +0800 Subject: [PATCH 01/20] feat: fall back to keyfiles for gpg-agent signing functionality --- cmd/piv-agent/serve.go | 11 ++- internal/assuan/assuan.go | 47 ++++++++-- internal/assuan/assuan_test.go | 46 ++++++++- internal/assuan/fsm.go | 3 + internal/assuan/helper_test.go | 8 ++ .../private/bar-protected@example.com.gpg | Bin 0 -> 1914 bytes .../testdata/private/bar@example.com.gpg | Bin 0 -> 1859 bytes internal/gpg/keygrip.go | 13 ++- internal/gpg/keygrip_test.go | 4 +- internal/mock/mock_assuan.go | 38 ++++++++ internal/mock/mock_server_gpg.go | 49 ++++++++++ internal/pinentry/pinentry.go | 40 ++++++++ internal/server/gpg.go | 88 +++++++++++++++++- 13 files changed, 326 insertions(+), 21 deletions(-) create mode 100644 internal/assuan/helper_test.go create mode 100644 internal/assuan/testdata/private/bar-protected@example.com.gpg create mode 100644 internal/assuan/testdata/private/bar@example.com.gpg create mode 100644 internal/mock/mock_server_gpg.go diff --git a/cmd/piv-agent/serve.go b/cmd/piv-agent/serve.go index 51cf72c..e69dc0d 100644 --- a/cmd/piv-agent/serve.go +++ b/cmd/piv-agent/serve.go @@ -3,9 +3,12 @@ package main import ( "context" "fmt" + "os" + "path/filepath" "time" "github.com/coreos/go-systemd/activation" + "github.com/smlx/piv-agent/internal/pinentry" "github.com/smlx/piv-agent/internal/pivservice" "github.com/smlx/piv-agent/internal/server" "github.com/smlx/piv-agent/internal/ssh" @@ -72,10 +75,16 @@ func (cmd *ServeCmd) Run(log *zap.Logger) error { return err }) } + // start GPG agent if given in agent-type flag + home, err := os.UserHomeDir() + if err != nil { + log.Warn("couldn't determine $HOME", zap.Error(err)) + } + fallbackKeys := filepath.Join(home, ".gnupg", "piv-agent.secring.gpg") if _, ok := cmd.AgentTypes["gpg"]; ok { log.Debug("starting GPG server") g.Go(func() error { - s := server.NewGPG(p, log) + s := server.NewGPG(p, &pinentry.PINEntry{}, log, fallbackKeys) err := s.Serve(ctx, ls[cmd.AgentTypes["gpg"]], exit, cmd.ExitTimeout) if err != nil { log.Debug("exiting GPG server", zap.Error(err)) diff --git a/internal/assuan/assuan.go b/internal/assuan/assuan.go index 22c90e5..a9da621 100644 --- a/internal/assuan/assuan.go +++ b/internal/assuan/assuan.go @@ -7,6 +7,7 @@ import ( "crypto" "crypto/ecdsa" "crypto/rand" + "crypto/rsa" "encoding/hex" "fmt" "io" @@ -27,6 +28,11 @@ type PIVService interface { SecurityKeys() ([]pivservice.SecurityKey, error) } +// The GPGService interface provides GPG functions used by the Assuan FSM. +type GPGService interface { + GetKey([]byte) *rsa.PrivateKey +} + // hashFunction maps the code used by assuan to the relevant hash function. var hashFunction = map[uint64]crypto.Hash{ 8: crypto.SHA256, @@ -35,7 +41,7 @@ var hashFunction = map[uint64]crypto.Hash{ // New initialises a new gpg-agent server assuan FSM. // It returns a *fsm.Machine configured in the ready state. -func New(w io.Writer, p PIVService) *Assuan { +func New(w io.Writer, p PIVService, g GPGService) *Assuan { var err error var keyFound bool var keygrip, signature []byte @@ -77,7 +83,7 @@ func New(w io.Writer, p PIVService) *Assuan { if err != nil { return fmt.Errorf("couldn't decode keygrips: %v", err) } - keyFound, _, err = haveKey(p, keygrips) + keyFound, _, err = haveKey(p, g, keygrips) if err != nil { _, _ = io.WriteString(w, "ERR 1 couldn't check for keygrip\n") return fmt.Errorf("couldn't check for keygrip: %v", err) @@ -95,7 +101,7 @@ func New(w io.Writer, p PIVService) *Assuan { if err != nil { return fmt.Errorf("couldn't decode keygrips: %v", err) } - keyFound, keygrip, err = haveKey(p, keygrips) + keyFound, keygrip, err = haveKey(p, g, keygrips) if err != nil { _, _ = io.WriteString(w, "ERR 1 couldn't check for keygrip\n") return fmt.Errorf("couldn't check for keygrip: %v", err) @@ -124,7 +130,11 @@ func New(w io.Writer, p PIVService) *Assuan { if err != nil { return fmt.Errorf("couldn't decode keygrips: %v", err) } - assuan.signingPrivKey, err = getKey(p, keygrips[0]) + assuan.signingPrivKey, err = tokenSigner(p, keygrips[0]) + if err != nil { + // fall back to keyfiles + assuan.signingPrivKey, err = keyfileSigner(g, keygrips[0]) + } if err != nil { _, _ = io.WriteString(w, "ERR 1 couldn't get key from keygrip\n") return fmt.Errorf("couldn't get key from keygrip: %v", err) @@ -188,11 +198,12 @@ func New(w io.Writer, p PIVService) *Assuan { // PIVService, and false otherwise. // It takes keygrips in raw byte format, so keygrip in hex-encoded form must // first be decoded before being passed to this function. -func haveKey(p PIVService, keygrips [][]byte) (bool, []byte, error) { +func haveKey(p PIVService, g GPGService, keygrips [][]byte) (bool, []byte, error) { securityKeys, err := p.SecurityKeys() if err != nil { return false, nil, fmt.Errorf("couldn't get security keys: %w", err) } + // check against tokens for _, sk := range securityKeys { for _, signingKey := range sk.SigningKeys() { ecdsaPubKey, ok := signingKey.Public.(*ecdsa.PublicKey) @@ -200,7 +211,7 @@ func haveKey(p PIVService, keygrips [][]byte) (bool, []byte, error) { // TODO: handle other key types continue } - thisKeygrip, err := gpg.Keygrip(ecdsaPubKey) + thisKeygrip, err := gpg.KeygripECDSA(ecdsaPubKey) if err != nil { return false, nil, fmt.Errorf("couldn't get keygrip: %w", err) } @@ -211,14 +222,20 @@ func haveKey(p PIVService, keygrips [][]byte) (bool, []byte, error) { } } } + // also check against keyfiles + for _, kg := range keygrips { + if key := g.GetKey(kg); key != nil { + return true, kg, nil + } + } return false, nil, nil } -// getKey returns the security key associated with the given keygrip. +// tokenSigner returns the security key associated with the given keygrip. // If the keygrip doesn't match any known key, err will be non-nil. // It takes a keygrip in raw byte format, so a keygrip in hex-encoded form must // first be decoded before being passed to this function. -func getKey(p PIVService, keygrip []byte) (crypto.Signer, error) { +func tokenSigner(p PIVService, keygrip []byte) (crypto.Signer, error) { securityKeys, err := p.SecurityKeys() if err != nil { return nil, fmt.Errorf("couldn't get security keys: %w", err) @@ -230,7 +247,7 @@ func getKey(p PIVService, keygrip []byte) (crypto.Signer, error) { // TODO: handle other key types continue } - thisKeygrip, err := gpg.Keygrip(ecdsaPubKey) + thisKeygrip, err := gpg.KeygripECDSA(ecdsaPubKey) if err != nil { return nil, fmt.Errorf("couldn't get keygrip: %w", err) } @@ -250,6 +267,18 @@ func getKey(p PIVService, keygrip []byte) (crypto.Signer, error) { return nil, fmt.Errorf("no matching key") } +// keyfileSigner returns a crypto.Signer associated with the given keygrip. +// If the keygrip doesn't match any known key, err will be non-nil. +// It takes a keygrip in raw byte format, so a keygrip in hex-encoded form must +// first be decoded before being passed to this function. +// The path is to a secret keys file exported from gpg. +func keyfileSigner(g GPGService, keygrip []byte) (crypto.Signer, error) { + if key := g.GetKey(keygrip); key != nil { + return key, nil + } + return nil, fmt.Errorf("no matching key") +} + func hexDecode(data ...[]byte) ([][]byte, error) { var decoded [][]byte for _, d := range data { diff --git a/internal/assuan/assuan_test.go b/internal/assuan/assuan_test.go index 28d56c6..a5fd9fd 100644 --- a/internal/assuan/assuan_test.go +++ b/internal/assuan/assuan_test.go @@ -4,6 +4,7 @@ import ( "bytes" "crypto" "crypto/ecdsa" + "encoding/hex" "fmt" "io" "math/big" @@ -15,6 +16,7 @@ import ( "github.com/smlx/piv-agent/internal/mock" "github.com/smlx/piv-agent/internal/pivservice" "github.com/smlx/piv-agent/internal/securitykey" + "github.com/smlx/piv-agent/internal/server" "golang.org/x/crypto/cryptobyte" "golang.org/x/crypto/cryptobyte/asn1" "golang.org/x/crypto/openpgp" @@ -130,7 +132,7 @@ func TestSign(t *testing.T) { writeBuf := bytes.Buffer{} // readBuf is the buffer that the assuan statemachine reads from readBuf := bytes.Buffer{} - a := assuan.New(&writeBuf, pivService) + a := assuan.New(&writeBuf, pivService, nil) // write all the lines into the statemachine for _, in := range tc.input { if _, err := readBuf.WriteString(in); err != nil { @@ -194,7 +196,7 @@ func TestKeyinfo(t *testing.T) { writeBuf := bytes.Buffer{} // readBuf is the buffer that the assuan statemachine reads from readBuf := bytes.Buffer{} - a := assuan.New(&writeBuf, pivService) + a := assuan.New(&writeBuf, pivService, nil) // write all the lines into the statemachine for _, in := range tc.input { if _, err := readBuf.WriteString(in); err != nil { @@ -247,3 +249,43 @@ func ecdsaPubKeyLoad(path string) (*ecdsa.PublicKey, error) { } return eccKey, nil } + +func hexMustDecode(s string) []byte { + raw, err := hex.DecodeString(s) + if err != nil { + panic(err) + } + return raw +} + +func TestKeyfileSigner(t *testing.T) { + var testCases = map[string]struct { + path string + keygrip []byte + protected bool + }{ + "unprotected key": { + path: "testdata/private/bar@example.com.gpg", + keygrip: hexMustDecode("23F4477DF0F0C0963F8C4DFDEA8911CE65CC7CE3"), + }, + "protected key": { + path: "testdata/private/bar-protected@example.com.gpg", + keygrip: hexMustDecode("75B7C5A35213E71BA282F64317DDB90EC5C3FEE0"), + protected: true, + }, + } + for name, tc := range testCases { + t.Run(name, func(tt *testing.T) { + ctrl := gomock.NewController(tt) + defer ctrl.Finish() + var mockPES = mock.NewMockPINEntryService(ctrl) + if tc.protected { + mockPES.EXPECT().GetPGPPassphrase(gomock.Any()).Return([]byte("trustno1"), nil) + } + g := server.NewGPG(nil, mockPES, nil, tc.path) + if _, err := assuan.KeyfileSigner(g, tc.keygrip); err != nil { + tt.Fatalf("couldn't find keygrip: %v", err) + } + }) + } +} diff --git a/internal/assuan/fsm.go b/internal/assuan/fsm.go index e27e6ca..48f3c27 100644 --- a/internal/assuan/fsm.go +++ b/internal/assuan/fsm.go @@ -2,6 +2,7 @@ package assuan import ( "crypto" + "crypto/rsa" "sync" "github.com/smlx/fsm" @@ -54,6 +55,8 @@ type Assuan struct { signingPrivKey crypto.Signer hashAlgo crypto.Hash hash []byte + // fallback keys + fallbackRSA []rsa.PrivateKey } // Occur handles an event occurence. diff --git a/internal/assuan/helper_test.go b/internal/assuan/helper_test.go new file mode 100644 index 0000000..1a0050d --- /dev/null +++ b/internal/assuan/helper_test.go @@ -0,0 +1,8 @@ +package assuan + +import "crypto" + +// KeyfileSigner wraps keyfileSigner() for testing purposes. +func KeyfileSigner(g GPGService, keygrip []byte) (crypto.Signer, error) { + return keyfileSigner(g, keygrip) +} diff --git a/internal/assuan/testdata/private/bar-protected@example.com.gpg b/internal/assuan/testdata/private/bar-protected@example.com.gpg new file mode 100644 index 0000000000000000000000000000000000000000..e82ae1b629e60b61896b0d19d4995a347c478ec7 GIT binary patch literal 1914 zcmV-=2Zi{R1%(7*03O%@3;?W`(e_#z#|v2wuHl0r#7i-sEIB#R%^;duUi>C9U%3n8 z=^4O!QrfgNW<>+icqLogHzDEIn(qsZis|3kHIDG@LrEz*ENY3hkpuX>fm`!oL~2i{ z(QhI_R$jeSP6=t!@V0Sj`Or79{)wHnVriQA1u3Sk z$>ffXTyLJXWG8E2sMc3)>uOFfIS;9p=JL=pTymv?;iUv4z)0YQOem=voOkd zzeWuH>V~8tN)bzAKY&7{`W4`qO=ri}O4+9I1wG$P5%Fg%!(V%TjH9J5_1&ns0T6(W zHBD#5OvfKto!mJ{ws`5wtJW3abjLTK{lyAMpH$&a`8?GZc6wCFue6v71*8#Q5p%ne zU0I8=dl-?r?~Z?TJB9t)YxjMg(0dFrP%7@P9qG(~O}iMy=Q#io0RRF12Ll3!jCL|G zwYTd30X6?yZS+(6cu>Gi{t2&ZAmf-ityRo$S`W_yu#h{XRsX{vht3Lh6Io32QZuP{ zHmG6Y`(kAgn0#Z`$HF%dJiukL*oJ@=b{cxWsC#jB+81QLR2o>%IC*gInKA%4f3orj z-&%D8?j(NsTCm#_%FyAK!^I=*BD7r+T)rZ@M0+=RSo4an&mVvi&JCuM_SR_c5E|xV zu;B=Y$qciGxcjWXuxbwm$^?N{dN#r7@d@Y)E+WOU85|`pE#QwOa1ZvzNi5*faFvf{ zo6~JvaX{PYdK^sj#0%KPy}(31`g&klNW8)UF1I#S>ItFuT9>@6tx6-g0<7;rz)Gy? zJPEANSb8{xg^n{4Z`ONIuLF|HeBu7>k+!>xg@BH=chq73U)3I~WW=dOg!wDX2q@8P z`7k^uvh*``J=tUihhZa-o#dpVyqOpbXbA8ygp)`EDMtFYOBx&Kt7`Mhj2A<_+k*G` zY>Zn3P0K!>3#hc*mA1Bins}KhQYM)8VZ{a^m{Fr}jQ0v+CbL|t^XCZ8gWW}e zv3xQfD(uuA=RapSQoHvnB|$O`+iT}kI*klciN0=$dTJ_Di)5NeUPI`5l9|LOC6>3* z$UKs0*BNpI=U5y7k{^Wle1I7aku@&bYP=*xHyS||I&C!ys@-nmwt#Oau5cCTTOE`6 zV^CSZ=h9}U>za16=SQ|x81zm00b%j;nc{^nz{>E;8C!eNNnRJDuWG{5b>-Rh;NyAj za1IoA&M5hTP1ylFw(PB?OZzx?sL;Pi*GxlsJZ)>i`*FK_qxPb`0aE`0HZnwy9F%7Icv@K$X7DP-w()r(23d>f0eXWtIEq&zB6E7W-U-pLW_u z<>>0_zy|SwP`wxd)+T3~m0z>f3e`Ad>Q&w4IlP4xS-X!_Q>L;(M5Y_Cg2%U4u_DcM zz&@><-vr}PqCt$f$8lV@MCW!wVdSpYvkq7~JKqV*6$e?WaF^k}_S9Fg&;IA2VsmTT z(en8+AO^|54I27={2caSD$VC+IO(7#(cc{Q40wPes6ZGPJ*DQv`Ji$79$$DnRi~5W zf%JjuP#Og;K(}yQDf$1_AZefWm;R;0{%|_#7|ylmlq42Utkivz=Ei!Sc>)gl4wGH; zKzB?oG!=8(jd4~*d}qwIhXmBaj*ojRMx+z?7q7=N4Zc2s{IE&O_5WGaw<4-hEXe;_ zk^4n8GaYoUWm(UR@}%%#uiKJ%y(8)fLXU zk#PmagNkm2CjbEz{()pvIWph08De2_EpT#gbY)|7Wn@5QcwudDY-KKEZ*7SI)C3a& z3IIM9Ap~+JK=zc}03IK?yd9hC6VquuQJVzqbELHkm2q`o*H_-)a~1mF(so2mv*5?QcKYz!O)(`lU%Ps3E8gxG}e%fbfrPoxoV5?1woUKZt zk<|B>6tHSe-_SmyIWoo-E|%3&5hWwX$fVEL8ONdB0_D~aA12Xf^M?;wEH_0nvtzm4 z8(jhsDG;(hEh&T)lvefG*3|jz9N()YbNx%cIwki>^ilUE0p~`n$<8mIs4Cb!HoCiH1FEgg#@=+5f0tKAc;r~TueK?cd z{OUHMhy-g9!z8yX2@e{jU;d#dyaP_bh&ufVT5Tkr7dTg#PUyHC4l_lwfzS-JJ&`gf z*OlQYwn6J2a5kDc`8{BfKMepHJ(M8R$W`vn7g8w+k}{k74d(7tY~K{#3L+98<*+FgmiI}xsinXyd9x$Z;GrN>I&vNkR~eo! zS=N+^) zqXl;11CZ^GNh81U~;5MB2^^*+BXedUUro^4BnERxbb%0RRC23;sUn{8pf8 zFVcJNSyMP@o_Z$z2Iw(7V423a z*-vqR^;H93#qwq`uI$vAD|p6j{n#lEdsqL8XTA@WC7MDWSm4wDPScLWuq!rPfYOyY zn!;}GhTGg9PS=`rOq?X|XjQ}c7mdbBg!A6!7tj2N75Ga18P(mkjJowc41DN&Gs=WUkvwcHSfVw&}f) zZ|KWg=hSv=OwWlsmMYjh8>OKw&~TZ9sqn9OgVH}3m7h4E7IhD7I_a?nY!BC6(_Ixt z|E0<78QsLx6xPYqQXvKa!0>zUm?Hj30h@M-*axDZ5%Fr+{rzbAyLbVUdQbgL4Nh5_ zOYGV5hZ0gI{$*CGW!a1_rJ_*JbiqG^>QzyVu(2`WFgCxsE_|dVnweDCi=)#J^x4nm z;FxRW4UKmTWmTa`yUdzViuAwK&q_AlQEmCmtpZp-BcLuA$ej$B9!77>l9OoS%)UK* zk4ri7Hq9&XbEJ3Gn$u$v-02YT_xMn|Z{&%gUZOYT35IeDb=qC^7EhpN0`P`s=l>L) z1^~_9#y}&n6eBT29MUJ9cfCt6r);Omo$!r{tD#QM!PKKfKS|OxO9XEs1c}&~%_Fj@ zQgn}D_NnuWvvXGoINu%?`gJAbjG$E4R0sS|=DGt`|0#ZcF&=n?1w6?*FH>*~X& zbSXt$B<(;ZT}6;1TJiX;FmSxH-!umGec>h#%mfA`CF96ZP>oWwA&5&(VWFqQAn8q) zQ=i-=-s!tg z$kd7p{&k>)kQG*A7-|8b9@8fZZH$F6`!LS)T4PCZv{n5vrzkWO&tM-y8gr4Uyc0++!7hu< zA9s1v4Fb7T_Ux!3PE*LlPIOOSX$RwA=m@kAVqtPXWq4t2aBO8RV{dJV0n`K&0SW*< z79j)ynJYub!|#4T=2S+=SnAS41yRxk0$~6i%K{q@1qlPfX8;8Y2?z%Q1{Dek2nzxP z76JnS0v-VZ7k~f?2@uFw>e536QPP8&3;>liQg6bEbRY^uIQiDqx}i%GMiMeDmWIJR z>B@SyI5kWFonYBAnr{KB?a*Fk27@$=npc;lWU4wH1-5h((PA^}EPd@SKfCg=$<0ju z!MTBs{0z2ju`viI1o+=$%YwWARRwBhGlKZWUJ>>fl&_$1200K2PkSR(it;r%y!TBg z?CO*(iXgcVT_BRqvx+1sGGDIr{tXC{U;{Q(630r}R`C>1VtR=!P&!7$x%MC4hgyjN zE~k;*j8>F`Q`~2I;GsKU4Ve|GAW&*Oe^PU2!mVZcqmQ xGLdVoqgm-v^xc>cr^TEWRH}lT*Vqa3EY2{LcGd+0x)bj%zNEgNrAIoQREFz&cOw7* literal 0 HcmV?d00001 diff --git a/internal/gpg/keygrip.go b/internal/gpg/keygrip.go index 1cb899a..4f4800e 100644 --- a/internal/gpg/keygrip.go +++ b/internal/gpg/keygrip.go @@ -3,6 +3,7 @@ package gpg import ( "bytes" "crypto/ecdsa" + "crypto/rsa" "crypto/sha1" "fmt" "math/big" @@ -13,7 +14,7 @@ type part struct { value []byte } -// Keygrip calculates a keygrip for an ECDSA public key. This is a SHA1 hash of +// KeygripECDSA calculates a keygrip for an ECDSA public key. This is a SHA1 hash of // public key parameters. It is pretty much undocumented outside of the // libgcrypt codebase. // @@ -22,7 +23,7 @@ type part struct { // key is byte-encoded, the parts are s-exp encoded in a particular order, and // then the s-exp is sha1-hashed to produced the keygrip, which is generally // displayed hex-encoded. -func Keygrip(pubKey *ecdsa.PublicKey) ([]byte, error) { +func KeygripECDSA(pubKey *ecdsa.PublicKey) ([]byte, error) { if pubKey == nil { return nil, fmt.Errorf("nil key") } @@ -85,3 +86,11 @@ func compute(parts []part) ([]byte, error) { s := sha1.Sum(h.Bytes()) return s[:], nil } + +// KeygripRSA calculates a keygrip for an RSA public key. +func KeygripRSA(pubKey *rsa.PublicKey) []byte { + keygrip := sha1.New() + keygrip.Write([]byte{0}) + keygrip.Write(pubKey.N.Bytes()) + return keygrip.Sum(nil) +} diff --git a/internal/gpg/keygrip_test.go b/internal/gpg/keygrip_test.go index 5c737b2..0a46e6c 100644 --- a/internal/gpg/keygrip_test.go +++ b/internal/gpg/keygrip_test.go @@ -32,7 +32,7 @@ func TestTrezorCompat(t *testing.T) { priv.D = tc.input priv.PublicKey.X, priv.PublicKey.Y = curve.ScalarBaseMult(tc.input.Bytes()) - keygrip, err := gpg.Keygrip(&priv.PublicKey) + keygrip, err := gpg.KeygripECDSA(&priv.PublicKey) if err != nil { tt.Fatal(err) } @@ -90,7 +90,7 @@ func TestKeyGrip(t *testing.T) { tt.Fatal("wrong curve") } - keygrip, err := gpg.Keygrip(eccKey) + keygrip, err := gpg.KeygripECDSA(eccKey) if err != nil { tt.Fatal(err) } diff --git a/internal/mock/mock_assuan.go b/internal/mock/mock_assuan.go index 4509530..58bbb58 100644 --- a/internal/mock/mock_assuan.go +++ b/internal/mock/mock_assuan.go @@ -5,6 +5,7 @@ package mock import ( + rsa "crypto/rsa" reflect "reflect" gomock "github.com/golang/mock/gomock" @@ -48,3 +49,40 @@ func (mr *MockPIVServiceMockRecorder) SecurityKeys() *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SecurityKeys", reflect.TypeOf((*MockPIVService)(nil).SecurityKeys)) } + +// MockGPGService is a mock of GPGService interface. +type MockGPGService struct { + ctrl *gomock.Controller + recorder *MockGPGServiceMockRecorder +} + +// MockGPGServiceMockRecorder is the mock recorder for MockGPGService. +type MockGPGServiceMockRecorder struct { + mock *MockGPGService +} + +// NewMockGPGService creates a new mock instance. +func NewMockGPGService(ctrl *gomock.Controller) *MockGPGService { + mock := &MockGPGService{ctrl: ctrl} + mock.recorder = &MockGPGServiceMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockGPGService) EXPECT() *MockGPGServiceMockRecorder { + return m.recorder +} + +// GetKey mocks base method. +func (m *MockGPGService) GetKey(arg0 []byte) *rsa.PrivateKey { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetKey", arg0) + ret0, _ := ret[0].(*rsa.PrivateKey) + return ret0 +} + +// GetKey indicates an expected call of GetKey. +func (mr *MockGPGServiceMockRecorder) GetKey(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetKey", reflect.TypeOf((*MockGPGService)(nil).GetKey), arg0) +} diff --git a/internal/mock/mock_server_gpg.go b/internal/mock/mock_server_gpg.go new file mode 100644 index 0000000..80d84cb --- /dev/null +++ b/internal/mock/mock_server_gpg.go @@ -0,0 +1,49 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: gpg.go + +// Package mock is a generated GoMock package. +package mock + +import ( + reflect "reflect" + + gomock "github.com/golang/mock/gomock" +) + +// MockPINEntryService is a mock of PINEntryService interface. +type MockPINEntryService struct { + ctrl *gomock.Controller + recorder *MockPINEntryServiceMockRecorder +} + +// MockPINEntryServiceMockRecorder is the mock recorder for MockPINEntryService. +type MockPINEntryServiceMockRecorder struct { + mock *MockPINEntryService +} + +// NewMockPINEntryService creates a new mock instance. +func NewMockPINEntryService(ctrl *gomock.Controller) *MockPINEntryService { + mock := &MockPINEntryService{ctrl: ctrl} + mock.recorder = &MockPINEntryServiceMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockPINEntryService) EXPECT() *MockPINEntryServiceMockRecorder { + return m.recorder +} + +// GetPGPPassphrase mocks base method. +func (m *MockPINEntryService) GetPGPPassphrase(arg0 string) ([]byte, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetPGPPassphrase", arg0) + ret0, _ := ret[0].([]byte) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetPGPPassphrase indicates an expected call of GetPGPPassphrase. +func (mr *MockPINEntryServiceMockRecorder) GetPGPPassphrase(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPGPPassphrase", reflect.TypeOf((*MockPINEntryService)(nil).GetPGPPassphrase), arg0) +} diff --git a/internal/pinentry/pinentry.go b/internal/pinentry/pinentry.go index 5869ed3..f32a3cf 100644 --- a/internal/pinentry/pinentry.go +++ b/internal/pinentry/pinentry.go @@ -12,6 +12,46 @@ type SecurityKey interface { Serial() uint32 } +// PINEntry implements useful pinentry service methods. +type PINEntry struct{} + +// GetPGPPassphrase uses pinentry to get the passphrase of the key with the +// given keygrip. +func (*PINEntry) GetPGPPassphrase(fingerprint string) ([]byte, error) { + p, err := pinentry.New() + if err != nil { + return []byte{}, fmt.Errorf("couldn't get pinentry client: %w", err) + } + defer p.Close() + err = p.Set("title", "piv-agent Passphrase Prompt") + if err != nil { + return nil, + fmt.Errorf("couldn't set title on passphrase pinentry: %w", err) + } + err = p.Set("prompt", "Please enter passphrase") + if err != nil { + return nil, + fmt.Errorf("couldn't set prompt on passphrase pinentry: %w", err) + } + err = p.Set("desc", fmt.Sprintf("PGP key fingerprint: %s", fingerprint)) + if err != nil { + return nil, + fmt.Errorf("couldn't set desc on passphrase pinentry: %w", err) + } + // optional PIN cache + err = p.Option("allow-external-password-cache") + if err != nil { + return nil, + fmt.Errorf("couldn't set option on passphrase pinentry: %w", err) + } + err = p.Set("KEYINFO", fingerprint) + if err != nil { + return nil, + fmt.Errorf("couldn't set KEYINFO on passphrase pinentry: %w", err) + } + return p.GetPin() +} + // GetPin uses pinentry to get the pin of the given token. func GetPin(k SecurityKey) func() (string, error) { return func() (string, error) { diff --git a/internal/server/gpg.go b/internal/server/gpg.go index 655d708..8d08ee4 100644 --- a/internal/server/gpg.go +++ b/internal/server/gpg.go @@ -1,28 +1,106 @@ package server +//go:generate mockgen -source=gpg.go -destination=../mock/mock_server_gpg.go -package=mock + import ( + "bytes" "context" + "crypto/rsa" "fmt" + "io" "net" + "os" "time" "github.com/smlx/piv-agent/internal/assuan" + "github.com/smlx/piv-agent/internal/gpg" "github.com/smlx/piv-agent/internal/pivservice" "go.uber.org/zap" + "golang.org/x/crypto/openpgp/packet" ) -// GPG represents an ssh-agent server. +// PINEntryService provides an interface to talk to a pinentry program. +type PINEntryService interface { + GetPGPPassphrase(string) ([]byte, error) +} + +// GPG represents a gpg-agent server. type GPG struct { - pivService *pivservice.PIVService - log *zap.Logger + pivService *pivservice.PIVService + log *zap.Logger + fallbackPrivKeys []*packet.PrivateKey + pinentry PINEntryService +} + +// LoadFallbackKeys reads the given path and returns any private keys found. +func LoadFallbackKeys(path string) ([]*packet.PrivateKey, error) { + f, err := os.Open(path) + if err != nil { + return nil, fmt.Errorf("couldn't read keyring %s: %v", path, err) + } + reader := packet.NewReader(f) + var pkt packet.Packet + var privKeys []*packet.PrivateKey + for pkt, err = reader.Next(); err != io.EOF; pkt, err = reader.Next() { + if err != nil { + return nil, fmt.Errorf("couldn't get next packet: %v", err) + } + k, ok := pkt.(*packet.PrivateKey) + if !ok { + continue + } + privKeys = append(privKeys, k) + } + return privKeys, nil } // NewGPG initialises a new gpg-agent server. -func NewGPG(p *pivservice.PIVService, l *zap.Logger) *GPG { +func NewGPG(p *pivservice.PIVService, pe PINEntryService, l *zap.Logger, path string) *GPG { + fallbackPrivKeys, err := LoadFallbackKeys(path) + if err != nil { + l.Info("couldn't load fallback RSA keys", zap.Error(err)) + } return &GPG{ pivService: p, log: l, + // fallback keyfile keys + fallbackPrivKeys: fallbackPrivKeys, + pinentry: pe, + } +} + +// GetKey returns a matching private RSA key if the keygrip matches, and nil +// otherwise. +func (g *GPG) GetKey(keygrip []byte) *rsa.PrivateKey { + for _, k := range g.fallbackPrivKeys { + pubKey, ok := k.PublicKey.PublicKey.(*rsa.PublicKey) + if !ok { + continue + } + if bytes.Equal(keygrip, gpg.KeygripRSA(pubKey)) { + if k.Encrypted { + pass, err := g.pinentry.GetPGPPassphrase( + fmt.Sprintf("%X %X %X %X", k.Fingerprint[:5], k.Fingerprint[5:10], + k.Fingerprint[10:15], k.Fingerprint[15:])) + if err != nil { + g.log.Warn("couldn't get passphrase for key", + zap.String("fingerprint", k.KeyIdString()), zap.Error(err)) + return nil + } + if err = k.Decrypt(pass); err != nil { + g.log.Warn("couldn't decrypt key", + zap.String("fingerprint", k.KeyIdString()), zap.Error(err)) + } + } + privKey, ok := k.PrivateKey.(*rsa.PrivateKey) + if !ok { + g.log.Info("not an RSA key", zap.String("fingerprint", k.KeyIdString())) + return nil + } + return privKey + } } + return nil } // Serve starts serving signing requests, and returns when the request socket @@ -44,7 +122,7 @@ func (g *GPG) Serve(ctx context.Context, l net.Listener, exit *time.Ticker, return fmt.Errorf("couldn't set deadline: %v", err) } // init protocol state machine - a := assuan.New(conn, g.pivService) + a := assuan.New(conn, g.pivService, g) // run the protocol state machine to completion // (client severs connection) if err := a.Run(conn); err != nil { From fffa169b420471fae763d570d04e4e555b83d416 Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Wed, 28 Jul 2021 02:15:20 +0800 Subject: [PATCH 02/20] fix: don't barf on gnupg non-standard hash types --- internal/server/gpg.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/server/gpg.go b/internal/server/gpg.go index 8d08ee4..2bdb262 100644 --- a/internal/server/gpg.go +++ b/internal/server/gpg.go @@ -16,6 +16,7 @@ import ( "github.com/smlx/piv-agent/internal/gpg" "github.com/smlx/piv-agent/internal/pivservice" "go.uber.org/zap" + "golang.org/x/crypto/openpgp/errors" "golang.org/x/crypto/openpgp/packet" ) @@ -42,6 +43,9 @@ func LoadFallbackKeys(path string) ([]*packet.PrivateKey, error) { var pkt packet.Packet var privKeys []*packet.PrivateKey for pkt, err = reader.Next(); err != io.EOF; pkt, err = reader.Next() { + if _, ok := err.(errors.UnsupportedError); ok { + continue // gpg writes some non-standard cruft + } if err != nil { return nil, fmt.Errorf("couldn't get next packet: %v", err) } From 3be1b9bb739ace4d41a0f73f12fefa076178a4de Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Wed, 28 Jul 2021 02:15:58 +0800 Subject: [PATCH 03/20] fix: wait longer for the gpg client before aborting It can take a while to decrypt the fallback keys if e.g. a password must be entered. --- internal/server/gpg.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/server/gpg.go b/internal/server/gpg.go index 2bdb262..90d36d1 100644 --- a/internal/server/gpg.go +++ b/internal/server/gpg.go @@ -121,8 +121,8 @@ func (g *GPG) Serve(ctx context.Context, l net.Listener, exit *time.Ticker, } // reset the exit timer exit.Reset(timeout) - // if the client stops responding for 16 seconds, give up. - if err := conn.SetDeadline(time.Now().Add(16 * time.Second)); err != nil { + // if the client stops responding for 60 seconds, give up. + if err := conn.SetDeadline(time.Now().Add(60 * time.Second)); err != nil { return fmt.Errorf("couldn't set deadline: %v", err) } // init protocol state machine From 1766e127acba2cde257f721b2270ad0572f38a78 Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Wed, 28 Jul 2021 21:31:39 +0800 Subject: [PATCH 04/20] feat: cache decryption passphrases for keyfiles --- internal/assuan/assuan_test.go | 7 ++++- internal/server/gpg.go | 54 +++++++++++++++++++++++----------- 2 files changed, 43 insertions(+), 18 deletions(-) diff --git a/internal/assuan/assuan_test.go b/internal/assuan/assuan_test.go index a5fd9fd..9683a2c 100644 --- a/internal/assuan/assuan_test.go +++ b/internal/assuan/assuan_test.go @@ -17,6 +17,7 @@ import ( "github.com/smlx/piv-agent/internal/pivservice" "github.com/smlx/piv-agent/internal/securitykey" "github.com/smlx/piv-agent/internal/server" + "go.uber.org/zap" "golang.org/x/crypto/cryptobyte" "golang.org/x/crypto/cryptobyte/asn1" "golang.org/x/crypto/openpgp" @@ -282,7 +283,11 @@ func TestKeyfileSigner(t *testing.T) { if tc.protected { mockPES.EXPECT().GetPGPPassphrase(gomock.Any()).Return([]byte("trustno1"), nil) } - g := server.NewGPG(nil, mockPES, nil, tc.path) + log, err := zap.NewDevelopment() + if err != nil { + tt.Fatal(err) + } + g := server.NewGPG(nil, mockPES, log, tc.path) if _, err := assuan.KeyfileSigner(g, tc.keygrip); err != nil { tt.Fatalf("couldn't find keygrip: %v", err) } diff --git a/internal/server/gpg.go b/internal/server/gpg.go index 90d36d1..19c34ff 100644 --- a/internal/server/gpg.go +++ b/internal/server/gpg.go @@ -31,6 +31,8 @@ type GPG struct { log *zap.Logger fallbackPrivKeys []*packet.PrivateKey pinentry PINEntryService + // cache passphrases used for decryption + passphrases [][]byte } // LoadFallbackKeys reads the given path and returns any private keys found. @@ -76,33 +78,51 @@ func NewGPG(p *pivservice.PIVService, pe PINEntryService, l *zap.Logger, path st // GetKey returns a matching private RSA key if the keygrip matches, and nil // otherwise. func (g *GPG) GetKey(keygrip []byte) *rsa.PrivateKey { + var pass []byte + var err error for _, k := range g.fallbackPrivKeys { pubKey, ok := k.PublicKey.PublicKey.(*rsa.PublicKey) if !ok { continue } - if bytes.Equal(keygrip, gpg.KeygripRSA(pubKey)) { - if k.Encrypted { - pass, err := g.pinentry.GetPGPPassphrase( - fmt.Sprintf("%X %X %X %X", k.Fingerprint[:5], k.Fingerprint[5:10], - k.Fingerprint[10:15], k.Fingerprint[15:])) - if err != nil { - g.log.Warn("couldn't get passphrase for key", - zap.String("fingerprint", k.KeyIdString()), zap.Error(err)) - return nil - } - if err = k.Decrypt(pass); err != nil { - g.log.Warn("couldn't decrypt key", - zap.String("fingerprint", k.KeyIdString()), zap.Error(err)) + if !bytes.Equal(keygrip, gpg.KeygripRSA(pubKey)) { + continue + } + if k.Encrypted { + // try existing passphrases + for _, pass := range g.passphrases { + if err = k.Decrypt(pass); err == nil { + g.log.Debug("decrypted using cached passphrase", + zap.String("fingerprint", k.KeyIdString())) + break } } - privKey, ok := k.PrivateKey.(*rsa.PrivateKey) - if !ok { - g.log.Info("not an RSA key", zap.String("fingerprint", k.KeyIdString())) + } + if k.Encrypted { + // ask for a passphrase + pass, err = g.pinentry.GetPGPPassphrase( + fmt.Sprintf("%X %X %X %X", k.Fingerprint[:5], k.Fingerprint[5:10], + k.Fingerprint[10:15], k.Fingerprint[15:])) + if err != nil { + g.log.Warn("couldn't get passphrase for key", + zap.String("fingerprint", k.KeyIdString()), zap.Error(err)) return nil } - return privKey + g.passphrases = append(g.passphrases, pass) + if err = k.Decrypt(pass); err != nil { + g.log.Warn("couldn't decrypt key", + zap.String("fingerprint", k.KeyIdString()), zap.Error(err)) + return nil + } + g.log.Debug("decrypted using passphrase", + zap.String("fingerprint", k.KeyIdString())) + } + privKey, ok := k.PrivateKey.(*rsa.PrivateKey) + if !ok { + g.log.Info("not an RSA key", zap.String("fingerprint", k.KeyIdString())) + return nil } + return privKey } return nil } From 07780d77b152b3699b54f4986c7bc463439d0b4f Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Wed, 28 Jul 2021 23:05:57 +0800 Subject: [PATCH 05/20] feat: implement decryption using rsa keyfiles --- go.mod | 1 + internal/assuan/assuan.go | 341 +++++++++--------- internal/assuan/assuan_test.go | 185 +++++++--- internal/assuan/event_enumer.go | 16 +- internal/assuan/fsm.go | 68 ++-- internal/assuan/helper_test.go | 8 - internal/assuan/run.go | 7 +- internal/assuan/sign.go | 44 +++ internal/assuan/state_enumer.go | 30 +- .../testdata/private/bar@example.com.gpg | Bin 1859 -> 0 bytes .../testdata/private/foo@example.com.gpg | Bin 0 -> 1859 bytes .../testdata/private/foo@example.com.priv.key | Bin 0 -> 1857 bytes internal/gpg/key.go | 39 ++ internal/gpg/keyfile.go | 81 +++++ internal/gpg/keygrip.go | 4 +- internal/gpg/keyservice.go | 131 +++++++ internal/gpg/keyservice_test.go | 59 +++ .../private/bar-protected@example.com.gpg | Bin .../gpg/testdata/private/bar@example.com.gpg | Bin 0 -> 1857 bytes internal/mock/mock_assuan.go | 91 ++--- ...{mock_server_gpg.go => mock_keyservice.go} | 2 +- internal/pivservice/pivservice.go | 77 ++++ internal/server/gpg.go | 119 +----- 23 files changed, 878 insertions(+), 425 deletions(-) delete mode 100644 internal/assuan/helper_test.go create mode 100644 internal/assuan/sign.go delete mode 100644 internal/assuan/testdata/private/bar@example.com.gpg create mode 100644 internal/assuan/testdata/private/foo@example.com.gpg create mode 100644 internal/assuan/testdata/private/foo@example.com.priv.key create mode 100644 internal/gpg/key.go create mode 100644 internal/gpg/keyfile.go create mode 100644 internal/gpg/keyservice.go create mode 100644 internal/gpg/keyservice_test.go rename internal/{assuan => gpg}/testdata/private/bar-protected@example.com.gpg (100%) create mode 100644 internal/gpg/testdata/private/bar@example.com.gpg rename internal/mock/{mock_server_gpg.go => mock_keyservice.go} (98%) diff --git a/go.mod b/go.mod index 9b6fd6b..5d47ed2 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,7 @@ go 1.15 require ( github.com/alecthomas/kong v0.2.17 github.com/coreos/go-systemd v0.0.0-20191104093116-d3cd4ed1dbcf + github.com/davecgh/go-spew v1.1.1 github.com/gen2brain/beeep v0.0.0-20200526185328-e9c15c258e28 github.com/go-piv/piv-go v1.8.0 github.com/golang/mock v1.6.0 diff --git a/internal/assuan/assuan.go b/internal/assuan/assuan.go index a9da621..eb54508 100644 --- a/internal/assuan/assuan.go +++ b/internal/assuan/assuan.go @@ -3,50 +3,40 @@ package assuan //go:generate mockgen -source=assuan.go -destination=../mock/mock_assuan.go -package=mock import ( + "bufio" "bytes" "crypto" - "crypto/ecdsa" - "crypto/rand" - "crypto/rsa" "encoding/hex" "fmt" "io" - "math/big" + "regexp" "strconv" "strings" "github.com/smlx/fsm" - "github.com/smlx/piv-agent/internal/gpg" - "github.com/smlx/piv-agent/internal/notify" - "github.com/smlx/piv-agent/internal/pivservice" - "golang.org/x/crypto/cryptobyte" - "golang.org/x/crypto/cryptobyte/asn1" + "go.uber.org/zap" + "golang.org/x/crypto/openpgp/s2k" ) -// The PIVService interface provides PIV functions used by the Assuan FSM. -type PIVService interface { - SecurityKeys() ([]pivservice.SecurityKey, error) +// The KeyService interface provides functions used by the Assuan FSM. +type KeyService interface { + Name() string + HaveKey([][]byte) (bool, []byte, error) + GetSigner([]byte) (crypto.Signer, error) + GetDecrypter([]byte) (crypto.Decrypter, error) } -// The GPGService interface provides GPG functions used by the Assuan FSM. -type GPGService interface { - GetKey([]byte) *rsa.PrivateKey -} - -// hashFunction maps the code used by assuan to the relevant hash function. -var hashFunction = map[uint64]crypto.Hash{ - 8: crypto.SHA256, - 10: crypto.SHA512, -} +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(w io.Writer, p PIVService, g GPGService) *Assuan { - var err error +func New(rw io.ReadWriter, log *zap.Logger, ks ...KeyService) *Assuan { var keyFound bool var keygrip, signature []byte var keygrips, hash [][]byte assuan := Assuan{ + reader: bufio.NewReader(rw), Machine: fsm.Machine{ State: fsm.State(ready), Transitions: assuanTransitions, @@ -55,23 +45,25 @@ func New(w io.Writer, p PIVService, g GPGService) *Assuan { assuan.OnEntry = map[fsm.State][]fsm.TransitionFunc{ fsm.State(connected): { func(e fsm.Event, _ fsm.State) error { + var err error switch Event(e) { case connect: // acknowledge connection using the format expected by the client - _, err = io.WriteString(w, + _, err = io.WriteString(rw, "OK Pleased to meet you, process 123456789\n") case reset: - assuan.signingPrivKey = nil + assuan.signer = nil + assuan.decrypter = nil assuan.hashAlgo = 0 assuan.hash = []byte{} - _, err = io.WriteString(w, "OK\n") + _, err = io.WriteString(rw, "OK\n") case option: // ignore option values - piv-agent doesn't use them - _, err = io.WriteString(w, "OK\n") + _, err = io.WriteString(rw, "OK\n") case getinfo: if bytes.Equal(assuan.data[0], []byte("version")) { // masquerade as a compatible gpg-agent - _, err = io.WriteString(w, "D 2.2.27\nOK\n") + _, err = io.WriteString(rw, "D 2.2.27\nOK\n") } else { err = fmt.Errorf("unknown getinfo command: %q", assuan.data[0]) } @@ -83,35 +75,35 @@ func New(w io.Writer, p PIVService, g GPGService) *Assuan { if err != nil { return fmt.Errorf("couldn't decode keygrips: %v", err) } - keyFound, _, err = haveKey(p, g, keygrips) + keyFound, _, err = haveKey(ks, keygrips) if err != nil { - _, _ = io.WriteString(w, "ERR 1 couldn't check for keygrip\n") - return fmt.Errorf("couldn't check for keygrip: %v", err) + _, _ = io.WriteString(rw, "ERR 1 couldn't check for keygrip\n") + return fmt.Errorf("couldn't check keygrips: %v", err) } if keyFound { - _, err = io.WriteString(w, "OK\n") + _, err = io.WriteString(rw, "OK\n") } else { - _, err = io.WriteString(w, "No_Secret_Key\n") + _, err = io.WriteString(rw, "No_Secret_Key\n") } case keyinfo: // KEYINFO arguments are a list of keygrips - // if _any_ key is available, we return OK, otherwise + // if _any_ key is available, we return info, otherwise // No_Secret_Key. keygrips, err = hexDecode(assuan.data...) if err != nil { return fmt.Errorf("couldn't decode keygrips: %v", err) } - keyFound, keygrip, err = haveKey(p, g, keygrips) + keyFound, keygrip, err = haveKey(ks, keygrips) if err != nil { - _, _ = io.WriteString(w, "ERR 1 couldn't check for keygrip\n") + _, _ = io.WriteString(rw, "ERR 1 couldn't check for keygrip\n") return fmt.Errorf("couldn't check for keygrip: %v", err) } if keyFound { - _, err = io.WriteString(w, + _, err = io.WriteString(rw, fmt.Sprintf("S KEYINFO %s D - - - P - - -\nOK\n", strings.ToUpper(hex.EncodeToString(keygrip)))) } else { - _, err = io.WriteString(w, "No_Secret_Key\n") + _, err = io.WriteString(rw, "No_Secret_Key\n") } default: return fmt.Errorf("unknown event: %v", e) @@ -119,7 +111,7 @@ func New(w io.Writer, p PIVService, g GPGService) *Assuan { return err }, }, - fsm.State(keyIsSet): { + fsm.State(signingKeyIsSet): { func(e fsm.Event, _ fsm.State) error { var err error switch Event(e) { @@ -130,20 +122,21 @@ func New(w io.Writer, p PIVService, g GPGService) *Assuan { if err != nil { return fmt.Errorf("couldn't decode keygrips: %v", err) } - assuan.signingPrivKey, err = tokenSigner(p, keygrips[0]) - if err != nil { - // fall back to keyfiles - assuan.signingPrivKey, err = keyfileSigner(g, keygrips[0]) + for _, k := range ks { + assuan.signer, err = k.GetSigner(keygrips[0]) + if err == nil { + break + } } if err != nil { - _, _ = io.WriteString(w, "ERR 1 couldn't get key from keygrip\n") - return fmt.Errorf("couldn't get key from keygrip: %v", err) + _, _ = io.WriteString(rw, "ERR 1 couldn't get key for keygrip\n") + return fmt.Errorf("couldn't get key for keygrip: %v", err) } - _, err = io.WriteString(w, "OK\n") + _, err = io.WriteString(rw, "OK\n") case setkeydesc: // ignore this event since we don't currently use the client's // description in the prompt - _, err = io.WriteString(w, "OK\n") + _, err = io.WriteString(rw, "OK\n") default: return fmt.Errorf("unknown event: %v", Event(e)) } @@ -161,29 +154,122 @@ func New(w io.Writer, p PIVService, g GPGService) *Assuan { if err != nil { return fmt.Errorf("couldn't parse uint %s: %v", assuan.data[0], err) } - if assuan.hashAlgo = hashFunction[n]; assuan.hashAlgo == 0 { - return fmt.Errorf("invalid hash algorithm value: %v", n) + var ok bool + if assuan.hashAlgo, ok = s2k.HashIdToHash(byte(n)); !ok { + return fmt.Errorf("invalid hash algorithm value: %x", n) } hash, err = hexDecode(assuan.data[1:]...) if err != nil { return fmt.Errorf("couldn't decode hash: %v", err) } assuan.hash = hash[0] - _, err = io.WriteString(w, "OK\n") + _, err = io.WriteString(rw, "OK\n") case pksign: signature, err = assuan.sign() if err != nil { return fmt.Errorf("couldn't sign: %v", err) } - _, err = w.Write(signature) + _, err = rw.Write(signature) if err != nil { return fmt.Errorf("couldn't write signature: %v", err) } - _, err = io.WriteString(w, "\n") + _, err = io.WriteString(rw, "\n") if err != nil { return fmt.Errorf("couldn't write newline: %v", err) } - _, err = io.WriteString(w, "OK\n") + _, err = io.WriteString(rw, "OK\n") + default: + return fmt.Errorf("unknown event: %v", Event(e)) + } + return err + }, + }, + fsm.State(decryptingKeyIsSet): { + func(e fsm.Event, _ fsm.State) error { + var err error + switch Event(e) { + case setkey: + // SETKEY has a single argument: a keygrip indicating the key which + // will be used for subsequent decrypting operations + keygrips, err = hexDecode(assuan.data...) + if err != nil { + return fmt.Errorf("couldn't decode keygrips: %v", err) + } + for _, k := range ks { + assuan.decrypter, err = k.GetDecrypter(keygrips[0]) + if err == nil { + break + } + } + if err != nil { + _, _ = io.WriteString(rw, "ERR 1 couldn't get key for keygrip\n") + return fmt.Errorf("couldn't get key for keygrip: %v", err) + } + _, err = io.WriteString(rw, "OK\n") + case setkeydesc: + // ignore this event since we don't currently use the client's + // description in the prompt + _, err = io.WriteString(rw, "OK\n") + default: + return fmt.Errorf("unknown event: %v", Event(e)) + } + return err + }, + }, + fsm.State(waitingForCiphertext): { + func(e fsm.Event, _ fsm.State) error { + var err error + switch Event(e) { + case pkdecrypt: + // once we receive PKDECRYPT we enter a "reversed" state where the + // agent drives the client by sending commands. + // ask for ciphertext + _, err = io.WriteString(rw, + "S INQUIRE_MAXLEN 4096\nINQUIRE CIPHERTEXT\n") + if err != nil { + return err + } + var chunk []byte + var chunks [][]byte + scanner := bufio.NewScanner(assuan.reader) + for { + if !scanner.Scan() { + break + } + chunk = scanner.Bytes() + if bytes.Equal([]byte("END"), chunk) { + break // end of ciphertext + } + chunks = append(chunks, chunk) + } + 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)) + 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) + case setkeydesc: + // ignore this event since we don't currently use the client's + // description in the prompt + _, err = io.WriteString(rw, "OK\n") default: return fmt.Errorf("unknown event: %v", Event(e)) } @@ -194,91 +280,29 @@ func New(w io.Writer, p PIVService, g GPGService) *Assuan { return &assuan } -// haveKey returns true if any of the keygrips refer to keys held by the local -// PIVService, and false otherwise. +// haveKey returns true if any of the keygrips refer to keys known locally, and +// false otherwise. // It takes keygrips in raw byte format, so keygrip in hex-encoded form must -// first be decoded before being passed to this function. -func haveKey(p PIVService, g GPGService, keygrips [][]byte) (bool, []byte, error) { - securityKeys, err := p.SecurityKeys() - if err != nil { - return false, nil, fmt.Errorf("couldn't get security keys: %w", err) - } - // check against tokens - for _, sk := range securityKeys { - for _, signingKey := range sk.SigningKeys() { - ecdsaPubKey, ok := signingKey.Public.(*ecdsa.PublicKey) - if !ok { - // TODO: handle other key types - continue - } - thisKeygrip, err := gpg.KeygripECDSA(ecdsaPubKey) - if err != nil { - return false, nil, fmt.Errorf("couldn't get keygrip: %w", err) - } - for _, kg := range keygrips { - if bytes.Equal(thisKeygrip, kg) { - return true, thisKeygrip, nil - } - } +// first be decoded before being passed to this function. It returns the +// keygrip found. +func haveKey(ks []KeyService, keygrips [][]byte) (bool, []byte, error) { + var keyFound bool + var keygrip []byte + var err error + for _, k := range ks { + keyFound, keygrip, err = k.HaveKey(keygrips) + if err != nil { + return false, nil, fmt.Errorf("couldn't check %s keygrips: %v", k.Name(), err) } - } - // also check against keyfiles - for _, kg := range keygrips { - if key := g.GetKey(kg); key != nil { - return true, kg, nil + if keyFound { + return true, keygrip, nil } } return false, nil, nil } -// tokenSigner returns the security key associated with the given keygrip. -// If the keygrip doesn't match any known key, err will be non-nil. -// It takes a keygrip in raw byte format, so a keygrip in hex-encoded form must -// first be decoded before being passed to this function. -func tokenSigner(p PIVService, keygrip []byte) (crypto.Signer, error) { - securityKeys, err := p.SecurityKeys() - if err != nil { - return nil, fmt.Errorf("couldn't get security keys: %w", err) - } - for _, k := range securityKeys { - for _, signingKey := range k.SigningKeys() { - ecdsaPubKey, ok := signingKey.Public.(*ecdsa.PublicKey) - if !ok { - // TODO: handle other key types - continue - } - thisKeygrip, err := gpg.KeygripECDSA(ecdsaPubKey) - if err != nil { - return nil, fmt.Errorf("couldn't get keygrip: %w", err) - } - if bytes.Equal(thisKeygrip, keygrip) { - cryptoPrivKey, err := k.PrivateKey(&signingKey) - if err != nil { - return nil, fmt.Errorf("couldn't get private key from slot") - } - signingPrivKey, ok := cryptoPrivKey.(crypto.Signer) - if !ok { - return nil, fmt.Errorf("private key is invalid type") - } - return signingPrivKey, nil - } - } - } - return nil, fmt.Errorf("no matching key") -} - -// keyfileSigner returns a crypto.Signer associated with the given keygrip. -// If the keygrip doesn't match any known key, err will be non-nil. -// It takes a keygrip in raw byte format, so a keygrip in hex-encoded form must -// first be decoded before being passed to this function. -// The path is to a secret keys file exported from gpg. -func keyfileSigner(g GPGService, keygrip []byte) (crypto.Signer, error) { - if key := g.GetKey(keygrip); key != nil { - return key, nil - } - return nil, fmt.Errorf("no matching key") -} - +// hexDecode take a list of hex-encoded bytestring values and converts them to +// their raw byte representation. func hexDecode(data ...[]byte) ([][]byte, error) { var decoded [][]byte for _, d := range data { @@ -292,35 +316,28 @@ func hexDecode(data ...[]byte) ([][]byte, error) { return decoded, nil } -// sign performs signing of the specified "hash" data, using the specified -// "hashAlgo" hash algorithm. It then encodes the response into an s-expression -// and returns it as a byte slice. +// 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. // -// This function's complexity is due to the fact that while Sign() returns the -// r and s components of the signature ASN1-encoded, gpg expects them to be -// separately s-exp encoded. So we have to decode the ASN1 signature, extract -// the params, and re-encode them into the s-exp. Ugh. -func (a *Assuan) sign() ([]byte, error) { - cancel := notify.Touch(nil) - defer cancel() - signature, err := a.signingPrivKey.Sign(rand.Reader, a.hash, a.hashAlgo) - if err != nil { - return nil, fmt.Errorf("couldn't sign: %v", err) - } - var sig cryptobyte.String = signature - var b []byte - if !sig.ReadASN1Bytes(&b, asn1.SEQUENCE) { - return nil, fmt.Errorf("couldn't read asn1.SEQUENCE") - } - var rawInts cryptobyte.String = b - var r, s big.Int - if !rawInts.ReadASN1Integer(&r) { - return nil, fmt.Errorf("couldn't read r as asn1.Integer") - } - if !rawInts.ReadASN1Integer(&s) { - return nil, fmt.Errorf("couldn't read s as asn1.Integer") - } - // encode the params (r, s) into s-exp - return []byte(fmt.Sprintf(`D (7:sig-val(5:ecdsa(1:r32#%X#)(1:s32#%X#)))`, - r.Bytes(), s.Bytes())), nil +// 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 } diff --git a/internal/assuan/assuan_test.go b/internal/assuan/assuan_test.go index 9683a2c..ad319c9 100644 --- a/internal/assuan/assuan_test.go +++ b/internal/assuan/assuan_test.go @@ -4,19 +4,18 @@ import ( "bytes" "crypto" "crypto/ecdsa" - "encoding/hex" "fmt" "io" "math/big" "os" "testing" + "github.com/davecgh/go-spew/spew" "github.com/golang/mock/gomock" "github.com/smlx/piv-agent/internal/assuan" + "github.com/smlx/piv-agent/internal/gpg" "github.com/smlx/piv-agent/internal/mock" - "github.com/smlx/piv-agent/internal/pivservice" "github.com/smlx/piv-agent/internal/securitykey" - "github.com/smlx/piv-agent/internal/server" "go.uber.org/zap" "golang.org/x/crypto/cryptobyte" "golang.org/x/crypto/cryptobyte/asn1" @@ -40,6 +39,21 @@ func (s *MockCryptoSigner) Sign(_ io.Reader, _ []byte, return s.Signature, nil } +// MockConn mocks a network connection, storing the read and write bytes +// internally to allow inspection. It implements io.ReadWriter. +type MockConn struct { + ReadBuf bytes.Buffer + WriteBuf bytes.Buffer +} + +func (c *MockConn) Read(p []byte) (int, error) { + return c.ReadBuf.Read(p) +} + +func (c *MockConn) Write(p []byte) (int, error) { + return c.WriteBuf.Write(p) +} + func TestSign(t *testing.T) { var testCases = map[string]struct { keyPath string @@ -119,34 +133,31 @@ func TestSign(t *testing.T) { if err != nil { tt.Fatal(err) } - var mockSecurityKey = mock.NewMockSecurityKey(ctrl) - mockSecurityKey.EXPECT().PrivateKey(gomock.Any()).Return(&MockCryptoSigner{ + keyService := mock.NewMockKeyService(ctrl) + keyService.EXPECT().HaveKey(gomock.Any()).AnyTimes().Return(true, nil, nil) + keyService.EXPECT().GetSigner(gomock.Any()).Return(&MockCryptoSigner{ PubKey: pubKey, Signature: signature, }, nil) - mockSecurityKey.EXPECT().SigningKeys().AnyTimes().Return( - []securitykey.SigningKey{{Public: pubKey}}) - pivService := mock.NewMockPIVService(ctrl) - pivService.EXPECT().SecurityKeys().AnyTimes().Return( - []pivservice.SecurityKey{mockSecurityKey}, nil) - // writeBuf is the buffer that the assuan statemachine writes to - writeBuf := bytes.Buffer{} - // readBuf is the buffer that the assuan statemachine reads from - readBuf := bytes.Buffer{} - a := assuan.New(&writeBuf, pivService, nil) + mockConn := MockConn{} + log, err := zap.NewDevelopment() + if err != nil { + tt.Fatal(err) + } + a := assuan.New(&mockConn, log, keyService) // write all the lines into the statemachine for _, in := range tc.input { - if _, err := readBuf.WriteString(in); err != nil { + if _, err := mockConn.ReadBuf.WriteString(in); err != nil { tt.Fatal(err) } } // start the state machine - if err := a.Run(&readBuf); err != nil { + if err := a.Run(); err != nil { tt.Fatal(err) } // check the responses for _, expected := range tc.expect { - line, err := writeBuf.ReadString(byte('\n')) + line, err := mockConn.WriteBuf.ReadString(byte('\n')) if err != nil && err != io.EOF { tt.Fatal(err) } @@ -185,32 +196,39 @@ func TestKeyinfo(t *testing.T) { if err != nil { tt.Fatal(err) } + keygrip, err := gpg.KeygripECDSA(pubKey) + if err != nil { + tt.Fatal(err) + } ctrl := gomock.NewController(tt) defer ctrl.Finish() var mockSecurityKey = mock.NewMockSecurityKey(ctrl) mockSecurityKey.EXPECT().SigningKeys().AnyTimes().Return( []securitykey.SigningKey{{Public: pubKey}}) - pivService := mock.NewMockPIVService(ctrl) - pivService.EXPECT().SecurityKeys().AnyTimes().Return( - []pivservice.SecurityKey{mockSecurityKey}, nil) - // writeBuf is the buffer that the assuan statemachine writes to - writeBuf := bytes.Buffer{} - // readBuf is the buffer that the assuan statemachine reads from - readBuf := bytes.Buffer{} - a := assuan.New(&writeBuf, pivService, nil) + keyService := mock.NewMockKeyService(ctrl) + keyService.EXPECT().HaveKey(gomock.Any()).AnyTimes().Return( + true, keygrip, nil) + // mockConn is a pair of buffers that the assuan statemachine reads/write + // to/from. + mockConn := MockConn{} + log, err := zap.NewDevelopment() + if err != nil { + tt.Fatal(err) + } + a := assuan.New(&mockConn, log, keyService) // write all the lines into the statemachine for _, in := range tc.input { - if _, err := readBuf.WriteString(in); err != nil { + if _, err := mockConn.ReadBuf.WriteString(in); err != nil { tt.Fatal(err) } } // start the state machine - if err := a.Run(&readBuf); err != nil { + if err := a.Run(); err != nil { tt.Fatal(err) } // check the responses for _, expected := range tc.expect { - line, err := writeBuf.ReadString(byte('\n')) + line, err := mockConn.WriteBuf.ReadString(byte('\n')) if err != nil && err != io.EOF { tt.Fatal(err) } @@ -251,45 +269,100 @@ func ecdsaPubKeyLoad(path string) (*ecdsa.PublicKey, error) { return eccKey, nil } -func hexMustDecode(s string) []byte { - raw, err := hex.DecodeString(s) - if err != nil { - panic(err) - } - return raw -} - -func TestKeyfileSigner(t *testing.T) { +func TestDecrypt(t *testing.T) { var testCases = map[string]struct { - path string - keygrip []byte - protected bool + keyPath string + input []string + expect []string }{ - "unprotected key": { - path: "testdata/private/bar@example.com.gpg", - keygrip: hexMustDecode("23F4477DF0F0C0963F8C4DFDEA8911CE65CC7CE3"), - }, - "protected key": { - path: "testdata/private/bar-protected@example.com.gpg", - keygrip: hexMustDecode("75B7C5A35213E71BA282F64317DDB90EC5C3FEE0"), - protected: true, + // test data is taken from a successful decrypt by gpg-agent + "decrypt file": { + keyPath: "testdata/private/foo@example.com.priv.key", + input: []string{ + "RESET\n", + "OPTION ttyname=/dev/pts/1\n", + "OPTION ttytype=screen\n", + "OPTION lc-ctype=C.UTF-8\n", + "OPTION lc-messages=C\n", + "GETINFO version\n", + "OPTION allow-pinentry-notify\n", + "OPTION agent-awareness=2.1.0\n", + "HAVEKEY FC0F9A401ADDB33C0F7225CCA83BFC14E7FEBC7D\n", + "HAVEKEY FC0F9A401ADDB33C0F7225CCA83BFC14E7FEBC7D\n", + "HAVEKEY FC0F9A401ADDB33C0F7225CCA83BFC14E7FEBC7D\n", + "RESET\n", + "SETKEY FC0F9A401ADDB33C0F7225CCA83BFC14E7FEBC7D\n", + "SETKEYDESC Please+enter+the+passphrase+to+unlock+the+OpenPGP+secret+key:%0A%22foo@example.com%22%0A3072-bit+RSA+key,+ID+8D0381C18D1E7CA6,%0Acreated+2021-08-04.%0A\n", + "PKDECRYPT\n", + "\x44\x20\x28\x37\x3a\x65\x6e\x63\x2d\x76\x61\x6c\x28\x33\x3a\x72\x73\x61\x28\x31\x3a\x61\x33\x38\x34\x3a\x59\xd1\x22\xac\x32\xf2\x15\xc7\xc6\xd8\x9c\xfa\xec\xf7\xd4\x71\x4f\x6f\xa7\x65\xf7\x7c\x38\x16\xff\x91\x7e\x7f\xb5\xc7\x6b\xb6\xf4\xcc\x24\x8b\xd8\x8e\x44\x25\x30\x44\xab\xf7\x79\x12\x8f\xe3\x06\x89\x7c\x2a\x31\xc3\x25\x30\x44\x46\xdf\xb5\x67\xde\x20\xc8\xce\xad\x72\x14\x5a\x2e\x0e\xfd\x25\x32\x35\x42\x25\x30\x41\x5d\x41\x3c\xb4\x75\xb3\xf0\x58\xd2\xd5\xe7\x2d\x1f\x12\xbc\x29\x59\x4a\xe1\x16\x16\xdf\x5a\x9a\x63\x48\xec\x00\x2f\x68\xa6\x82\x32\x70\x36\xbc\x4c\xf1\x0b\x69\x60\x06\xbd\x04\x37\xc1\x2c\x34\x8f\x13\xd8\x23\xbf\x86\x8c\xcd\x6c\xfa\xb1\xfa\x59\x28\x46\xcd\x55\x27\xa9\x80\x67\xd2\x7d\x63\xf5\xe6\x15\x14\x00\x97\x36\x70\x37\xde\xd9\x49\xa6\xbd\x4d\x44\x48\x69\x28\x25\x32\x35\xf4\x06\xeb\xbf\x89\x39\xbb\xb9\x0f\x8e\x92\x5a\x57\x15\xdc\x85\x87\x39\xae\x3d\xeb\x5c\x02\x7c\x08\xcc\x31\x0e\x55\x4d\x3e\xda\xb4\xba\x42\xce\x9a\xa5\x8d\xec\x4b\x45\x8c\x3a\xa2\x92\x70\xbe\x30\x48\x86\xae\x52\x2f\x83\x00\xba\x99\xcf\xdd\x8d\x69\x23\x8b\x25\x30\x41\x3b\x39\x7b\xa0\xc4\x81\x65\x32\xed\xa9\x37\x23\x12\xcb\x8d\xe9\xeb\xa6\x6e\x05\x03\x3f\x5f\x9d\x72\x29\xe0\x27\x17\x2a\x23\x34\xad\x83\xb2\xbc\x5e\x0e\x8e\x0e\xe5\xfb\xbd\xd6\x25\x30\x41\x63\x7e\x9a\x12\x15\x14\x8b\x98\x56\x0c\x2e\x50\xe3\xbb\xb4\x19\x7b\x1b\x6a\xd8\xdc\xa8\xbe\x8b\x38\xa8\x09\x07\xeb\x00\x60\x66\xf0\xd1\xb8\xe2\x37\x7e\x7f\xa4\x78\x62\xcb\xb6\xcb\x8c\xad\x73\x90\xcd\x4b\xb7\xb4\xf2\xb1\x80\x38\x23\x6f\x11\x11\xe4\x83\x6d\x93\x4f\x22\x26\xff\x60\xda\xdb\x85\x1b\x25\x30\x44\xa4\x3c\x26\xd9\x09\x86\xd9\xa3\x5f\x7c\xb4\xb5\xf5\x6a\x3d\xbe\x96\x25\x30\x41\x49\xbc\x92\x84\x02\xac\x0c\x30\x17\x9f\xb2\xd2\x11\x93\xfa\x1d\x37\x9c\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", + "D 2.2.27\n", + "OK\n", + "OK\n", + "OK\n", + "OK\n", + "OK\n", + "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\x33\x38\x33\x3a\x02\xfd\x56\x90\x50\xc0\x73\xcf\x96\x6a\x12\xfb\xc7\x25\x30\x44\xa2\xc6\x0f\x4c\x3b\xd4\x0f\x2a\x89\xff\x66\x3f\x28\xe6\xd1\x39\x17\x78\x87\x25\x32\x35\x32\x0c\x9d\x2d\x73\xe3\xab\x79\xe5\x03\xc3\x78\x88\x5e\x11\x98\x4b\x44\x42\xd1\xfc\x75\xe4\xfb\xbf\x2f\x9f\x79\x3a\xf1\xe7\xa6\xe3\x23\xea\xcf\xed\x1f\x29\x77\x67\x50\x42\xba\xe9\x98\x78\x30\x07\x44\x73\x9c\x15\x16\xd3\x7a\x9a\xe3\xe9\x36\xf2\x8a\x29\xf4\x3d\xb0\xa5\x18\xf2\x45\xf2\x33\xd4\x25\x30\x41\xb2\xe5\x18\x1b\xad\x55\xec\x8d\x16\x66\xce\xf9\xe5\x3d\xcd\x21\x6e\x57\xd0\x61\xf1\xb5\xc9\x16\x40\x06\x59\x64\xaa\x15\xcf\x01\xf7\xd2\x4c\x21\x3e\xd7\xe4\xeb\xbe\xf1\x8f\xb9\x50\xef\x14\x39\xb6\x9c\x12\xac\x8a\x1e\x1c\xe6\x0e\x45\xa8\x81\x4f\xbf\xc4\x9d\xb4\xb1\x50\x28\xbb\x14\x7b\xb3\xbb\xd9\x37\x38\xb3\x11\x43\xbc\xab\x32\xf2\x74\x67\xf3\x36\xb8\x11\x5f\x97\x7e\x91\x42\x6c\xee\x23\xe4\x81\x8b\xf8\x5a\xd7\x18\x27\x03\x6f\xa6\xff\xa2\x4b\x54\x18\x20\x74\x12\x21\x5c\x7a\x5e\x26\x25\x30\x41\xc6\xd3\x58\x94\x45\x3b\x90\x63\x7f\xf7\x9a\xb3\x30\x9d\x0e\xfe\xa7\xa9\xb5\xff\x92\x38\x15\x8b\x13\x46\x48\xd8\x9e\xca\xc4\xc2\xae\x65\x4d\xbb\xc1\xe5\x36\xf0\x56\x27\x96\x2b\x45\x4d\xc4\xed\xe5\x6f\x0e\x2b\x2f\x52\x47\x7f\x60\x09\x27\x0b\x30\xcb\x14\x65\x4e\xd2\xff\x9b\xdf\xd9\xb9\x0b\x7e\x07\x29\xba\x78\x47\x8e\x9d\x4a\x37\x0c\xee\x02\xb3\x65\xd7\x15\xba\xbb\xeb\x4b\xbd\xed\xd0\xcf\xae\x90\x31\x8a\x2d\x47\xfa\xc6\x1a\xac\xee\xf5\x82\x77\x28\x46\xce\x8a\x50\xc6\x00\x09\x9e\xf9\xb9\x35\x26\xbb\x2d\xcb\x9b\x60\x8d\x2e\xd3\x04\x95\xc7\xf5\x64\x97\xe6\x90\xf4\x7a\xb0\x50\xf4\x96\x99\x67\x36\xe6\x2f\x11\xf0\x29\x00\x0a", + "OK\n", + }, }, } for name, tc := range testCases { t.Run(name, func(tt *testing.T) { ctrl := gomock.NewController(tt) defer ctrl.Finish() - var mockPES = mock.NewMockPINEntryService(ctrl) - if tc.protected { - mockPES.EXPECT().GetPGPPassphrase(gomock.Any()).Return([]byte("trustno1"), nil) - } + // no securityKeys available + mockPES := mock.NewMockPINEntryService(ctrl) log, err := zap.NewDevelopment() if err != nil { tt.Fatal(err) } - g := server.NewGPG(nil, mockPES, log, tc.path) - if _, err := assuan.KeyfileSigner(g, tc.keygrip); err != nil { - tt.Fatalf("couldn't find keygrip: %v", err) + keyfileService, err := gpg.NewKeyfileService(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) + // 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(); 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") + } } }) } diff --git a/internal/assuan/event_enumer.go b/internal/assuan/event_enumer.go index 50002e3..63e758f 100644 --- a/internal/assuan/event_enumer.go +++ b/internal/assuan/event_enumer.go @@ -7,11 +7,11 @@ import ( "strings" ) -const _EventName = "INVALIDEVENTCONNECTRESETOPTIONGETINFOHAVEKEYKEYINFOSIGKEYSETKEYDESCSETHASHPKSIGN" +const _EventName = "INVALIDEVENTCONNECTRESETOPTIONGETINFOHAVEKEYKEYINFOSIGKEYSETKEYDESCSETHASHPKSIGNSETKEYPKDECRYPT" -var _EventIndex = [...]uint8{0, 12, 19, 24, 30, 37, 44, 51, 57, 67, 74, 80} +var _EventIndex = [...]uint8{0, 12, 19, 24, 30, 37, 44, 51, 57, 67, 74, 80, 86, 95} -const _EventLowerName = "invalideventconnectresetoptiongetinfohavekeykeyinfosigkeysetkeydescsethashpksign" +const _EventLowerName = "invalideventconnectresetoptiongetinfohavekeykeyinfosigkeysetkeydescsethashpksignsetkeypkdecrypt" func (i Event) String() string { if i < 0 || i >= Event(len(_EventIndex)-1) { @@ -35,9 +35,11 @@ func _EventNoOp() { _ = x[setkeydesc-(8)] _ = x[sethash-(9)] _ = x[pksign-(10)] + _ = x[setkey-(11)] + _ = x[pkdecrypt-(12)] } -var _EventValues = []Event{invalidEvent, connect, reset, option, getinfo, havekey, keyinfo, sigkey, setkeydesc, sethash, pksign} +var _EventValues = []Event{invalidEvent, connect, reset, option, getinfo, havekey, keyinfo, sigkey, setkeydesc, sethash, pksign, setkey, pkdecrypt} var _EventNameToValueMap = map[string]Event{ _EventName[0:12]: invalidEvent, @@ -62,6 +64,10 @@ var _EventNameToValueMap = map[string]Event{ _EventLowerName[67:74]: sethash, _EventName[74:80]: pksign, _EventLowerName[74:80]: pksign, + _EventName[80:86]: setkey, + _EventLowerName[80:86]: setkey, + _EventName[86:95]: pkdecrypt, + _EventLowerName[86:95]: pkdecrypt, } var _EventNames = []string{ @@ -76,6 +82,8 @@ var _EventNames = []string{ _EventName[57:67], _EventName[67:74], _EventName[74:80], + _EventName[80:86], + _EventName[86:95], } // EventString retrieves an enum value from the enum constants string name. diff --git a/internal/assuan/fsm.go b/internal/assuan/fsm.go index 48f3c27..e66fd1f 100644 --- a/internal/assuan/fsm.go +++ b/internal/assuan/fsm.go @@ -1,8 +1,8 @@ package assuan import ( + "bufio" "crypto" - "crypto/rsa" "sync" "github.com/smlx/fsm" @@ -26,6 +26,8 @@ const ( setkeydesc sethash pksign + setkey + pkdecrypt ) //go:generate enumer -type=State -text -transform upper @@ -35,28 +37,31 @@ type State fsm.Event // Enumeration of all possible states in the assuan FSM. // connected is the initial state when the client connects. -// keyIsSet indicates that the client has selected a key. +// signingKeyIsSet indicates that the client has selected a key. // hashIsSet indicates that the client has selected a hash (and key). const ( invalidState State = iota ready connected - keyIsSet + signingKeyIsSet hashIsSet + decryptingKeyIsSet + waitingForCiphertext ) // Assuan is the Assuan protocol FSM. type Assuan struct { fsm.Machine mu sync.Mutex + // buffered IO for linewise reading + reader *bufio.Reader // data is passed during Occur() data [][]byte // remaining fields store Assuan internal state - signingPrivKey crypto.Signer - hashAlgo crypto.Hash - hash []byte - // fallback keys - fallbackRSA []rsa.PrivateKey + signer crypto.Signer + decrypter crypto.Decrypter + hashAlgo crypto.Hash + hash []byte } // Occur handles an event occurence. @@ -72,50 +77,57 @@ var assuanTransitions = []fsm.Transition{ Src: fsm.State(ready), Event: fsm.Event(connect), Dst: fsm.State(connected), - }, - { + }, { Src: fsm.State(connected), Event: fsm.Event(reset), Dst: fsm.State(connected), - }, - { + }, { Src: fsm.State(connected), Event: fsm.Event(option), Dst: fsm.State(connected), - }, - { + }, { Src: fsm.State(connected), Event: fsm.Event(getinfo), Dst: fsm.State(connected), - }, - { + }, { Src: fsm.State(connected), Event: fsm.Event(havekey), Dst: fsm.State(connected), - }, - { + }, { Src: fsm.State(connected), Event: fsm.Event(keyinfo), Dst: fsm.State(connected), }, + // signing transitions { Src: fsm.State(connected), Event: fsm.Event(sigkey), - Dst: fsm.State(keyIsSet), - }, - { - Src: fsm.State(keyIsSet), + Dst: fsm.State(signingKeyIsSet), + }, { + Src: fsm.State(signingKeyIsSet), Event: fsm.Event(setkeydesc), - Dst: fsm.State(keyIsSet), - }, - { - Src: fsm.State(keyIsSet), + Dst: fsm.State(signingKeyIsSet), + }, { + Src: fsm.State(signingKeyIsSet), Event: fsm.Event(sethash), Dst: fsm.State(hashIsSet), - }, - { + }, { Src: fsm.State(hashIsSet), Event: fsm.Event(pksign), Dst: fsm.State(hashIsSet), }, + // decrypting transitions + { + Src: fsm.State(connected), + Event: fsm.Event(setkey), + Dst: fsm.State(decryptingKeyIsSet), + }, { + Src: fsm.State(decryptingKeyIsSet), + Event: fsm.Event(setkeydesc), + Dst: fsm.State(decryptingKeyIsSet), + }, { + Src: fsm.State(decryptingKeyIsSet), + Event: fsm.Event(pkdecrypt), + Dst: fsm.State(waitingForCiphertext), + }, } diff --git a/internal/assuan/helper_test.go b/internal/assuan/helper_test.go deleted file mode 100644 index 1a0050d..0000000 --- a/internal/assuan/helper_test.go +++ /dev/null @@ -1,8 +0,0 @@ -package assuan - -import "crypto" - -// KeyfileSigner wraps keyfileSigner() for testing purposes. -func KeyfileSigner(g GPGService, keygrip []byte) (crypto.Signer, error) { - return keyfileSigner(g, keygrip) -} diff --git a/internal/assuan/run.go b/internal/assuan/run.go index d7294d4..dbdfe65 100644 --- a/internal/assuan/run.go +++ b/internal/assuan/run.go @@ -1,23 +1,20 @@ package assuan import ( - "bufio" "bytes" "fmt" "io" ) // Run the event machine loop -func (a *Assuan) Run(conn io.Reader) error { +func (a *Assuan) Run() error { // register connection if err := a.Occur(connect); err != nil { return fmt.Errorf("error handling connect: %w", err) } - // parse incoming messages to events - r := bufio.NewReader(conn) var e Event for { - line, err := r.ReadBytes(byte('\n')) + line, err := a.reader.ReadBytes(byte('\n')) if err != nil { if err == io.EOF { return nil // connection closed diff --git a/internal/assuan/sign.go b/internal/assuan/sign.go new file mode 100644 index 0000000..a6c70b7 --- /dev/null +++ b/internal/assuan/sign.go @@ -0,0 +1,44 @@ +package assuan + +import ( + "crypto/rand" + "fmt" + "math/big" + + "github.com/smlx/piv-agent/internal/notify" + "golang.org/x/crypto/cryptobyte" + "golang.org/x/crypto/cryptobyte/asn1" +) + +// sign performs signing of the specified "hash" data, using the specified +// "hashAlgo" hash algorithm. It then encodes the response into an s-expression +// and returns it as a byte slice. +// +// This function's complexity is due to the fact that while Sign() returns the +// r and s components of the signature ASN1-encoded, gpg expects them to be +// separately s-exp encoded. So we have to decode the ASN1 signature, extract +// the params, and re-encode them into the s-exp. Ugh. +func (a *Assuan) sign() ([]byte, error) { + cancel := notify.Touch(nil) + defer cancel() + signature, err := a.signer.Sign(rand.Reader, a.hash, a.hashAlgo) + if err != nil { + return nil, fmt.Errorf("couldn't sign: %v", err) + } + var sig cryptobyte.String = signature + var b []byte + if !sig.ReadASN1Bytes(&b, asn1.SEQUENCE) { + return nil, fmt.Errorf("couldn't read asn1.SEQUENCE") + } + var rawInts cryptobyte.String = b + var r, s big.Int + if !rawInts.ReadASN1Integer(&r) { + return nil, fmt.Errorf("couldn't read r as asn1.Integer") + } + if !rawInts.ReadASN1Integer(&s) { + return nil, fmt.Errorf("couldn't read s as asn1.Integer") + } + // encode the params (r, s) into s-exp + return []byte(fmt.Sprintf(`D (7:sig-val(5:ecdsa(1:r32#%X#)(1:s32#%X#)))`, + r.Bytes(), s.Bytes())), nil +} diff --git a/internal/assuan/state_enumer.go b/internal/assuan/state_enumer.go index 42dd5c3..16c7e4c 100644 --- a/internal/assuan/state_enumer.go +++ b/internal/assuan/state_enumer.go @@ -7,11 +7,11 @@ import ( "strings" ) -const _StateName = "INVALIDSTATEREADYCONNECTEDKEYISSETHASHISSET" +const _StateName = "INVALIDSTATEREADYCONNECTEDSIGNINGKEYISSETHASHISSETDECRYPTINGKEYISSETWAITINGFORCIPHERTEXT" -var _StateIndex = [...]uint8{0, 12, 17, 26, 34, 43} +var _StateIndex = [...]uint8{0, 12, 17, 26, 41, 50, 68, 88} -const _StateLowerName = "invalidstatereadyconnectedkeyissethashisset" +const _StateLowerName = "invalidstatereadyconnectedsigningkeyissethashissetdecryptingkeyissetwaitingforciphertext" func (i State) String() string { if i < 0 || i >= State(len(_StateIndex)-1) { @@ -27,11 +27,13 @@ func _StateNoOp() { _ = x[invalidState-(0)] _ = x[ready-(1)] _ = x[connected-(2)] - _ = x[keyIsSet-(3)] + _ = x[signingKeyIsSet-(3)] _ = x[hashIsSet-(4)] + _ = x[decryptingKeyIsSet-(5)] + _ = x[waitingForCiphertext-(6)] } -var _StateValues = []State{invalidState, ready, connected, keyIsSet, hashIsSet} +var _StateValues = []State{invalidState, ready, connected, signingKeyIsSet, hashIsSet, decryptingKeyIsSet, waitingForCiphertext} var _StateNameToValueMap = map[string]State{ _StateName[0:12]: invalidState, @@ -40,18 +42,24 @@ var _StateNameToValueMap = map[string]State{ _StateLowerName[12:17]: ready, _StateName[17:26]: connected, _StateLowerName[17:26]: connected, - _StateName[26:34]: keyIsSet, - _StateLowerName[26:34]: keyIsSet, - _StateName[34:43]: hashIsSet, - _StateLowerName[34:43]: hashIsSet, + _StateName[26:41]: signingKeyIsSet, + _StateLowerName[26:41]: signingKeyIsSet, + _StateName[41:50]: hashIsSet, + _StateLowerName[41:50]: hashIsSet, + _StateName[50:68]: decryptingKeyIsSet, + _StateLowerName[50:68]: decryptingKeyIsSet, + _StateName[68:88]: waitingForCiphertext, + _StateLowerName[68:88]: waitingForCiphertext, } var _StateNames = []string{ _StateName[0:12], _StateName[12:17], _StateName[17:26], - _StateName[26:34], - _StateName[34:43], + _StateName[26:41], + _StateName[41:50], + _StateName[50:68], + _StateName[68:88], } // StateString retrieves an enum value from the enum constants string name. diff --git a/internal/assuan/testdata/private/bar@example.com.gpg b/internal/assuan/testdata/private/bar@example.com.gpg deleted file mode 100644 index 8f2d2384293f09f64049f63b82767543d4675164..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 1859 zcmV-J2fX-|1y}@O03OQ$3;>%cIwki>^ilUE0p~`n$<8mIs4Cb!HoCiH1FEgg#@=+5f0tKAc;r~TueK?cd z{OUHMhy-g9!z8yX2@e{jU;d#dyaP_bh&ufVT5Tkr7dTg#PUyHC4l_lwfzS-JJ&`gf z*OlQYwn6J2a5kDc`8{BfKMepHJ(M8R$W`vn7g8w+k}{k74d(7tY~K{#3L+98<*+FgmiI}xsinXyd9x$Z;GrN>I&vNkR~eo! zS=N+^) zqXl;11CZ^GNh81U~;5MB2^^*+BXedUUro^4BnERxbb%0RRC23;sUn{8pf8 zFVcJNSyMP@o_Z$z2Iw(7V423a z*-vqR^;H93#qwq`uI$vAD|p6j{n#lEdsqL8XTA@WC7MDWSm4wDPScLWuq!rPfYOyY zn!;}GhTGg9PS=`rOq?X|XjQ}c7mdbBg!A6!7tj2N75Ga18P(mkjJowc41DN&Gs=WUkvwcHSfVw&}f) zZ|KWg=hSv=OwWlsmMYjh8>OKw&~TZ9sqn9OgVH}3m7h4E7IhD7I_a?nY!BC6(_Ixt z|E0<78QsLx6xPYqQXvKa!0>zUm?Hj30h@M-*axDZ5%Fr+{rzbAyLbVUdQbgL4Nh5_ zOYGV5hZ0gI{$*CGW!a1_rJ_*JbiqG^>QzyVu(2`WFgCxsE_|dVnweDCi=)#J^x4nm z;FxRW4UKmTWmTa`yUdzViuAwK&q_AlQEmCmtpZp-BcLuA$ej$B9!77>l9OoS%)UK* zk4ri7Hq9&XbEJ3Gn$u$v-02YT_xMn|Z{&%gUZOYT35IeDb=qC^7EhpN0`P`s=l>L) z1^~_9#y}&n6eBT29MUJ9cfCt6r);Omo$!r{tD#QM!PKKfKS|OxO9XEs1c}&~%_Fj@ zQgn}D_NnuWvvXGoINu%?`gJAbjG$E4R0sS|=DGt`|0#ZcF&=n?1w6?*FH>*~X& zbSXt$B<(;ZT}6;1TJiX;FmSxH-!umGec>h#%mfA`CF96ZP>oWwA&5&(VWFqQAn8q) zQ=i-=-s!tg z$kd7p{&k>)kQG*A7-|8b9@8fZZH$F6`!LS)T4PCZv{n5vrzkWO&tM-y8gr4Uyc0++!7hu< zA9s1v4Fb7T_Ux!3PE*LlPIOOSX$RwA=m@kAVqtPXWq4t2aBO8RV{dJV0n`K&0SW*< z79j)ynJYub!|#4T=2S+=SnAS41yRxk0$~6i%K{q@1qlPfX8;8Y2?z%Q1{Dek2nzxP z76JnS0v-VZ7k~f?2@uFw>e536QPP8&3;>liQg6bEbRY^uIQiDqx}i%GMiMeDmWIJR z>B@SyI5kWFonYBAnr{KB?a*Fk27@$=npc;lWU4wH1-5h((PA^}EPd@SKfCg=$<0ju z!MTBs{0z2ju`viI1o+=$%YwWARRwBhGlKZWUJ>>fl&_$1200K2PkSR(it;r%y!TBg z?CO*(iXgcVT_BRqvx+1sGGDIr{tXC{U;{Q(630r}R`C>1VtR=!P&!7$x%MC4hgyjN zE~k;*j8>F`Q`~2I;GsKU4Ve|GAW&*Oe^PU2!mVZcqmQ xGLdVoqgm-v^xc>cr^TEWRH}lT*Vqa3EY2{LcGd+0x)bj%zNEgNrAIoQREFz&cOw7* diff --git a/internal/assuan/testdata/private/foo@example.com.gpg b/internal/assuan/testdata/private/foo@example.com.gpg new file mode 100644 index 0000000000000000000000000000000000000000..41d78dd581e38f8ed4636c1dc639b4b2c11e9a5a GIT binary patch literal 1859 zcmV-J2fX-|1y}@O3aNns3;@C?0iFgK!cuMBi|kwCUlbw-ms^#Au`zGT|2rp$J4z{7 zv*Y(*nh~MSgUc`&aRN8;8Ig7vyk0QRJB%_;a8`-RtV(DTJ3ZiJL*2yc^q6PM>bMsL z?gGHPFE%jYX*_8@!PF>xput|d7|$pu%)|c?nf8ny=p-^9Djin!IO`Uee#v>W*~sw{ z@*VUs=#J7;a@>5CmBzisO`m`}x+tO3h;~Yz)Y{}&U9qsu0zM{SI{Lp1%B}_CV>-xT zZLYjjDCzzd0BMceXj+CLp@)QQYRZYJdv$rqAlqep>pR5a%npHUma0YC12lVgMCoPU zGsStwpOJ6>K^zgW1AW`%FAzZStqys~Wggpw6WVJ@1q`?ohK<`lhU~Y6U*EHdD!M{R z-K;+;*m3^Y&yx=AeuQ%9{!itENf92su_J2V4=j)K>~e^~Gfw)sU5U#n6=TOE1`96U zX$7NJ0^~@0yiJhhv+U`@Ifoc00I82VrqW!Cq} z1`cWC5JOV$({6Xh(<~0UnbL>g`3u2^2-{`UOEd#f3x}L(h#SHNH%rYGzE=4l?Khh< zge%U*9J1i)mk6vz`ko5hb;NLU_lSK)qXqWWZ3ZtF#eG%d99NM!LGy0nB>hh1o;Pfu zj6)i;73 zOp3$}a0|A5C*TM;9ZfWmncC`0R+zb&)?J~TYs@XQU z|4+VcBnuTpz{9ZRbBuO!6bIYPC>IWpe|xFE{Q2+-zMv*fEXlMH?$TDagjNtUWlF8^&ed##l>B(K9Wo?Ho}ECI@TXm`Euc3Rcf)?5LAGfBe9Mn5LW%3w4`EN) zv>hN?L(VP#)t%54d(Fse&u7EDve3;?N$lBm`6KE4ArMJY1e2#pP75fp@Vby!5# zWbC+PPDRS0KpkYlua3*n-ii&()W*tB5S8_(g*b=R-dwLU*Ac$t#5;uLT(EPFVrp+;^X zM)dKj6Lp~S3%~P9?Fsdlg`=Qva(^e*Tx;1LdQutoaR2Z?Vs_nWYioERyuV4AJng#J zxFohdAtK-Fa#PRHSjl9rrJAY;Q-j|13ybxOUXs34cA#Yx_ph3wrZswe`O}@hL6LiC zSEO#S%S(hNX~o!yZ`hyxq1iF_amKNev-G?Gl{6)99`fpt5*GhGO(!h*ZxY(*W$XAq zWEwP#r?xO<5MXK*0Ua}Vo*|e)DLEy9WTR}SUgemte2|iEdns8%kN7J;hNzQ{(T#}( xZwZ;}0oiXdDqH+Kk@(Uf1fv!;vKz2a*RChqvC(A?cpvgz;x77NM8JaD4pd;Ee9Qm< literal 0 HcmV?d00001 diff --git a/internal/assuan/testdata/private/foo@example.com.priv.key b/internal/assuan/testdata/private/foo@example.com.priv.key new file mode 100644 index 0000000000000000000000000000000000000000..6e9504489742e4c9bc76efa3a9fa4942ca6771ed GIT binary patch literal 1857 zcmV-H2fp}~1y}@O3YRef3;@X=^IKx9ZeD-xWA%#rdQz+5u_G%=Bcg>X{dHP0b-h6A@7i8H~#n3-W&X91`gL$X=N>ZwUaEs|5!x93(T%Q(=mHtS?h z8s=s01PY?Fm7vLu6!w3xurs}3Xocfc65PdD(oyLuo}5+r>#Ax7@a2{u~{3dPd zVNBpR@-&Glbd5#_Im?e)ojmi}g%dlRj)9{du_sdQ{5mJ9gOSvVkozMtS+BvA;4Xpu z82((I$DocSnzFOQ?KUD6`Cj=qHb-C1m27g$U@FX12-1oYDO&&$0RRC23;sM`S7jKl z3g}t=oXXLk;9Z@QzhM8j9}q)oYlunDqg_9(CAU0@dRq}(e@ZB&N7 zICvizVI&%Mf1x|9DlrCkK!rotnoK9lFS=a7tpXg9I)Qt1DDiN=1BSs&HV)2F;Y$vgRIz=M#1EzF33~epLCBzz zzc3(!mxMF`PG`DIxd9-;`wf~bE8{=)99$yv8mb37!p0zo9oh@SH zhB%bw_%NZHtiLY}oBC5Sx~XjBm_XhBiM<;IN&FPQ&n zI>JtYA{H+SoOtc#u+kA>ka+d%S<23vG`?$e6iF4VaAhRtcrp2NH!AOZhL2WnX#CoB z1_124VU_3ke^mrI(%%PcM1m=9Fa5vn5$|`@Q8u3}vv1?Iki*v-RNYWs=-3k=}Ej_kcgThb7*F=@`{H_Af~l*%Y@MCma4_&sYY?A14w_o#->H82mbAV~V>$JN%&&u$ z#7>8S2}ru1V&)_IQ)ugfDjGSvnXFwHV5rL)NAU3f?MR6WT8{<*l1jiS3wn6BYw)Fi zaH+-NV4>I`Whj*W)QW`M%is~Wh+rqrZ74>Vl6Hm>a7tk-;s z0#nmjN_D~kF5dqgAZlYg6+F_3-%rG&{gISqP%gywtXgr!%{OzI;3;1+6R;>lGl(w* zw%&igzA#Rp28#o1RfY0s&tW6}d&|_urA=NGFByNdK7vSfiX^2RH5-lMb+W^ZpmWq4t2aBO8RV{dJV0n!8$0SEv* z79j)!BN@I_LW+M1>Ui;u1A)Pf9(<++0$~c5F#;P81qlPfX8;8Y2?z%Q1{Dek2nzxN z761Ys0Rk6*0162ZjRS$fjUIfaAfF5WU&)uhi~a#AY+@RUQ38FW$+_6WjA(pZd>#sa z`FNg?A`Nlqn(3@3Px@OdK-Tb@=sgVC%RMQ~P`cmeY@<2t!N2^Ut6h+4+H?TP2X$>r zg%&n_tyw}?ha`WZmr!@)eAo@L%Ac)#u@;5zT+4@Vi?VU1mp7dSV*j=p9r)?d{u?N55Yv=jnLYyo)axomf3nblAQT2<*Z<^! za4~V9Y0U~LQ0kaLDCt}gPiBd`2}*M(H2gm8M~kVt?m!zhMVdYD#HW!g!bEoP&IwmI^4DK24y7W!(N_noD|+` zG!_k8+<7@0ca-l*pdi$yd+Keb9nJSRXtb0GJ)6cGNb*ji`&6JcPCzp>sStk$L)E$B vFeN#kBA@Fr4 literal 0 HcmV?d00001 diff --git a/internal/gpg/key.go b/internal/gpg/key.go new file mode 100644 index 0000000..bfeabb1 --- /dev/null +++ b/internal/gpg/key.go @@ -0,0 +1,39 @@ +package gpg + +import ( + "crypto" + "crypto/rsa" + "fmt" + "io" + "math/big" +) + +// RSAKey represents a GPG loaded from a keyfile. +// It implements the crypto.Decrypter and crypto.Signer interfaces. +type RSAKey struct { + rsa *rsa.PrivateKey +} + +// Decrypt performs RSA decryption as per gpg-agent. +func (k *RSAKey) Decrypt(_ io.Reader, ciphertext []byte, + _ crypto.DecrypterOpts) ([]byte, error) { + c := new(big.Int) + c.SetBytes(ciphertext) + // libgcrypt does this, not sure if required + c.Rem(c, k.rsa.N) + // perform arithmetic manually + c.Exp(c, k.rsa.D, k.rsa.N) + return c.Bytes(), nil +} + +// Public implements the crypto.Decrypter interface. +func (k *RSAKey) Public() crypto.PublicKey { + return k.rsa.Public() +} + +// Sign performs RSA signing as per gpg-agent. +func (k *RSAKey) Sign(_ io.Reader, digest []byte, + _ crypto.SignerOpts) ([]byte, error) { + // TODO: implement this + return nil, fmt.Errorf("not implemented") +} diff --git a/internal/gpg/keyfile.go b/internal/gpg/keyfile.go new file mode 100644 index 0000000..8c496f5 --- /dev/null +++ b/internal/gpg/keyfile.go @@ -0,0 +1,81 @@ +package gpg + +import ( + "fmt" + "io" + "os" + "path" + + "golang.org/x/crypto/openpgp/errors" + "golang.org/x/crypto/openpgp/packet" +) + +// keyfilePrivateKeys reads the given path and returns any private keys found. +func keyfilePrivateKeys(p string) ([]*packet.PrivateKey, error) { + f, err := os.Open(p) + if err != nil { + return nil, fmt.Errorf("couldn't open path %s: %v", p, err) + } + fileInfo, err := f.Stat() + if err != nil { + return nil, fmt.Errorf("couldn't stat path %s: %v", p, err) + + } + switch { + case fileInfo.Mode().IsRegular(): + return keysFromFile(f) + case fileInfo.IsDir(): + // enumerate files in directory + dirents, err := f.ReadDir(0) + if err != nil { + return nil, fmt.Errorf("couldn't read directory") + } + // get any private keys from each file + var privKeys []*packet.PrivateKey + for _, dirent := range dirents { + direntInfo, err := dirent.Info() + if err != nil { + return nil, fmt.Errorf("couldn't stat directory entry") + } + // ignore subdirectories + if direntInfo.Mode().IsRegular() { + subPath := path.Join(p, dirent.Name()) + ff, err := os.Open(subPath) + if err != nil { + return nil, fmt.Errorf("couldn't open path %s: %v", subPath, err) + } + subPrivKeys, err := keysFromFile(ff) + if err != nil { + return nil, + fmt.Errorf("couldn't get keys from file %s: %v", subPath, err) + } + privKeys = append(privKeys, subPrivKeys...) + } + } + return privKeys, nil + default: + return nil, fmt.Errorf("invalid file type for path to keyfiles") + } +} + +// keysFromFile read a file and return any private keys found +func keysFromFile(f *os.File) ([]*packet.PrivateKey, error) { + var err error + var pkt packet.Packet + var privKeys []*packet.PrivateKey + reader := packet.NewReader(f) + for pkt, err = reader.Next(); err != io.EOF; pkt, err = reader.Next() { + if _, ok := err.(errors.UnsupportedError); ok { + continue // gpg writes some non-standard cruft + } + if err != nil { + return nil, fmt.Errorf("couldn't get next packet: %v", err) + } + k, ok := pkt.(*packet.PrivateKey) + if !ok { + continue + } + privKeys = append(privKeys, k) + } + return privKeys, nil +} diff --git a/internal/gpg/keygrip.go b/internal/gpg/keygrip.go index 4f4800e..ab2fc7f 100644 --- a/internal/gpg/keygrip.go +++ b/internal/gpg/keygrip.go @@ -87,8 +87,8 @@ func compute(parts []part) ([]byte, error) { return s[:], nil } -// KeygripRSA calculates a keygrip for an RSA public key. -func KeygripRSA(pubKey *rsa.PublicKey) []byte { +// keygripRSA calculates a keygrip for an RSA public key. +func keygripRSA(pubKey *rsa.PublicKey) []byte { keygrip := sha1.New() keygrip.Write([]byte{0}) keygrip.Write(pubKey.N.Bytes()) diff --git a/internal/gpg/keyservice.go b/internal/gpg/keyservice.go new file mode 100644 index 0000000..f1a8581 --- /dev/null +++ b/internal/gpg/keyservice.go @@ -0,0 +1,131 @@ +package gpg + +//go:generate mockgen -source=keyservice.go -destination=../mock/mock_keyservice.go -package=mock + +import ( + "bytes" + "crypto" + "crypto/rsa" + "fmt" + + "go.uber.org/zap" + "golang.org/x/crypto/openpgp/packet" +) + +// PINEntryService provides an interface to talk to a pinentry program. +type PINEntryService interface { + GetPGPPassphrase(string) ([]byte, error) +} + +// KeyfileService implements an interface for getting cryptographic keys from +// keyfiles on disk. +type KeyfileService struct { + // cache passphrases used for decryption + passphrases [][]byte + privKeys []*packet.PrivateKey + log *zap.Logger + pinentry PINEntryService +} + +// NewKeyfileService returns a keyservice initialised with keys found at path. +// Path can be a file or directory. +func NewKeyfileService(l *zap.Logger, pe PINEntryService, + path string) (*KeyfileService, error) { + p, err := keyfilePrivateKeys(path) + if err != nil { + return nil, err + } + return &KeyfileService{ + privKeys: p, + log: l, + pinentry: pe, + }, nil +} + +// Name returns the name of the keyservice. +func (g *KeyfileService) Name() string { + return "GPG Keyfile" +} + +// HaveKey takes a list of keygrips, and returns a boolean indicating if any of +// the given keygrips were found, the found keygrip, and an error, if any. +func (g *KeyfileService) HaveKey(keygrips [][]byte) (bool, []byte, error) { + for _, kg := range keygrips { + key, err := g.getKey(kg) + if err != nil { + return false, nil, err + } + if key != nil { + return true, kg, nil + } + } + return false, nil, nil +} + +// getKey returns a matching private RSA key if the keygrip matches. If a key +// is returned err will be nil. If no key is found, both values may be nil. +func (g *KeyfileService) getKey(keygrip []byte) (*rsa.PrivateKey, error) { + var pass []byte + var err error + for _, k := range g.privKeys { + pubKey, ok := k.PublicKey.PublicKey.(*rsa.PublicKey) + if !ok { + continue + } + if !bytes.Equal(keygrip, keygripRSA(pubKey)) { + continue + } + if k.Encrypted { + // try existing passphrases + for _, pass := range g.passphrases { + if err = k.Decrypt(pass); err == nil { + g.log.Debug("decrypted using cached passphrase", + zap.String("fingerprint", k.KeyIdString())) + break + } + } + } + if k.Encrypted { + // ask for a passphrase + pass, err = g.pinentry.GetPGPPassphrase( + fmt.Sprintf("%X %X %X %X", k.Fingerprint[:5], k.Fingerprint[5:10], + k.Fingerprint[10:15], k.Fingerprint[15:])) + if err != nil { + return nil, fmt.Errorf("couldn't get passphrase for key %s: %v", + k.KeyIdString(), err) + } + g.passphrases = append(g.passphrases, pass) + if err = k.Decrypt(pass); err != nil { + return nil, fmt.Errorf("couldn't decrypt key %s: %v", + k.KeyIdString(), err) + } + g.log.Debug("decrypted using passphrase", + zap.String("fingerprint", k.KeyIdString())) + } + privKey, ok := k.PrivateKey.(*rsa.PrivateKey) + if !ok { + return nil, fmt.Errorf("not an RSA key %s: %v", + k.KeyIdString(), err) + } + return privKey, nil + } + return nil, nil +} + +// GetSigner returns a crypto.Signer associated with the given keygrip. +func (g *KeyfileService) GetSigner(keygrip []byte) (crypto.Signer, error) { + rsaPrivKey, err := g.getKey(keygrip) + if err != nil { + return nil, fmt.Errorf("couldn't getKey: %v", err) + } + return &RSAKey{rsa: rsaPrivKey}, nil +} + +// GetDecrypter returns a crypto.Decrypter associated with the given keygrip. +func (g *KeyfileService) GetDecrypter(keygrip []byte) (crypto.Decrypter, error) { + rsaPrivKey, err := g.getKey(keygrip) + if err != nil { + return nil, fmt.Errorf("couldn't getKey: %v", err) + } + return &RSAKey{rsa: rsaPrivKey}, nil +} diff --git a/internal/gpg/keyservice_test.go b/internal/gpg/keyservice_test.go new file mode 100644 index 0000000..1762f2f --- /dev/null +++ b/internal/gpg/keyservice_test.go @@ -0,0 +1,59 @@ +package gpg_test + +import ( + "encoding/hex" + "testing" + + "github.com/golang/mock/gomock" + "github.com/smlx/piv-agent/internal/gpg" + "github.com/smlx/piv-agent/internal/mock" + "go.uber.org/zap" +) + +func hexMustDecode(s string) []byte { + raw, err := hex.DecodeString(s) + if err != nil { + panic(err) + } + return raw +} + +func TestGetSigner(t *testing.T) { + var testCases = map[string]struct { + path string + keygrip []byte + protected bool + }{ + "unprotected key": { + path: "testdata/private/bar@example.com.gpg", + keygrip: hexMustDecode("9128BB9362750577445FAAE9E737684EBB74FD6C"), + }, + "protected key": { + path: "testdata/private/bar-protected@example.com.gpg", + keygrip: hexMustDecode("75B7C5A35213E71BA282F64317DDB90EC5C3FEE0"), + protected: true, + }, + } + log, err := zap.NewDevelopment() + if err != nil { + t.Fatal(err) + } + for name, tc := range testCases { + t.Run(name, func(tt *testing.T) { + ctrl := gomock.NewController(tt) + defer ctrl.Finish() + var mockPES = mock.NewMockPINEntryService(ctrl) + if tc.protected { + mockPES.EXPECT().GetPGPPassphrase(gomock.Any()). + Return([]byte("trustno1"), nil) + } + ks, err := gpg.NewKeyfileService(log, mockPES, tc.path) + if err != nil { + tt.Fatal(err) + } + if _, err := ks.GetSigner(tc.keygrip); err != nil { + tt.Fatal(err) + } + }) + } +} diff --git a/internal/assuan/testdata/private/bar-protected@example.com.gpg b/internal/gpg/testdata/private/bar-protected@example.com.gpg similarity index 100% rename from internal/assuan/testdata/private/bar-protected@example.com.gpg rename to internal/gpg/testdata/private/bar-protected@example.com.gpg diff --git a/internal/gpg/testdata/private/bar@example.com.gpg b/internal/gpg/testdata/private/bar@example.com.gpg new file mode 100644 index 0000000000000000000000000000000000000000..26dd30d87ee5871600e968cc6bd36189c6e22bc9 GIT binary patch literal 1857 zcmV-H2fp}~1y}@O3d(u`3;^EZFNI%?lG6II#%QHi^|4$fnZ>)2QR9y;7Gm4FEW>6%O@GA>k!XHj9xDZay}~teXeW?z+JR zKkhU(p8NW)96UF{pWcsDp>k8r^Xx|%&w&JJ8lNigD$>+W8e#T>(TqUAYy5$`Q8Uo+ z;&++o*WFUl=F-0jVVIfD@xhzfMc3=b$Grtp7xZ~70rziip&jD%iOf-`^9pYbSH!#kL7_ca>j zQD{(;CNHk=?@CT}^BE=uJP%D&(F_HX~)O1funScYNF^%qU&^hL}3xDnRD+1sQqO4RMWFP;i4Z!xA2~Il*-|9>b$uLq? zSVFO|oxNHWr~Tx02_;aM08q7U{WK;qLZd%tNgn|cuDuRmkjitkue=1i@i8k6Yqyg|h#m5JQ63714ZEK<;|VN`d2vupT0o z%m)1qOfMd9-BN$S87P~cNlr=@lmGqg=3aej*3n63+$LR+7kBv)Uu3YD`(5c7Ko0`C^j*t(7g~?G zJGO}4JoIR#d}Uv;^a8!PvA5l-FkQQ?@r>HTZiAF!luhCpr@ii{Kw=`RxrZ;g!$;aBQGTtgRykAEBi z3HYEGqWU!V=q)21F&Zudq+N7uLPr+dm41tJVnL!PuG~DDe7tcJ`%$I`aLbfR3BZ`_ z1i<+*mJ3O{6A>g|Bh&wcNbCkS$wP0ypvqY}=pQOn(AGlLh*%ecUGSIR{X4z){l4$1 zn#Mc`Vz33P)Dd&vsz1L6FzII-_^`sP`ox|n6!#{-s!XSyf@=l;Zu8}YG9=gpG<$-_ zuH@p6<2@3;wpMZCm0S~2ak!gU*yzZ*4^d~|Pa`356Hb4uab^-6 z9I=(e2GssK%P31SFNfi`)V8b%A&})D$JsZ?6P=0)z&HfMfd~x3EdW#Nkcpe%J77%kTsw!`FRHqxF z#AHTOB~QWnnYVB=^T* Date: Sat, 7 Aug 2021 00:38:23 +0800 Subject: [PATCH 06/20] feat: add touch policy to synthesized openpgp key comment --- internal/securitykey/string.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/internal/securitykey/string.go b/internal/securitykey/string.go index 286b0d9..47c0d92 100644 --- a/internal/securitykey/string.go +++ b/internal/securitykey/string.go @@ -46,10 +46,6 @@ func (k *SecurityKey) StringsSSH() []string { // on the yubikey for slots with touch policies that require it. func (k *SecurityKey) synthesizeEntities(name, email string) ([]Entity, error) { now := time.Now() - uid := packet.NewUserId(name, "piv-agent synthesized user ID", email) - if uid == nil { - return nil, errors.InvalidArgumentError("invalid characters in user ID") - } var entities []Entity for _, signingKey := range k.SigningKeys() { cryptoPrivKey, err := k.PrivateKey(&signingKey) @@ -60,6 +56,12 @@ func (k *SecurityKey) synthesizeEntities(name, email string) ([]Entity, error) { if !ok { return nil, fmt.Errorf("private key is invalid type") } + comment := fmt.Sprintf("piv-agent synthesized; touch-policy %s", + touchStringMap[signingKey.SlotSpec.TouchPolicy]) + uid := packet.NewUserId(name, comment, email) + if uid == nil { + return nil, errors.InvalidArgumentError("invalid characters in user ID") + } ecdsaPubKey, ok := signingKey.Public.(*ecdsa.PublicKey) if !ok { // TODO: handle ed25519 keys From dc30888798f7b6223187245dd5ed566c8024f06e Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Sat, 7 Aug 2021 00:56:34 +0800 Subject: [PATCH 07/20] fix: fall back to keyfile when yubikey unplugged --- internal/pivservice/list.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/pivservice/list.go b/internal/pivservice/list.go index 9a80c91..f8f042a 100644 --- a/internal/pivservice/list.go +++ b/internal/pivservice/list.go @@ -47,7 +47,7 @@ func (p *PIVService) reloadSecurityKeys() error { p.securityKeys = append(p.securityKeys, sk) } if len(p.securityKeys) == 0 { - return fmt.Errorf("no valid security keys found") + p.log.Warn("no valid security keys found") } return nil } From 1b8b506f62697814b43617a7fe09533fe5209bca Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Sat, 7 Aug 2021 00:57:00 +0800 Subject: [PATCH 08/20] feat: implement RSA keyfile signing --- internal/assuan/assuan.go | 5 +- internal/assuan/assuan_test.go | 102 +++++++++++++++++++++++++++++- internal/assuan/event_enumer.go | 12 ++-- internal/assuan/fsm.go | 5 ++ internal/assuan/sign.go | 24 ++++++- internal/gpg/key.go | 13 ++-- internal/pivservice/pivservice.go | 2 +- 7 files changed, 147 insertions(+), 16 deletions(-) diff --git a/internal/assuan/assuan.go b/internal/assuan/assuan.go index eb54508..1c703a6 100644 --- a/internal/assuan/assuan.go +++ b/internal/assuan/assuan.go @@ -100,11 +100,14 @@ func New(rw io.ReadWriter, log *zap.Logger, ks ...KeyService) *Assuan { } if keyFound { _, err = io.WriteString(rw, - fmt.Sprintf("S KEYINFO %s D - - - P - - -\nOK\n", + fmt.Sprintf("S KEYINFO %s D - - - - - - -\nOK\n", strings.ToUpper(hex.EncodeToString(keygrip)))) } else { _, err = io.WriteString(rw, "No_Secret_Key\n") } + case scd: + // ignore scdaemon requests + _, err = io.WriteString(rw, "ERR 100696144 No such device \n") default: return fmt.Errorf("unknown event: %v", e) } diff --git a/internal/assuan/assuan_test.go b/internal/assuan/assuan_test.go index ad319c9..1b1f2bc 100644 --- a/internal/assuan/assuan_test.go +++ b/internal/assuan/assuan_test.go @@ -184,7 +184,7 @@ func TestKeyinfo(t *testing.T) { expect: []string{ "OK Pleased to meet you, process 123456789\n", "OK\n", - "S KEYINFO 38F053358EFD6C923D08EE4FC4CEB208CBCDF73C D - - - P - - -\n", + "S KEYINFO 38F053358EFD6C923D08EE4FC4CEB208CBCDF73C D - - - - - - -\n", "OK\n", }, }, @@ -269,7 +269,7 @@ func ecdsaPubKeyLoad(path string) (*ecdsa.PublicKey, error) { return eccKey, nil } -func TestDecrypt(t *testing.T) { +func TestDecryptRSAKeyfile(t *testing.T) { var testCases = map[string]struct { keyPath string input []string @@ -367,3 +367,101 @@ func TestDecrypt(t *testing.T) { }) } } + +func TestSignRSAKeyfile(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/foo@example.com.priv.key", + input: []string{ + "RESET\n", + "OPTION ttyname=/dev/pts/1\n", + "OPTION ttytype=screen\n", + "OPTION lc-ctype=C.UTF-8\n", + "OPTION lc-messages=C\n", + "GETINFO version\n", + "OPTION allow-pinentry-notify\n", + "OPTION agent-awareness=2.1.0\n", + "SCD SERIALNO\n", + "HAVEKEY FC0F9A401ADDB33C0F7225CCA83BFC14E7FEBC7D\n", + "KEYINFO FC0F9A401ADDB33C0F7225CCA83BFC14E7FEBC7D\n", + "RESET\n", + "SIGKEY FC0F9A401ADDB33C0F7225CCA83BFC14E7FEBC7D\n", + "SETKEYDESC Please+enter+the+passphrase+to+unlock+the+OpenPGP+secret+key:%0A%22foo@example.com%22%0A3072-bit+RSA+key,+ID+8D0381C18D1E7CA6,%0Acreated+2021-08-04.%0A\n", + "SETHASH 8 5963E1FA635CA32A85CA43CDCE3CB7A0CB0429B0EB1A94D1AEF08801D3BEB465\n", + "PKSIGN\n", + }, + expect: []string{ + "OK Pleased to meet you, process 123456789\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 \n", + "OK\n", + "S KEYINFO FC0F9A401ADDB33C0F7225CCA83BFC14E7FEBC7D D - - - - - - -\n", + "OK\n", + "OK\n", + "OK\n", + "OK\n", + "OK\n", + "\x44\x20\x28\x37\x3a\x73\x69\x67\x2d\x76\x61\x6c\x28\x33\x3a\x72\x73\x61\x28\x31\x3a\x73\x33\x38\x34\x3a\xb3\x26\x74\x5f\x59\xb5\x50\x8a\x46\x37\xa0\xc0\x91\x3a\x4b\x18\x61\xcb\x4f\xd2\x52\x5d\xbc\xe5\x51\x41\x00\x25\x30\x44\x08\x20\x25\x30\x41\xac\x0b\xff\x3e\xed\x6a\xa4\xf0\xdc\xb9\x1f\x8f\x76\xf1\x30\x8f\xce\xdc\xf5\x79\x2d\x2f\x06\x52\x3b\x49\xd5\x7d\xa1\x4a\xa2\x38\x81\x56\x6c\x59\xb0\x56\x22\xd8\x13\xeb\x7a\xee\xb1\xc5\xd6\xe9\xa0\x3a\xf4\x1b\x12\xa0\x85\x74\xe9\x93\x80\x7d\x7f\x24\xc8\x59\x9d\xb2\x8a\xe6\xc3\x95\xee\x50\x4c\x12\x4a\x1d\x84\x46\x3f\xa2\xc8\x96\xc2\xdf\xb7\x3d\x54\xa0\x55\x4a\x46\x4b\x35\x9f\xf0\x32\x9a\xd9\x0e\xe8\xa3\xa9\xb1\x3b\xa6\x52\x63\x02\xce\x36\x8f\x94\x18\x39\x3e\x11\x26\xb0\xa9\x71\xb8\x1c\x35\x47\xe8\x78\x8d\x12\xcf\x42\x96\xc7\x37\x25\x30\x41\x16\xa4\xbb\x83\x42\xe0\xa7\xed\x11\x35\x84\x5b\x40\xcd\x52\xc5\xd2\xf4\xe2\x86\x8b\x23\x42\x54\xda\xd1\xcd\xfc\x3e\xb2\x84\x1e\x2b\x04\xfb\x72\x04\x2f\xa9\x80\xf7\xa3\x13\x9a\xee\xe0\x26\x17\x6f\xdb\x57\x91\x85\xce\xbc\x5a\x97\x62\x8b\xa4\xa2\x54\x1c\x03\xc0\x3a\x9b\x8e\x4b\x32\x5e\x39\x71\x25\x30\x44\x8e\xae\x14\x09\x05\xcb\x77\x8d\x61\x2a\x4b\x1f\x19\x21\x8a\x68\x80\xd0\x4e\x53\x30\xc3\xab\x03\xd3\x79\x77\x55\xff\x2e\x46\xe3\x08\x03\x86\xef\xe1\xed\x34\x20\x08\x7a\xee\x1f\x0e\xd6\xf0\xbe\xe7\xdd\xab\xf6\x46\xec\xce\xd5\xa6\xc4\xf4\x02\x58\x5a\xcb\x6d\x9f\x2e\xf7\x24\x71\x9e\x13\x24\x22\x42\xe4\x48\xd5\x25\x32\x35\x1f\xac\xfc\x2c\xe2\x5c\x7c\xdb\xaf\xd2\x45\x3c\x99\xe1\xba\xd3\xd4\x95\x9d\xf8\xa1\x21\xca\x3f\xf9\x7b\x08\x50\x75\x13\x7a\x3d\xc9\x48\x9d\x4a\x93\xb6\xb5\x7a\x15\xef\xa6\x4d\xa9\x87\x41\x0e\xde\x25\x32\x35\x04\x18\x41\xa9\x4d\x9c\xbf\x12\x1f\x48\xc0\xa8\x92\xfd\x37\x7d\xec\x29\x29\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) + } + keyfileService, err := gpg.NewKeyfileService(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) + // 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(); 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") + } + } + }) + } +} diff --git a/internal/assuan/event_enumer.go b/internal/assuan/event_enumer.go index 63e758f..0821ab9 100644 --- a/internal/assuan/event_enumer.go +++ b/internal/assuan/event_enumer.go @@ -7,11 +7,11 @@ import ( "strings" ) -const _EventName = "INVALIDEVENTCONNECTRESETOPTIONGETINFOHAVEKEYKEYINFOSIGKEYSETKEYDESCSETHASHPKSIGNSETKEYPKDECRYPT" +const _EventName = "INVALIDEVENTCONNECTRESETOPTIONGETINFOHAVEKEYKEYINFOSIGKEYSETKEYDESCSETHASHPKSIGNSETKEYPKDECRYPTSCD" -var _EventIndex = [...]uint8{0, 12, 19, 24, 30, 37, 44, 51, 57, 67, 74, 80, 86, 95} +var _EventIndex = [...]uint8{0, 12, 19, 24, 30, 37, 44, 51, 57, 67, 74, 80, 86, 95, 98} -const _EventLowerName = "invalideventconnectresetoptiongetinfohavekeykeyinfosigkeysetkeydescsethashpksignsetkeypkdecrypt" +const _EventLowerName = "invalideventconnectresetoptiongetinfohavekeykeyinfosigkeysetkeydescsethashpksignsetkeypkdecryptscd" func (i Event) String() string { if i < 0 || i >= Event(len(_EventIndex)-1) { @@ -37,9 +37,10 @@ func _EventNoOp() { _ = x[pksign-(10)] _ = x[setkey-(11)] _ = x[pkdecrypt-(12)] + _ = x[scd-(13)] } -var _EventValues = []Event{invalidEvent, connect, reset, option, getinfo, havekey, keyinfo, sigkey, setkeydesc, sethash, pksign, setkey, pkdecrypt} +var _EventValues = []Event{invalidEvent, connect, reset, option, getinfo, havekey, keyinfo, sigkey, setkeydesc, sethash, pksign, setkey, pkdecrypt, scd} var _EventNameToValueMap = map[string]Event{ _EventName[0:12]: invalidEvent, @@ -68,6 +69,8 @@ var _EventNameToValueMap = map[string]Event{ _EventLowerName[80:86]: setkey, _EventName[86:95]: pkdecrypt, _EventLowerName[86:95]: pkdecrypt, + _EventName[95:98]: scd, + _EventLowerName[95:98]: scd, } var _EventNames = []string{ @@ -84,6 +87,7 @@ var _EventNames = []string{ _EventName[74:80], _EventName[80:86], _EventName[86:95], + _EventName[95:98], } // EventString retrieves an enum value from the enum constants string name. diff --git a/internal/assuan/fsm.go b/internal/assuan/fsm.go index e66fd1f..75e5e76 100644 --- a/internal/assuan/fsm.go +++ b/internal/assuan/fsm.go @@ -28,6 +28,7 @@ const ( pksign setkey pkdecrypt + scd ) //go:generate enumer -type=State -text -transform upper @@ -97,6 +98,10 @@ var assuanTransitions = []fsm.Transition{ Src: fsm.State(connected), Event: fsm.Event(keyinfo), Dst: fsm.State(connected), + }, { + Src: fsm.State(connected), + Event: fsm.Event(scd), + Dst: fsm.State(connected), }, // signing transitions { diff --git a/internal/assuan/sign.go b/internal/assuan/sign.go index a6c70b7..cee44b8 100644 --- a/internal/assuan/sign.go +++ b/internal/assuan/sign.go @@ -5,6 +5,7 @@ import ( "fmt" "math/big" + "github.com/smlx/piv-agent/internal/gpg" "github.com/smlx/piv-agent/internal/notify" "golang.org/x/crypto/cryptobyte" "golang.org/x/crypto/cryptobyte/asn1" @@ -13,12 +14,33 @@ import ( // sign performs signing of the specified "hash" data, using the specified // "hashAlgo" hash algorithm. It then encodes the response into an s-expression // and returns it as a byte slice. +func (a *Assuan) sign() ([]byte, error) { + switch a.signer.(type) { + case *gpg.RSAKey: + return a.signRSA() + default: + // default also handles mock signers in the test suite + return a.signECDSA() + } +} + +// signRSA returns a signature for the given hash. +func (a *Assuan) signRSA() ([]byte, error) { + signature, err := a.signer.Sign(rand.Reader, a.hash, a.hashAlgo) + if err != nil { + return nil, fmt.Errorf("couldn't sign: %v", err) + } + return []byte(fmt.Sprintf(`D (7:sig-val(3:rsa(1:s%d:%s)))`, len(signature), + percentEncodeSExp(signature))), nil +} + +// signECDSA returns a signature for the given hash. // // This function's complexity is due to the fact that while Sign() returns the // r and s components of the signature ASN1-encoded, gpg expects them to be // separately s-exp encoded. So we have to decode the ASN1 signature, extract // the params, and re-encode them into the s-exp. Ugh. -func (a *Assuan) sign() ([]byte, error) { +func (a *Assuan) signECDSA() ([]byte, error) { cancel := notify.Touch(nil) defer cancel() signature, err := a.signer.Sign(rand.Reader, a.hash, a.hashAlgo) diff --git a/internal/gpg/key.go b/internal/gpg/key.go index bfeabb1..1ba1b6e 100644 --- a/internal/gpg/key.go +++ b/internal/gpg/key.go @@ -3,7 +3,6 @@ package gpg import ( "crypto" "crypto/rsa" - "fmt" "io" "math/big" ) @@ -19,21 +18,21 @@ func (k *RSAKey) Decrypt(_ io.Reader, ciphertext []byte, _ crypto.DecrypterOpts) ([]byte, error) { c := new(big.Int) c.SetBytes(ciphertext) - // libgcrypt does this, not sure if required + // TODO: libgcrypt does this, not sure if required? c.Rem(c, k.rsa.N) // perform arithmetic manually c.Exp(c, k.rsa.D, k.rsa.N) return c.Bytes(), nil } -// Public implements the crypto.Decrypter interface. +// Public implements the other required method of the crypto.Decrypter and +// crypto.Signer interfaces. func (k *RSAKey) Public() crypto.PublicKey { return k.rsa.Public() } // Sign performs RSA signing as per gpg-agent. -func (k *RSAKey) Sign(_ io.Reader, digest []byte, - _ crypto.SignerOpts) ([]byte, error) { - // TODO: implement this - return nil, fmt.Errorf("not implemented") +func (k *RSAKey) Sign(r io.Reader, digest []byte, + o crypto.SignerOpts) ([]byte, error) { + return rsa.SignPKCS1v15(r, k.rsa, o.HashFunc(), digest) } diff --git a/internal/pivservice/pivservice.go b/internal/pivservice/pivservice.go index 3951ea0..15c95b6 100644 --- a/internal/pivservice/pivservice.go +++ b/internal/pivservice/pivservice.go @@ -89,7 +89,7 @@ func (p *PIVService) GetSigner(keygrip []byte) (crypto.Signer, error) { } } } - return nil, nil + return nil, fmt.Errorf("couldn't find keygrip") } // GetDecrypter returns a crypto.Decrypter associated with the given keygrip. From 4155098b8885e99fbab75e92092608c312ca460b Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Sun, 8 Aug 2021 00:21:11 +0800 Subject: [PATCH 09/20] chore: moving modules around Hopefully this is a more logical layout. --- cmd/piv-agent/list.go | 4 +-- cmd/piv-agent/serve.go | 4 +-- internal/assuan/assuan_test.go | 11 +++++--- internal/assuan/sign.go | 6 ++--- internal/{ => keyservice}/gpg/keyfile.go | 0 internal/{ => keyservice}/gpg/keygrip.go | 0 internal/{ => keyservice}/gpg/keygrip_test.go | 2 +- internal/{ => keyservice}/gpg/keyservice.go | 24 +++++++++--------- .../{ => keyservice}/gpg/keyservice_test.go | 4 +-- .../{gpg/key.go => keyservice/gpg/rsakey.go} | 0 .../{ => keyservice}/gpg/testdata/key1.asc | 0 .../{ => keyservice}/gpg/testdata/key2.asc | 0 .../{ => keyservice}/gpg/testdata/key3.asc | 0 .../{ => keyservice}/gpg/testdata/key4.asc | 0 .../private/bar-protected@example.com.gpg | Bin .../gpg/testdata/private/bar@example.com.gpg | Bin .../piv/keyservice.go} | 20 +++++++-------- .../{pivservice => keyservice/piv}/list.go | 8 +++--- internal/securitykey/string.go | 4 +-- internal/server/gpg.go | 22 ++++++++-------- internal/ssh/agent.go | 12 ++++----- 21 files changed, 62 insertions(+), 59 deletions(-) rename internal/{ => keyservice}/gpg/keyfile.go (100%) rename internal/{ => keyservice}/gpg/keygrip.go (100%) rename internal/{ => keyservice}/gpg/keygrip_test.go (97%) rename internal/{ => keyservice}/gpg/keyservice.go (79%) rename internal/{ => keyservice}/gpg/keyservice_test.go (92%) rename internal/{gpg/key.go => keyservice/gpg/rsakey.go} (100%) rename internal/{ => keyservice}/gpg/testdata/key1.asc (100%) rename internal/{ => keyservice}/gpg/testdata/key2.asc (100%) rename internal/{ => keyservice}/gpg/testdata/key3.asc (100%) rename internal/{ => keyservice}/gpg/testdata/key4.asc (100%) rename internal/{ => keyservice}/gpg/testdata/private/bar-protected@example.com.gpg (100%) rename internal/{ => keyservice}/gpg/testdata/private/bar@example.com.gpg (100%) rename internal/{pivservice/pivservice.go => keyservice/piv/keyservice.go} (83%) rename internal/{pivservice => keyservice/piv}/list.go (89%) diff --git a/cmd/piv-agent/list.go b/cmd/piv-agent/list.go index f19154c..969874b 100644 --- a/cmd/piv-agent/list.go +++ b/cmd/piv-agent/list.go @@ -4,7 +4,7 @@ import ( "fmt" "strings" - "github.com/smlx/piv-agent/internal/pivservice" + "github.com/smlx/piv-agent/internal/keyservice/piv" "go.uber.org/zap" ) @@ -17,7 +17,7 @@ type ListCmd struct { // Run the list command. func (cmd *ListCmd) Run(l *zap.Logger) error { - p := pivservice.New(l) + p := piv.New(l) securityKeys, err := p.SecurityKeys() if err != nil { return fmt.Errorf("couldn't get security keys: %w", err) diff --git a/cmd/piv-agent/serve.go b/cmd/piv-agent/serve.go index e69dc0d..bb875de 100644 --- a/cmd/piv-agent/serve.go +++ b/cmd/piv-agent/serve.go @@ -8,8 +8,8 @@ import ( "time" "github.com/coreos/go-systemd/activation" + "github.com/smlx/piv-agent/internal/keyservice/piv" "github.com/smlx/piv-agent/internal/pinentry" - "github.com/smlx/piv-agent/internal/pivservice" "github.com/smlx/piv-agent/internal/server" "github.com/smlx/piv-agent/internal/ssh" "go.uber.org/zap" @@ -48,7 +48,7 @@ func (flagAgents *agentTypeFlag) AfterApply() error { func (cmd *ServeCmd) Run(log *zap.Logger) error { log.Info("startup", zap.String("version", version), zap.String("build date", date)) - p := pivservice.New(log) + p := piv.New(log) // use systemd socket activation ls, err := activation.Listeners() if err != nil { diff --git a/internal/assuan/assuan_test.go b/internal/assuan/assuan_test.go index 1b1f2bc..9c7e8bf 100644 --- a/internal/assuan/assuan_test.go +++ b/internal/assuan/assuan_test.go @@ -4,6 +4,7 @@ import ( "bytes" "crypto" "crypto/ecdsa" + "encoding/hex" "fmt" "io" "math/big" @@ -13,7 +14,7 @@ import ( "github.com/davecgh/go-spew/spew" "github.com/golang/mock/gomock" "github.com/smlx/piv-agent/internal/assuan" - "github.com/smlx/piv-agent/internal/gpg" + "github.com/smlx/piv-agent/internal/keyservice/gpg" "github.com/smlx/piv-agent/internal/mock" "github.com/smlx/piv-agent/internal/securitykey" "go.uber.org/zap" @@ -172,11 +173,13 @@ func TestSign(t *testing.T) { func TestKeyinfo(t *testing.T) { var testCases = map[string]struct { keyPath string + keyGrip string input []string expect []string }{ "keyinfo": { keyPath: "testdata/C54A8868468BC138.asc", + keyGrip: "38F053358EFD6C923D08EE4FC4CEB208CBCDF73C", input: []string{ "RESET\n", "KEYINFO 38F053358EFD6C923D08EE4FC4CEB208CBCDF73C\n", @@ -196,7 +199,7 @@ func TestKeyinfo(t *testing.T) { if err != nil { tt.Fatal(err) } - keygrip, err := gpg.KeygripECDSA(pubKey) + keygrip, err := hex.DecodeString(tc.keyGrip) if err != nil { tt.Fatal(err) } @@ -331,7 +334,7 @@ func TestDecryptRSAKeyfile(t *testing.T) { if err != nil { tt.Fatal(err) } - keyfileService, err := gpg.NewKeyfileService(log, mockPES, tc.keyPath) + keyfileService, err := gpg.New(log, mockPES, tc.keyPath) if err != nil { tt.Fatal(err) } @@ -429,7 +432,7 @@ func TestSignRSAKeyfile(t *testing.T) { if err != nil { tt.Fatal(err) } - keyfileService, err := gpg.NewKeyfileService(log, mockPES, tc.keyPath) + keyfileService, err := gpg.New(log, mockPES, tc.keyPath) if err != nil { tt.Fatal(err) } diff --git a/internal/assuan/sign.go b/internal/assuan/sign.go index cee44b8..629099d 100644 --- a/internal/assuan/sign.go +++ b/internal/assuan/sign.go @@ -2,10 +2,10 @@ package assuan import ( "crypto/rand" + "crypto/rsa" "fmt" "math/big" - "github.com/smlx/piv-agent/internal/gpg" "github.com/smlx/piv-agent/internal/notify" "golang.org/x/crypto/cryptobyte" "golang.org/x/crypto/cryptobyte/asn1" @@ -15,8 +15,8 @@ import ( // "hashAlgo" hash algorithm. It then encodes the response into an s-expression // and returns it as a byte slice. func (a *Assuan) sign() ([]byte, error) { - switch a.signer.(type) { - case *gpg.RSAKey: + switch a.signer.Public().(type) { + case *rsa.PublicKey: return a.signRSA() default: // default also handles mock signers in the test suite diff --git a/internal/gpg/keyfile.go b/internal/keyservice/gpg/keyfile.go similarity index 100% rename from internal/gpg/keyfile.go rename to internal/keyservice/gpg/keyfile.go diff --git a/internal/gpg/keygrip.go b/internal/keyservice/gpg/keygrip.go similarity index 100% rename from internal/gpg/keygrip.go rename to internal/keyservice/gpg/keygrip.go diff --git a/internal/gpg/keygrip_test.go b/internal/keyservice/gpg/keygrip_test.go similarity index 97% rename from internal/gpg/keygrip_test.go rename to internal/keyservice/gpg/keygrip_test.go index 0a46e6c..4837fe7 100644 --- a/internal/gpg/keygrip_test.go +++ b/internal/keyservice/gpg/keygrip_test.go @@ -9,7 +9,7 @@ import ( "strings" "testing" - "github.com/smlx/piv-agent/internal/gpg" + "github.com/smlx/piv-agent/internal/keyservice/gpg" "golang.org/x/crypto/openpgp" "golang.org/x/crypto/openpgp/armor" "golang.org/x/crypto/openpgp/packet" diff --git a/internal/gpg/keyservice.go b/internal/keyservice/gpg/keyservice.go similarity index 79% rename from internal/gpg/keyservice.go rename to internal/keyservice/gpg/keyservice.go index f1a8581..efdb1bd 100644 --- a/internal/gpg/keyservice.go +++ b/internal/keyservice/gpg/keyservice.go @@ -1,6 +1,6 @@ package gpg -//go:generate mockgen -source=keyservice.go -destination=../mock/mock_keyservice.go -package=mock +//go:generate mockgen -source=keyservice.go -destination=../../mock/mock_keyservice.go -package=mock import ( "bytes" @@ -17,9 +17,9 @@ type PINEntryService interface { GetPGPPassphrase(string) ([]byte, error) } -// KeyfileService implements an interface for getting cryptographic keys from +// KeyService implements an interface for getting cryptographic keys from // keyfiles on disk. -type KeyfileService struct { +type KeyService struct { // cache passphrases used for decryption passphrases [][]byte privKeys []*packet.PrivateKey @@ -27,15 +27,15 @@ type KeyfileService struct { pinentry PINEntryService } -// NewKeyfileService returns a keyservice initialised with keys found at path. +// New returns a keyservice initialised with keys found at path. // Path can be a file or directory. -func NewKeyfileService(l *zap.Logger, pe PINEntryService, - path string) (*KeyfileService, error) { +func New(l *zap.Logger, pe PINEntryService, + path string) (*KeyService, error) { p, err := keyfilePrivateKeys(path) if err != nil { return nil, err } - return &KeyfileService{ + return &KeyService{ privKeys: p, log: l, pinentry: pe, @@ -43,13 +43,13 @@ func NewKeyfileService(l *zap.Logger, pe PINEntryService, } // Name returns the name of the keyservice. -func (g *KeyfileService) Name() string { +func (*KeyService) Name() string { return "GPG Keyfile" } // HaveKey takes a list of keygrips, and returns a boolean indicating if any of // the given keygrips were found, the found keygrip, and an error, if any. -func (g *KeyfileService) HaveKey(keygrips [][]byte) (bool, []byte, error) { +func (g *KeyService) HaveKey(keygrips [][]byte) (bool, []byte, error) { for _, kg := range keygrips { key, err := g.getKey(kg) if err != nil { @@ -64,7 +64,7 @@ func (g *KeyfileService) HaveKey(keygrips [][]byte) (bool, []byte, error) { // getKey returns a matching private RSA key if the keygrip matches. If a key // is returned err will be nil. If no key is found, both values may be nil. -func (g *KeyfileService) getKey(keygrip []byte) (*rsa.PrivateKey, error) { +func (g *KeyService) getKey(keygrip []byte) (*rsa.PrivateKey, error) { var pass []byte var err error for _, k := range g.privKeys { @@ -113,7 +113,7 @@ func (g *KeyfileService) getKey(keygrip []byte) (*rsa.PrivateKey, error) { } // GetSigner returns a crypto.Signer associated with the given keygrip. -func (g *KeyfileService) GetSigner(keygrip []byte) (crypto.Signer, error) { +func (g *KeyService) GetSigner(keygrip []byte) (crypto.Signer, error) { rsaPrivKey, err := g.getKey(keygrip) if err != nil { return nil, fmt.Errorf("couldn't getKey: %v", err) @@ -122,7 +122,7 @@ func (g *KeyfileService) GetSigner(keygrip []byte) (crypto.Signer, error) { } // GetDecrypter returns a crypto.Decrypter associated with the given keygrip. -func (g *KeyfileService) GetDecrypter(keygrip []byte) (crypto.Decrypter, error) { +func (g *KeyService) GetDecrypter(keygrip []byte) (crypto.Decrypter, error) { rsaPrivKey, err := g.getKey(keygrip) if err != nil { return nil, fmt.Errorf("couldn't getKey: %v", err) diff --git a/internal/gpg/keyservice_test.go b/internal/keyservice/gpg/keyservice_test.go similarity index 92% rename from internal/gpg/keyservice_test.go rename to internal/keyservice/gpg/keyservice_test.go index 1762f2f..f5739e9 100644 --- a/internal/gpg/keyservice_test.go +++ b/internal/keyservice/gpg/keyservice_test.go @@ -5,7 +5,7 @@ import ( "testing" "github.com/golang/mock/gomock" - "github.com/smlx/piv-agent/internal/gpg" + "github.com/smlx/piv-agent/internal/keyservice/gpg" "github.com/smlx/piv-agent/internal/mock" "go.uber.org/zap" ) @@ -47,7 +47,7 @@ func TestGetSigner(t *testing.T) { mockPES.EXPECT().GetPGPPassphrase(gomock.Any()). Return([]byte("trustno1"), nil) } - ks, err := gpg.NewKeyfileService(log, mockPES, tc.path) + ks, err := gpg.New(log, mockPES, tc.path) if err != nil { tt.Fatal(err) } diff --git a/internal/gpg/key.go b/internal/keyservice/gpg/rsakey.go similarity index 100% rename from internal/gpg/key.go rename to internal/keyservice/gpg/rsakey.go diff --git a/internal/gpg/testdata/key1.asc b/internal/keyservice/gpg/testdata/key1.asc similarity index 100% rename from internal/gpg/testdata/key1.asc rename to internal/keyservice/gpg/testdata/key1.asc diff --git a/internal/gpg/testdata/key2.asc b/internal/keyservice/gpg/testdata/key2.asc similarity index 100% rename from internal/gpg/testdata/key2.asc rename to internal/keyservice/gpg/testdata/key2.asc diff --git a/internal/gpg/testdata/key3.asc b/internal/keyservice/gpg/testdata/key3.asc similarity index 100% rename from internal/gpg/testdata/key3.asc rename to internal/keyservice/gpg/testdata/key3.asc diff --git a/internal/gpg/testdata/key4.asc b/internal/keyservice/gpg/testdata/key4.asc similarity index 100% rename from internal/gpg/testdata/key4.asc rename to internal/keyservice/gpg/testdata/key4.asc diff --git a/internal/gpg/testdata/private/bar-protected@example.com.gpg b/internal/keyservice/gpg/testdata/private/bar-protected@example.com.gpg similarity index 100% rename from internal/gpg/testdata/private/bar-protected@example.com.gpg rename to internal/keyservice/gpg/testdata/private/bar-protected@example.com.gpg diff --git a/internal/gpg/testdata/private/bar@example.com.gpg b/internal/keyservice/gpg/testdata/private/bar@example.com.gpg similarity index 100% rename from internal/gpg/testdata/private/bar@example.com.gpg rename to internal/keyservice/gpg/testdata/private/bar@example.com.gpg diff --git a/internal/pivservice/pivservice.go b/internal/keyservice/piv/keyservice.go similarity index 83% rename from internal/pivservice/pivservice.go rename to internal/keyservice/piv/keyservice.go index 15c95b6..c7c403e 100644 --- a/internal/pivservice/pivservice.go +++ b/internal/keyservice/piv/keyservice.go @@ -1,4 +1,4 @@ -package pivservice +package piv import ( "bytes" @@ -7,33 +7,33 @@ import ( "fmt" "sync" - "github.com/smlx/piv-agent/internal/gpg" + "github.com/smlx/piv-agent/internal/keyservice/gpg" "go.uber.org/zap" ) -// PIVService represents a collection of tokens and slots accessed by the +// KeyService represents a collection of tokens and slots accessed by the // Personal Identity Verifaction card interface. -type PIVService struct { +type KeyService struct { mu sync.Mutex log *zap.Logger securityKeys []SecurityKey } // New constructs a PIV and returns it. -func New(l *zap.Logger) *PIVService { - return &PIVService{ +func New(l *zap.Logger) *KeyService { + return &KeyService{ log: l, } } // Name returns the name of the keyservice. -func (p *PIVService) Name() string { +func (*KeyService) Name() string { return "PIV" } // HaveKey takes a list of keygrips, and returns a boolean indicating if any of // the given keygrips were found, the found keygrip, and an error, if any. -func (p *PIVService) HaveKey(keygrips [][]byte) (bool, []byte, error) { +func (p *KeyService) HaveKey(keygrips [][]byte) (bool, []byte, error) { securityKeys, err := p.SecurityKeys() if err != nil { return false, nil, fmt.Errorf("couldn't get security keys: %w", err) @@ -60,7 +60,7 @@ func (p *PIVService) HaveKey(keygrips [][]byte) (bool, []byte, error) { } // GetSigner returns a crypto.Signer associated with the given keygrip. -func (p *PIVService) GetSigner(keygrip []byte) (crypto.Signer, error) { +func (p *KeyService) GetSigner(keygrip []byte) (crypto.Signer, error) { securityKeys, err := p.SecurityKeys() if err != nil { return nil, fmt.Errorf("couldn't get security keys: %w", err) @@ -93,7 +93,7 @@ func (p *PIVService) GetSigner(keygrip []byte) (crypto.Signer, error) { } // GetDecrypter returns a crypto.Decrypter associated with the given keygrip. -func (p *PIVService) GetDecrypter(keygrip []byte) (crypto.Decrypter, error) { +func (p *KeyService) GetDecrypter(keygrip []byte) (crypto.Decrypter, error) { // TODO: implement this return nil, fmt.Errorf("not implemented") } diff --git a/internal/pivservice/list.go b/internal/keyservice/piv/list.go similarity index 89% rename from internal/pivservice/list.go rename to internal/keyservice/piv/list.go index f8f042a..7979247 100644 --- a/internal/pivservice/list.go +++ b/internal/keyservice/piv/list.go @@ -1,6 +1,6 @@ -package pivservice +package piv -//go:generate mockgen -source=list.go -destination=../mock/mock_pivservice.go -package=mock +//go:generate mockgen -source=list.go -destination=../../mock/mock_pivservice.go -package=mock import ( "crypto" @@ -26,7 +26,7 @@ type SecurityKey interface { StringsSSH() []string } -func (p *PIVService) reloadSecurityKeys() error { +func (p *KeyService) reloadSecurityKeys() error { // try to clean up and reset state for _, k := range p.securityKeys { _ = k.Close() @@ -53,7 +53,7 @@ func (p *PIVService) reloadSecurityKeys() error { } // SecurityKeys returns a slice containing all available security keys. -func (p *PIVService) SecurityKeys() ([]SecurityKey, error) { +func (p *KeyService) SecurityKeys() ([]SecurityKey, error) { p.mu.Lock() defer p.mu.Unlock() var err error diff --git a/internal/securitykey/string.go b/internal/securitykey/string.go index 47c0d92..57d39ee 100644 --- a/internal/securitykey/string.go +++ b/internal/securitykey/string.go @@ -35,7 +35,7 @@ func (k *SecurityKey) StringsSSH() []string { for _, s := range k.SigningKeys() { ss = append(ss, fmt.Sprintf("%s %s\n", strings.TrimSuffix(string(ssh.MarshalAuthorizedKey(s.PubSSH)), "\n"), - fmt.Sprintf("%v #%v, touch policy: %s", k.Card(), k.Serial(), + fmt.Sprintf("%v #%v, touch policy: %s", k.card, k.serial, touchStringMap[s.SlotSpec.TouchPolicy]))) } return ss @@ -114,7 +114,7 @@ func (k *SecurityKey) StringsGPG(name, email string) ([]string, error) { w, err := armor.Encode(&buf, openpgp.PublicKeyType, map[string]string{ "Comment": fmt.Sprintf("%v #%v, touch policy: %s", - k.Card(), k.Serial(), touchStringMap[e.SlotSpec.TouchPolicy]), + k.card, k.serial, touchStringMap[e.SlotSpec.TouchPolicy]), }) if err != nil { return nil, fmt.Errorf("couldn't get PGP public key armorer: %w", err) diff --git a/internal/server/gpg.go b/internal/server/gpg.go index c3bf1c2..4494672 100644 --- a/internal/server/gpg.go +++ b/internal/server/gpg.go @@ -7,29 +7,29 @@ import ( "time" "github.com/smlx/piv-agent/internal/assuan" - "github.com/smlx/piv-agent/internal/gpg" - "github.com/smlx/piv-agent/internal/pivservice" + "github.com/smlx/piv-agent/internal/keyservice/gpg" + "github.com/smlx/piv-agent/internal/keyservice/piv" "go.uber.org/zap" ) // GPG represents a gpg-agent server. type GPG struct { - log *zap.Logger - pivService *pivservice.PIVService - keyfileService *gpg.KeyfileService // fallback keyfile keys + log *zap.Logger + pivKeyService *piv.KeyService + gpgKeyService *gpg.KeyService // fallback keyfile keys } // NewGPG initialises a new gpg-agent server. -func NewGPG(piv *pivservice.PIVService, pinentry gpg.PINEntryService, +func NewGPG(piv *piv.KeyService, pinentry gpg.PINEntryService, log *zap.Logger, path string) *GPG { - kfs, err := gpg.NewKeyfileService(log, pinentry, path) + kfs, err := gpg.New(log, pinentry, path) if err != nil { log.Info("couldn't load keyfiles", zap.String("path", path), zap.Error(err)) } return &GPG{ - pivService: piv, - log: log, - keyfileService: kfs, + log: log, + pivKeyService: piv, + gpgKeyService: kfs, } } @@ -53,7 +53,7 @@ func (g *GPG) Serve(ctx context.Context, l net.Listener, exit *time.Ticker, return fmt.Errorf("couldn't set deadline: %v", err) } // init protocol state machine - a := assuan.New(conn, g.log, g.pivService, g.keyfileService) + a := assuan.New(conn, g.log, g.pivKeyService, g.gpgKeyService) // run the protocol state machine to completion // (client severs connection) if err := a.Run(); err != nil { diff --git a/internal/ssh/agent.go b/internal/ssh/agent.go index f0cd370..d8cbf5a 100644 --- a/internal/ssh/agent.go +++ b/internal/ssh/agent.go @@ -10,9 +10,9 @@ import ( "path/filepath" "sync" + "github.com/smlx/piv-agent/internal/keyservice/piv" "github.com/smlx/piv-agent/internal/notify" pinentry "github.com/smlx/piv-agent/internal/pinentry" - "github.com/smlx/piv-agent/internal/pivservice" "go.uber.org/zap" gossh "golang.org/x/crypto/ssh" "golang.org/x/crypto/ssh/agent" @@ -22,7 +22,7 @@ import ( // https://pkg.go.dev/golang.org/x/crypto/ssh/agent#Agent type Agent struct { mu sync.Mutex - pivService *pivservice.PIVService + piv *piv.KeyService log *zap.Logger loadKeyfile bool } @@ -37,8 +37,8 @@ var ErrUnknownKey = errors.New("requested signature of unknown key") var passphrases = map[string][]byte{} // NewAgent returns a new Agent. -func NewAgent(p *pivservice.PIVService, log *zap.Logger, loadKeyfile bool) *Agent { - return &Agent{pivService: p, log: log, loadKeyfile: loadKeyfile} +func NewAgent(p *piv.KeyService, log *zap.Logger, loadKeyfile bool) *Agent { + return &Agent{piv: p, log: log, loadKeyfile: loadKeyfile} } // List returns the identities known to the agent. @@ -64,7 +64,7 @@ func (a *Agent) List() ([]*agent.Key, error) { // returns the identities from hardware tokens func (a *Agent) securityKeyIDs() ([]*agent.Key, error) { var keys []*agent.Key - securityKeys, err := a.pivService.SecurityKeys() + securityKeys, err := a.piv.SecurityKeys() if err != nil { return nil, fmt.Errorf("couldn't get security keys: %v", err) } @@ -201,7 +201,7 @@ func (a *Agent) Signers() ([]gossh.Signer, error) { // get signers for all keys stored in hardware tokens func (a *Agent) tokenSigners() ([]gossh.Signer, error) { var signers []gossh.Signer - securityKeys, err := a.pivService.SecurityKeys() + securityKeys, err := a.piv.SecurityKeys() if err != nil { return nil, fmt.Errorf("couldn't get security keys: %v", err) } From 60b86cc33fb6acd621b568c10b50871e396ddda9 Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Sun, 8 Aug 2021 00:24:31 +0800 Subject: [PATCH 10/20] chore: update Makefile to ease local testing --- Makefile | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 2852255..6d04b88 100644 --- a/Makefile +++ b/Makefile @@ -1,8 +1,15 @@ +.PHONY: build +build: test + go build ./cmd/piv-agent + +.PHONY: test test: mod-tidy generate go test -v ./... -mod-tidy: +.PHONY: mod-tidy +mod-tidy: generate go mod tidy +.PHONY: generate generate: go generate ./... From 6b0c8dd8901fbb2b98e7a17064e187d44e254ce8 Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Sun, 8 Aug 2021 00:27:57 +0800 Subject: [PATCH 11/20] chore: remove unused test fixtures --- internal/assuan/assuan_test.go | 4 ++-- .../testdata/private/foo@example.com.gpg | Bin 1859 -> 1857 bytes .../testdata/private/foo@example.com.priv.key | Bin 1857 -> 0 bytes 3 files changed, 2 insertions(+), 2 deletions(-) delete mode 100644 internal/assuan/testdata/private/foo@example.com.priv.key diff --git a/internal/assuan/assuan_test.go b/internal/assuan/assuan_test.go index 9c7e8bf..bd398b0 100644 --- a/internal/assuan/assuan_test.go +++ b/internal/assuan/assuan_test.go @@ -280,7 +280,7 @@ func TestDecryptRSAKeyfile(t *testing.T) { }{ // test data is taken from a successful decrypt by gpg-agent "decrypt file": { - keyPath: "testdata/private/foo@example.com.priv.key", + keyPath: "testdata/private/foo@example.com.gpg", input: []string{ "RESET\n", "OPTION ttyname=/dev/pts/1\n", @@ -379,7 +379,7 @@ func TestSignRSAKeyfile(t *testing.T) { }{ // test data is taken from a successful decrypt by gpg-agent "decrypt file": { - keyPath: "testdata/private/foo@example.com.priv.key", + keyPath: "testdata/private/foo@example.com.gpg", input: []string{ "RESET\n", "OPTION ttyname=/dev/pts/1\n", diff --git a/internal/assuan/testdata/private/foo@example.com.gpg b/internal/assuan/testdata/private/foo@example.com.gpg index 41d78dd581e38f8ed4636c1dc639b4b2c11e9a5a..6e9504489742e4c9bc76efa3a9fa4942ca6771ed 100644 GIT binary patch literal 1857 zcmV-H2fp}~1y}@O3YRef3;@X=^IKx9ZeD-xWA%#rdQz+5u_G%=Bcg>X{dHP0b-h6A@7i8H~#n3-W&X91`gL$X=N>ZwUaEs|5!x93(T%Q(=mHtS?h z8s=s01PY?Fm7vLu6!w3xurs}3Xocfc65PdD(oyLuo}5+r>#Ax7@a2{u~{3dPd zVNBpR@-&Glbd5#_Im?e)ojmi}g%dlRj)9{du_sdQ{5mJ9gOSvVkozMtS+BvA;4Xpu z82((I$DocSnzFOQ?KUD6`Cj=qHb-C1m27g$U@FX12-1oYDO&&$0RRC23;sM`S7jKl z3g}t=oXXLk;9Z@QzhM8j9}q)oYlunDqg_9(CAU0@dRq}(e@ZB&N7 zICvizVI&%Mf1x|9DlrCkK!rotnoK9lFS=a7tpXg9I)Qt1DDiN=1BSs&HV)2F;Y$vgRIz=M#1EzF33~epLCBzz zzc3(!mxMF`PG`DIxd9-;`wf~bE8{=)99$yv8mb37!p0zo9oh@SH zhB%bw_%NZHtiLY}oBC5Sx~XjBm_XhBiM<;IN&FPQ&n zI>JtYA{H+SoOtc#u+kA>ka+d%S<23vG`?$e6iF4VaAhRtcrp2NH!AOZhL2WnX#CoB z1_124VU_3ke^mrI(%%PcM1m=9Fa5vn5$|`@Q8u3}vv1?Iki*v-RNYWs=-3k=}Ej_kcgThb7*F=@`{H_Af~l*%Y@MCma4_&sYY?A14w_o#->H82mbAV~V>$JN%&&u$ z#7>8S2}ru1V&)_IQ)ugfDjGSvnXFwHV5rL)NAU3f?MR6WT8{<*l1jiS3wn6BYw)Fi zaH+-NV4>I`Whj*W)QW`M%is~Wh+rqrZ74>Vl6Hm>a7tk-;s z0#nmjN_D~kF5dqgAZlYg6+F_3-%rG&{gISqP%gywtXgr!%{OzI;3;1+6R;>lGl(w* zw%&igzA#Rp28#o1RfY0s&tW6}d&|_urA=NGFByNdK7vSfiX^2RH5-lMb+W^ZpmWq4t2aBO8RV{dJV0n!8$0SEv* z79j)!BN@I_LW+M1>Ui;u1A)Pf9(<++0$~c5F#;P81qlPfX8;8Y2?z%Q1{Dek2nzxN z761Ys0Rk6*0162ZjRS$fjUIfaAfF5WU&)uhi~a#AY+@RUQ38FW$+_6WjA(pZd>#sa z`FNg?A`Nlqn(3@3Px@OdK-Tb@=sgVC%RMQ~P`cmeY@<2t!N2^Ut6h+4+H?TP2X$>r zg%&n_tyw}?ha`WZmr!@)eAo@L%Ac)#u@;5zT+4@Vi?VU1mp7dSV*j=p9r)?d{u?N55Yv=jnLYyo)axomf3nblAQT2<*Z<^! za4~V9Y0U~LQ0kaLDCt}gPiBd`2}*M(H2gm8M~kVt?m!zhMVdYD#HW!g!bEoP&IwmI^4DK24y7W!(N_noD|+` zG!_k8+<7@0ca-l*pdi$yd+Keb9nJSRXtb0GJ)6cGNb*ji`&6JcPCzp>sStk$L)E$B vFeN#kBA@Fr4 literal 1859 zcmV-J2fX-|1y}@O3aNns3;@C?0iFgK!cuMBi|kwCUlbw-ms^#Au`zGT|2rp$J4z{7 zv*Y(*nh~MSgUc`&aRN8;8Ig7vyk0QRJB%_;a8`-RtV(DTJ3ZiJL*2yc^q6PM>bMsL z?gGHPFE%jYX*_8@!PF>xput|d7|$pu%)|c?nf8ny=p-^9Djin!IO`Uee#v>W*~sw{ z@*VUs=#J7;a@>5CmBzisO`m`}x+tO3h;~Yz)Y{}&U9qsu0zM{SI{Lp1%B}_CV>-xT zZLYjjDCzzd0BMceXj+CLp@)QQYRZYJdv$rqAlqep>pR5a%npHUma0YC12lVgMCoPU zGsStwpOJ6>K^zgW1AW`%FAzZStqys~Wggpw6WVJ@1q`?ohK<`lhU~Y6U*EHdD!M{R z-K;+;*m3^Y&yx=AeuQ%9{!itENf92su_J2V4=j)K>~e^~Gfw)sU5U#n6=TOE1`96U zX$7NJ0^~@0yiJhhv+U`@Ifoc00I82VrqW!Cq} z1`cWC5JOV$({6Xh(<~0UnbL>g`3u2^2-{`UOEd#f3x}L(h#SHNH%rYGzE=4l?Khh< zge%U*9J1i)mk6vz`ko5hb;NLU_lSK)qXqWWZ3ZtF#eG%d99NM!LGy0nB>hh1o;Pfu zj6)i;73 zOp3$}a0|A5C*TM;9ZfWmncC`0R+zb&)?J~TYs@XQU z|4+VcBnuTpz{9ZRbBuO!6bIYPC>IWpe|xFE{Q2+-zMv*fEXlMH?$TDagjNtUWlF8^&ed##l>B(K9Wo?Ho}ECI@TXm`Euc3Rcf)?5LAGfBe9Mn5LW%3w4`EN) zv>hN?L(VP#)t%54d(Fse&u7EDve3;?N$lBm`6KE4ArMJY1e2#pP75fp@Vby!5# zWbC+PPDRS0KpkYlua3*n-ii&()W*tB5S8_(g*b=R-dwLU*Ac$t#5;uLT(EPFVrp+;^X zM)dKj6Lp~S3%~P9?Fsdlg`=Qva(^e*Tx;1LdQutoaR2Z?Vs_nWYioERyuV4AJng#J zxFohdAtK-Fa#PRHSjl9rrJAY;Q-j|13ybxOUXs34cA#Yx_ph3wrZswe`O}@hL6LiC zSEO#S%S(hNX~o!yZ`hyxq1iF_amKNev-G?Gl{6)99`fpt5*GhGO(!h*ZxY(*W$XAq zWEwP#r?xO<5MXK*0Ua}Vo*|e)DLEy9WTR}SUgemte2|iEdns8%kN7J;hNzQ{(T#}( xZwZ;}0oiXdDqH+Kk@(Uf1fv!;vKz2a*RChqvC(A?cpvgz;x77NM8JaD4pd;Ee9Qm< diff --git a/internal/assuan/testdata/private/foo@example.com.priv.key b/internal/assuan/testdata/private/foo@example.com.priv.key deleted file mode 100644 index 6e9504489742e4c9bc76efa3a9fa4942ca6771ed..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 1857 zcmV-H2fp}~1y}@O3YRef3;@X=^IKx9ZeD-xWA%#rdQz+5u_G%=Bcg>X{dHP0b-h6A@7i8H~#n3-W&X91`gL$X=N>ZwUaEs|5!x93(T%Q(=mHtS?h z8s=s01PY?Fm7vLu6!w3xurs}3Xocfc65PdD(oyLuo}5+r>#Ax7@a2{u~{3dPd zVNBpR@-&Glbd5#_Im?e)ojmi}g%dlRj)9{du_sdQ{5mJ9gOSvVkozMtS+BvA;4Xpu z82((I$DocSnzFOQ?KUD6`Cj=qHb-C1m27g$U@FX12-1oYDO&&$0RRC23;sM`S7jKl z3g}t=oXXLk;9Z@QzhM8j9}q)oYlunDqg_9(CAU0@dRq}(e@ZB&N7 zICvizVI&%Mf1x|9DlrCkK!rotnoK9lFS=a7tpXg9I)Qt1DDiN=1BSs&HV)2F;Y$vgRIz=M#1EzF33~epLCBzz zzc3(!mxMF`PG`DIxd9-;`wf~bE8{=)99$yv8mb37!p0zo9oh@SH zhB%bw_%NZHtiLY}oBC5Sx~XjBm_XhBiM<;IN&FPQ&n zI>JtYA{H+SoOtc#u+kA>ka+d%S<23vG`?$e6iF4VaAhRtcrp2NH!AOZhL2WnX#CoB z1_124VU_3ke^mrI(%%PcM1m=9Fa5vn5$|`@Q8u3}vv1?Iki*v-RNYWs=-3k=}Ej_kcgThb7*F=@`{H_Af~l*%Y@MCma4_&sYY?A14w_o#->H82mbAV~V>$JN%&&u$ z#7>8S2}ru1V&)_IQ)ugfDjGSvnXFwHV5rL)NAU3f?MR6WT8{<*l1jiS3wn6BYw)Fi zaH+-NV4>I`Whj*W)QW`M%is~Wh+rqrZ74>Vl6Hm>a7tk-;s z0#nmjN_D~kF5dqgAZlYg6+F_3-%rG&{gISqP%gywtXgr!%{OzI;3;1+6R;>lGl(w* zw%&igzA#Rp28#o1RfY0s&tW6}d&|_urA=NGFByNdK7vSfiX^2RH5-lMb+W^ZpmWq4t2aBO8RV{dJV0n!8$0SEv* z79j)!BN@I_LW+M1>Ui;u1A)Pf9(<++0$~c5F#;P81qlPfX8;8Y2?z%Q1{Dek2nzxN z761Ys0Rk6*0162ZjRS$fjUIfaAfF5WU&)uhi~a#AY+@RUQ38FW$+_6WjA(pZd>#sa z`FNg?A`Nlqn(3@3Px@OdK-Tb@=sgVC%RMQ~P`cmeY@<2t!N2^Ut6h+4+H?TP2X$>r zg%&n_tyw}?ha`WZmr!@)eAo@L%Ac)#u@;5zT+4@Vi?VU1mp7dSV*j=p9r)?d{u?N55Yv=jnLYyo)axomf3nblAQT2<*Z<^! za4~V9Y0U~LQ0kaLDCt}gPiBd`2}*M(H2gm8M~kVt?m!zhMVdYD#HW!g!bEoP&IwmI^4DK24y7W!(N_noD|+` zG!_k8+<7@0ca-l*pdi$yd+Keb9nJSRXtb0GJ)6cGNb*ji`&6JcPCzp>sStk$L)E$B vFeN#kBA@Fr4 From 51d6056aaec1f026c2aa6661b27ada2d7825015e Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Sun, 8 Aug 2021 22:06:02 +0800 Subject: [PATCH 12/20] feat: provide the UserID in pinentry prompt for a PGP keyfile --- internal/keyservice/gpg/keyfile.go | 29 +++++--- internal/keyservice/gpg/keyservice.go | 84 ++++++++++++---------- internal/keyservice/gpg/keyservice_test.go | 2 +- internal/mock/mock_keyservice.go | 8 +-- internal/pinentry/pinentry.go | 7 +- 5 files changed, 74 insertions(+), 56 deletions(-) diff --git a/internal/keyservice/gpg/keyfile.go b/internal/keyservice/gpg/keyfile.go index 8c496f5..1e058b4 100644 --- a/internal/keyservice/gpg/keyfile.go +++ b/internal/keyservice/gpg/keyfile.go @@ -11,7 +11,7 @@ import ( ) // keyfilePrivateKeys reads the given path and returns any private keys found. -func keyfilePrivateKeys(p string) ([]*packet.PrivateKey, error) { +func keyfilePrivateKeys(p string) ([]privateKeyfile, error) { f, err := os.Open(p) if err != nil { return nil, fmt.Errorf("couldn't open path %s: %v", p, err) @@ -23,7 +23,8 @@ func keyfilePrivateKeys(p string) ([]*packet.PrivateKey, error) { } switch { case fileInfo.Mode().IsRegular(): - return keysFromFile(f) + pk, err := keysFromFile(f) + return []privateKeyfile{*pk}, err case fileInfo.IsDir(): // enumerate files in directory dirents, err := f.ReadDir(0) @@ -31,7 +32,7 @@ func keyfilePrivateKeys(p string) ([]*packet.PrivateKey, error) { return nil, fmt.Errorf("couldn't read directory") } // get any private keys from each file - var privKeys []*packet.PrivateKey + var privKeys []privateKeyfile for _, dirent := range dirents { direntInfo, err := dirent.Info() if err != nil { @@ -49,7 +50,7 @@ func keyfilePrivateKeys(p string) ([]*packet.PrivateKey, error) { return nil, fmt.Errorf("couldn't get keys from file %s: %v", subPath, err) } - privKeys = append(privKeys, subPrivKeys...) + privKeys = append(privKeys, *subPrivKeys) } } return privKeys, nil @@ -59,9 +60,10 @@ func keyfilePrivateKeys(p string) ([]*packet.PrivateKey, error) { } // keysFromFile read a file and return any private keys found -func keysFromFile(f *os.File) ([]*packet.PrivateKey, error) { +func keysFromFile(f *os.File) (*privateKeyfile, error) { var err error var pkt packet.Packet + var uid *packet.UserId var privKeys []*packet.PrivateKey reader := packet.NewReader(f) for pkt, err = reader.Next(); err != io.EOF; pkt, err = reader.Next() { @@ -71,11 +73,20 @@ func keysFromFile(f *os.File) ([]*packet.PrivateKey, error) { if err != nil { return nil, fmt.Errorf("couldn't get next packet: %v", err) } - k, ok := pkt.(*packet.PrivateKey) - if !ok { + switch k := pkt.(type) { + case *packet.PrivateKey: + privKeys = append(privKeys, k) + case *packet.UserId: + uid = k + default: continue } - privKeys = append(privKeys, k) } - return privKeys, nil + if uid == nil { + uid = packet.NewUserId("n/a", "n/a", "n/a") + } + return &privateKeyfile{ + uid: uid, + keys: privKeys, + }, nil } diff --git a/internal/keyservice/gpg/keyservice.go b/internal/keyservice/gpg/keyservice.go index efdb1bd..a5792cf 100644 --- a/internal/keyservice/gpg/keyservice.go +++ b/internal/keyservice/gpg/keyservice.go @@ -14,7 +14,12 @@ import ( // PINEntryService provides an interface to talk to a pinentry program. type PINEntryService interface { - GetPGPPassphrase(string) ([]byte, error) + GetPGPPassphrase(string, string) ([]byte, error) +} + +type privateKeyfile struct { + uid *packet.UserId + keys []*packet.PrivateKey } // KeyService implements an interface for getting cryptographic keys from @@ -22,15 +27,14 @@ type PINEntryService interface { type KeyService struct { // cache passphrases used for decryption passphrases [][]byte - privKeys []*packet.PrivateKey + privKeys []privateKeyfile log *zap.Logger pinentry PINEntryService } // New returns a keyservice initialised with keys found at path. // Path can be a file or directory. -func New(l *zap.Logger, pe PINEntryService, - path string) (*KeyService, error) { +func New(l *zap.Logger, pe PINEntryService, path string) (*KeyService, error) { p, err := keyfilePrivateKeys(path) if err != nil { return nil, err @@ -67,47 +71,49 @@ func (g *KeyService) HaveKey(keygrips [][]byte) (bool, []byte, error) { func (g *KeyService) getKey(keygrip []byte) (*rsa.PrivateKey, error) { var pass []byte var err error - for _, k := range g.privKeys { - pubKey, ok := k.PublicKey.PublicKey.(*rsa.PublicKey) - if !ok { - continue - } - if !bytes.Equal(keygrip, keygripRSA(pubKey)) { - continue - } - if k.Encrypted { - // try existing passphrases - for _, pass := range g.passphrases { - if err = k.Decrypt(pass); err == nil { - g.log.Debug("decrypted using cached passphrase", - zap.String("fingerprint", k.KeyIdString())) - break + for _, pk := range g.privKeys { + for _, k := range pk.keys { + pubKey, ok := k.PublicKey.PublicKey.(*rsa.PublicKey) + if !ok { + continue + } + if !bytes.Equal(keygrip, keygripRSA(pubKey)) { + continue + } + if k.Encrypted { + // try existing passphrases + for _, pass := range g.passphrases { + if err = k.Decrypt(pass); err == nil { + g.log.Debug("decrypted using cached passphrase", + zap.String("fingerprint", k.KeyIdString())) + break + } } } - } - if k.Encrypted { - // ask for a passphrase - pass, err = g.pinentry.GetPGPPassphrase( - fmt.Sprintf("%X %X %X %X", k.Fingerprint[:5], k.Fingerprint[5:10], - k.Fingerprint[10:15], k.Fingerprint[15:])) - if err != nil { - return nil, fmt.Errorf("couldn't get passphrase for key %s: %v", - k.KeyIdString(), err) + if k.Encrypted { + // ask for a passphrase + pass, err = g.pinentry.GetPGPPassphrase( + fmt.Sprintf("%s (%s) <%s>", pk.uid.Name, pk.uid.Comment, pk.uid.Email), + fmt.Sprintf("%X %X %X %X", k.Fingerprint[:5], k.Fingerprint[5:10], k.Fingerprint[10:15], k.Fingerprint[15:])) + if err != nil { + return nil, fmt.Errorf("couldn't get passphrase for key %s: %v", + k.KeyIdString(), err) + } + g.passphrases = append(g.passphrases, pass) + if err = k.Decrypt(pass); err != nil { + return nil, fmt.Errorf("couldn't decrypt key %s: %v", + k.KeyIdString(), err) + } + g.log.Debug("decrypted using passphrase", + zap.String("fingerprint", k.KeyIdString())) } - g.passphrases = append(g.passphrases, pass) - if err = k.Decrypt(pass); err != nil { - return nil, fmt.Errorf("couldn't decrypt key %s: %v", + privKey, ok := k.PrivateKey.(*rsa.PrivateKey) + if !ok { + return nil, fmt.Errorf("not an RSA key %s: %v", k.KeyIdString(), err) } - g.log.Debug("decrypted using passphrase", - zap.String("fingerprint", k.KeyIdString())) - } - privKey, ok := k.PrivateKey.(*rsa.PrivateKey) - if !ok { - return nil, fmt.Errorf("not an RSA key %s: %v", - k.KeyIdString(), err) + return privKey, nil } - return privKey, nil } return nil, nil } diff --git a/internal/keyservice/gpg/keyservice_test.go b/internal/keyservice/gpg/keyservice_test.go index f5739e9..a4c2646 100644 --- a/internal/keyservice/gpg/keyservice_test.go +++ b/internal/keyservice/gpg/keyservice_test.go @@ -44,7 +44,7 @@ func TestGetSigner(t *testing.T) { defer ctrl.Finish() var mockPES = mock.NewMockPINEntryService(ctrl) if tc.protected { - mockPES.EXPECT().GetPGPPassphrase(gomock.Any()). + mockPES.EXPECT().GetPGPPassphrase(gomock.Any(), gomock.Any()). Return([]byte("trustno1"), nil) } ks, err := gpg.New(log, mockPES, tc.path) diff --git a/internal/mock/mock_keyservice.go b/internal/mock/mock_keyservice.go index 9e29ead..c22e200 100644 --- a/internal/mock/mock_keyservice.go +++ b/internal/mock/mock_keyservice.go @@ -34,16 +34,16 @@ func (m *MockPINEntryService) EXPECT() *MockPINEntryServiceMockRecorder { } // GetPGPPassphrase mocks base method. -func (m *MockPINEntryService) GetPGPPassphrase(arg0 string) ([]byte, error) { +func (m *MockPINEntryService) GetPGPPassphrase(arg0, arg1 string) ([]byte, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetPGPPassphrase", arg0) + ret := m.ctrl.Call(m, "GetPGPPassphrase", arg0, arg1) ret0, _ := ret[0].([]byte) ret1, _ := ret[1].(error) return ret0, ret1 } // GetPGPPassphrase indicates an expected call of GetPGPPassphrase. -func (mr *MockPINEntryServiceMockRecorder) GetPGPPassphrase(arg0 interface{}) *gomock.Call { +func (mr *MockPINEntryServiceMockRecorder) GetPGPPassphrase(arg0, arg1 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPGPPassphrase", reflect.TypeOf((*MockPINEntryService)(nil).GetPGPPassphrase), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPGPPassphrase", reflect.TypeOf((*MockPINEntryService)(nil).GetPGPPassphrase), arg0, arg1) } diff --git a/internal/pinentry/pinentry.go b/internal/pinentry/pinentry.go index f32a3cf..24cc6e5 100644 --- a/internal/pinentry/pinentry.go +++ b/internal/pinentry/pinentry.go @@ -16,8 +16,8 @@ type SecurityKey interface { type PINEntry struct{} // GetPGPPassphrase uses pinentry to get the passphrase of the key with the -// given keygrip. -func (*PINEntry) GetPGPPassphrase(fingerprint string) ([]byte, error) { +// given fingerprint. +func (*PINEntry) GetPGPPassphrase(userID, fingerprint string) ([]byte, error) { p, err := pinentry.New() if err != nil { return []byte{}, fmt.Errorf("couldn't get pinentry client: %w", err) @@ -33,7 +33,8 @@ func (*PINEntry) GetPGPPassphrase(fingerprint string) ([]byte, error) { return nil, fmt.Errorf("couldn't set prompt on passphrase pinentry: %w", err) } - err = p.Set("desc", fmt.Sprintf("PGP key fingerprint: %s", fingerprint)) + err = p.Set("desc", fmt.Sprintf("UserID: %s, Fingerprint: %s", userID, + fingerprint)) if err != nil { return nil, fmt.Errorf("couldn't set desc on passphrase pinentry: %w", err) From e3a8b087f50b574e303b0c355569fb683cdaed60 Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Tue, 10 Aug 2021 17:04:24 +0800 Subject: [PATCH 13/20] feat: implement ecdsa keyfile signing support --- internal/assuan/assuan.go | 36 ++++- internal/assuan/assuan_test.go | 86 ++++++++++++ internal/assuan/event_enumer.go | 72 +++++----- internal/assuan/fsm.go | 14 ++ internal/assuan/readkey.go | 39 ++++++ .../foo-sub-ecdsa@example.com.sub-ecdsa.gpg | Bin 0 -> 298 bytes .../foo-sub@example.com.sub-rsa.gpg | Bin 0 -> 1860 bytes .../foo@example.com.primary-rsa.gpg | Bin 0 -> 1857 bytes internal/keyservice/gpg/keyservice.go | 130 +++++++++++++----- internal/keyservice/gpg/rsakey.go | 10 +- 10 files changed, 317 insertions(+), 70 deletions(-) create mode 100644 internal/assuan/readkey.go create mode 100644 internal/assuan/testdata/private-subkeys/foo-sub-ecdsa@example.com.sub-ecdsa.gpg create mode 100644 internal/assuan/testdata/private-subkeys/foo-sub@example.com.sub-rsa.gpg create mode 100644 internal/assuan/testdata/private-subkeys/foo@example.com.primary-rsa.gpg diff --git a/internal/assuan/assuan.go b/internal/assuan/assuan.go index 1c703a6..cb92fef 100644 --- a/internal/assuan/assuan.go +++ b/internal/assuan/assuan.go @@ -95,8 +95,8 @@ func New(rw io.ReadWriter, log *zap.Logger, ks ...KeyService) *Assuan { } keyFound, keygrip, err = haveKey(ks, keygrips) if err != nil { - _, _ = io.WriteString(rw, "ERR 1 couldn't check for keygrip\n") - return fmt.Errorf("couldn't check for keygrip: %v", err) + _, _ = io.WriteString(rw, "ERR 1 couldn't match keygrip\n") + return fmt.Errorf("couldn't match keygrip: %v", err) } if keyFound { _, err = io.WriteString(rw, @@ -108,6 +108,38 @@ func New(rw io.ReadWriter, log *zap.Logger, ks ...KeyService) *Assuan { case scd: // ignore scdaemon requests _, err = io.WriteString(rw, "ERR 100696144 No such device \n") + case readkey: + // READKEY argument is a keygrip + // return information about the given key + keygrips, err = hexDecode(assuan.data...) + if err != nil { + return fmt.Errorf("couldn't decode keygrips: %v", err) + } + var signer crypto.Signer + for _, k := range ks { + signer, err = k.GetSigner(keygrips[0]) + if err == nil { + break + } + } + if signer == nil { + _, _ = io.WriteString(rw, "ERR 1 couldn't match keygrip\n") + return fmt.Errorf("couldn't match keygrip: %v", err) + } + readKeyData, err := readKeyData(signer.Public()) + if err != nil { + _, _ = io.WriteString(rw, "ERR 1 couldn't get key data\n") + return fmt.Errorf("couldn't get key data: %v", err) + } + _, err = io.WriteString(rw, readKeyData) + case setkeydesc: + // ignore this event since we don't currently use the client's + // description in the prompt + _, err = io.WriteString(rw, "OK\n") + case passwd: + // ignore this event since we assume that if the key is decrypted the + // user has permissions + _, err = io.WriteString(rw, "OK\n") default: return fmt.Errorf("unknown event: %v", e) } diff --git a/internal/assuan/assuan_test.go b/internal/assuan/assuan_test.go index bd398b0..31aee80 100644 --- a/internal/assuan/assuan_test.go +++ b/internal/assuan/assuan_test.go @@ -468,3 +468,89 @@ func TestSignRSAKeyfile(t *testing.T) { }) } } + +func TestReadKey(t *testing.T) { + var testCases = map[string]struct { + keyPath string + input []string + expect []string + }{ + "rsa": { + keyPath: "testdata/private-subkeys", + input: []string{ + "RESET\n", + "READKEY EA8E47C68880D1620FF10CC7CB91E5605758CC8D\n", + "SETKEYDESC Please+enter+the+passphrase+to+unlock+the+OpenPGP+secret+key:%0A%22foo@example.com%22%0A3072-bit+RSA+key,+ID+AD024955495A860B,%0Acreated+2021-08-07.%0A\n", + "PASSWD --verify B242AADA8260B77F0F5069F127D6B7E4F44B5FAA\n", + }, + expect: []string{ + "OK Pleased to meet you, process 123456789\n", + "OK\n", + "\x44\x20\x28\x31\x30\x3a\x70\x75\x62\x6c\x69\x63\x2d\x6b\x65\x79\x28\x33\x3a\x72\x73\x61\x28\x31\x3a\x6e\x33\x38\x35\x3a\x00\xbe\xe3\x07\x13\x3c\xae\xd7\x10\xe4\xdd\x84\x20\xc3\x96\xba\xdc\xe0\x09\x6d\xce\xbf\xc2\x55\xe3\x24\x4b\x96\x76\xf5\xd9\xcf\x02\x58\xbf\x69\x16\xcf\x2a\xa4\xdc\x8c\x82\x57\xb0\x5a\x16\x74\xf6\xd5\x21\xee\xdc\xce\x89\x64\xcd\x66\xf5\xee\x89\x09\xa6\x44\xce\x9d\x03\xc0\x44\x4d\x90\xdf\x60\x07\xc6\xf8\x2f\x98\x07\x9b\x95\xb3\xe5\x16\xb8\x1d\x59\xd1\x19\x97\x4c\x36\xbd\xce\xc7\xe1\x17\x7d\x6a\xdc\xa0\x16\x93\x2c\x91\x70\x7c\xf2\x1b\xd9\x5b\x4a\xd5\x46\x65\x9e\x09\xcc\x38\xbe\x86\xbd\xdd\xbf\x91\x7c\x04\x6c\xba\x38\xaf\xe6\xb4\xbb\x38\xa0\x3b\x3b\x07\x60\x2e\xbb\x6d\x45\x31\x1b\x0e\x37\x85\xdb\xa0\x93\xa5\x5c\xf6\xde\x69\x9e\x66\x3e\xa2\x3c\xf9\x59\x4b\x18\xc5\x5b\xdb\x4d\xa8\xcb\x80\xe6\xf9\x52\x1e\x2c\xb8\xab\xac\x7b\x14\xe9\xa8\x6a\x6d\xc6\x51\xb1\x74\x02\xa5\x13\x58\x66\x25\x32\x35\x3b\xed\xe3\x63\xb2\x7a\x8f\x93\x9b\x2c\x04\xdd\xf6\x56\xa9\xb2\x40\x34\xa9\x9b\xe6\xe1\x33\x5b\xe2\xa8\x12\x18\x48\x4e\xa6\xb7\xdd\xbf\xf0\xd2\x70\x18\x7b\x9d\xd3\xec\x55\x5f\xb7\xe8\x07\x1a\x90\x1e\xe4\x68\xa9\x67\x5c\xda\xe9\xea\x29\x19\xeb\x4c\x1c\x6a\x44\x06\x39\xea\xa2\xda\x29\x49\xdf\xd1\x00\x86\x5a\xe2\xe2\xe0\xa4\xa6\x2f\x74\x57\xbc\x78\x75\xa9\xd6\x81\xb1\x11\xbd\xca\x08\x17\x56\x9f\x42\xfe\x3f\x1a\xd1\x7e\xb2\x90\x27\x8a\x31\x8c\x88\x32\x3a\x28\x90\x10\xaf\x4d\xf8\x51\x94\x6f\x29\x21\xa4\x74\xfb\x65\x24\xcc\x5f\x48\x68\xdd\xff\x41\xb2\xe4\xa7\xbf\x25\x32\x35\xbe\x8d\xd8\x9f\x95\xd3\x7d\xe8\xf2\x4b\x78\xa1\x93\x29\xa5\x8b\xfa\x8d\x83\x6e\xbf\x9c\x5b\x1e\x38\xe3\x47\x60\xc6\xde\x4a\xd0\x78\x80\x6f\x20\xbf\xfd\x63\x12\x6f\xdd\xa3\x81\xf5\xf9\x29\x28\x31\x3a\x65\x33\x3a\x01\x00\x01\x29\x29\x29\x0a", + "OK\n", + "OK\n", + }, + }, + "ecdsa": { + keyPath: "testdata/private-subkeys", + input: []string{ + "RESET\n", + "READKEY 586A6F8E9CD839FD26D868D084DDFEBB0CCC7EF0\n", + "SETKEYDESC Please+enter+the+passphrase+to+unlock+the+OpenPGP+secret+key:%0A%22foo@example.com%22%0A3072-bit+RSA+key,+ID+AD024955495A860B,%0Acreated+2021-08-07.%0A\n", + "PASSWD --verify B242AADA8260B77F0F5069F127D6B7E4F44B5FAA\n", + }, + expect: []string{ + "OK Pleased to meet you, process 123456789\n", + "OK\n", + "\x44\x20\x28\x31\x30\x3a\x70\x75\x62\x6c\x69\x63\x2d\x6b\x65\x79\x28\x33\x3a\x65\x63\x63\x28\x35\x3a\x63\x75\x72\x76\x65\x31\x30\x3a\x4e\x49\x53\x54\x20\x50\x2d\x32\x35\x36\x29\x28\x31\x3a\x71\x36\x35\x3a\x04\xbf\x06\xac\x95\x31\xae\x04\x93\x98\x21\x03\x83\x35\x9d\x4e\x58\x92\xa2\xe9\x24\x2f\x76\x54\x67\x45\xf0\x35\x28\xf4\x47\x14\x59\x26\x0c\xf9\x1b\x24\x10\x6b\x07\xe3\x33\x05\x4c\xcb\x96\xe2\xdd\x96\xd4\x0f\x3e\x4b\xd7\x67\x44\xdb\x82\x42\x24\xe6\x8b\x7f\xa6\x29\x29\x29\x0a", + "OK\n", + "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) + } + 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) + // 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(); 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") + } + } + }) + } +} diff --git a/internal/assuan/event_enumer.go b/internal/assuan/event_enumer.go index 0821ab9..fcd08ff 100644 --- a/internal/assuan/event_enumer.go +++ b/internal/assuan/event_enumer.go @@ -7,11 +7,11 @@ import ( "strings" ) -const _EventName = "INVALIDEVENTCONNECTRESETOPTIONGETINFOHAVEKEYKEYINFOSIGKEYSETKEYDESCSETHASHPKSIGNSETKEYPKDECRYPTSCD" +const _EventName = "INVALIDEVENTCONNECTRESETOPTIONGETINFOHAVEKEYKEYINFOSIGKEYSETKEYDESCSETHASHPKSIGNSETKEYPKDECRYPTSCDREADKEYPASSWD" -var _EventIndex = [...]uint8{0, 12, 19, 24, 30, 37, 44, 51, 57, 67, 74, 80, 86, 95, 98} +var _EventIndex = [...]uint8{0, 12, 19, 24, 30, 37, 44, 51, 57, 67, 74, 80, 86, 95, 98, 105, 111} -const _EventLowerName = "invalideventconnectresetoptiongetinfohavekeykeyinfosigkeysetkeydescsethashpksignsetkeypkdecryptscd" +const _EventLowerName = "invalideventconnectresetoptiongetinfohavekeykeyinfosigkeysetkeydescsethashpksignsetkeypkdecryptscdreadkeypasswd" func (i Event) String() string { if i < 0 || i >= Event(len(_EventIndex)-1) { @@ -38,39 +38,45 @@ func _EventNoOp() { _ = x[setkey-(11)] _ = x[pkdecrypt-(12)] _ = x[scd-(13)] + _ = x[readkey-(14)] + _ = x[passwd-(15)] } -var _EventValues = []Event{invalidEvent, connect, reset, option, getinfo, havekey, keyinfo, sigkey, setkeydesc, sethash, pksign, setkey, pkdecrypt, scd} +var _EventValues = []Event{invalidEvent, connect, reset, option, getinfo, havekey, keyinfo, sigkey, setkeydesc, sethash, pksign, setkey, pkdecrypt, scd, readkey, passwd} var _EventNameToValueMap = map[string]Event{ - _EventName[0:12]: invalidEvent, - _EventLowerName[0:12]: invalidEvent, - _EventName[12:19]: connect, - _EventLowerName[12:19]: connect, - _EventName[19:24]: reset, - _EventLowerName[19:24]: reset, - _EventName[24:30]: option, - _EventLowerName[24:30]: option, - _EventName[30:37]: getinfo, - _EventLowerName[30:37]: getinfo, - _EventName[37:44]: havekey, - _EventLowerName[37:44]: havekey, - _EventName[44:51]: keyinfo, - _EventLowerName[44:51]: keyinfo, - _EventName[51:57]: sigkey, - _EventLowerName[51:57]: sigkey, - _EventName[57:67]: setkeydesc, - _EventLowerName[57:67]: setkeydesc, - _EventName[67:74]: sethash, - _EventLowerName[67:74]: sethash, - _EventName[74:80]: pksign, - _EventLowerName[74:80]: pksign, - _EventName[80:86]: setkey, - _EventLowerName[80:86]: setkey, - _EventName[86:95]: pkdecrypt, - _EventLowerName[86:95]: pkdecrypt, - _EventName[95:98]: scd, - _EventLowerName[95:98]: scd, + _EventName[0:12]: invalidEvent, + _EventLowerName[0:12]: invalidEvent, + _EventName[12:19]: connect, + _EventLowerName[12:19]: connect, + _EventName[19:24]: reset, + _EventLowerName[19:24]: reset, + _EventName[24:30]: option, + _EventLowerName[24:30]: option, + _EventName[30:37]: getinfo, + _EventLowerName[30:37]: getinfo, + _EventName[37:44]: havekey, + _EventLowerName[37:44]: havekey, + _EventName[44:51]: keyinfo, + _EventLowerName[44:51]: keyinfo, + _EventName[51:57]: sigkey, + _EventLowerName[51:57]: sigkey, + _EventName[57:67]: setkeydesc, + _EventLowerName[57:67]: setkeydesc, + _EventName[67:74]: sethash, + _EventLowerName[67:74]: sethash, + _EventName[74:80]: pksign, + _EventLowerName[74:80]: pksign, + _EventName[80:86]: setkey, + _EventLowerName[80:86]: setkey, + _EventName[86:95]: pkdecrypt, + _EventLowerName[86:95]: pkdecrypt, + _EventName[95:98]: scd, + _EventLowerName[95:98]: scd, + _EventName[98:105]: readkey, + _EventLowerName[98:105]: readkey, + _EventName[105:111]: passwd, + _EventLowerName[105:111]: passwd, } var _EventNames = []string{ @@ -88,6 +94,8 @@ var _EventNames = []string{ _EventName[80:86], _EventName[86:95], _EventName[95:98], + _EventName[98:105], + _EventName[105:111], } // EventString retrieves an enum value from the enum constants string name. diff --git a/internal/assuan/fsm.go b/internal/assuan/fsm.go index 75e5e76..48c28d2 100644 --- a/internal/assuan/fsm.go +++ b/internal/assuan/fsm.go @@ -29,6 +29,8 @@ const ( setkey pkdecrypt scd + readkey + passwd ) //go:generate enumer -type=State -text -transform upper @@ -102,6 +104,18 @@ var assuanTransitions = []fsm.Transition{ Src: fsm.State(connected), Event: fsm.Event(scd), Dst: fsm.State(connected), + }, { + Src: fsm.State(connected), + Event: fsm.Event(readkey), + Dst: fsm.State(connected), + }, { + Src: fsm.State(connected), + Event: fsm.Event(setkeydesc), + Dst: fsm.State(connected), + }, { + Src: fsm.State(connected), + Event: fsm.Event(passwd), + Dst: fsm.State(connected), }, // signing transitions { diff --git a/internal/assuan/readkey.go b/internal/assuan/readkey.go new file mode 100644 index 0000000..c658c66 --- /dev/null +++ b/internal/assuan/readkey.go @@ -0,0 +1,39 @@ +package assuan + +import ( + "crypto" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rsa" + "fmt" + "math/big" +) + +// readKeyData returns information about the given key in a libgcrypt-specific +// format +func readKeyData(pub crypto.PublicKey) (string, error) { + switch k := pub.(type) { + case *rsa.PublicKey: + n := k.N.Bytes() + nLen := len(n) // need the actual byte length before munging + n = percentEncodeSExp(n) // ugh + ei := new(big.Int) + ei.SetInt64(int64(k.E)) + e := ei.Bytes() + // prefix the key with a null byte for compatibility + return fmt.Sprintf("D (10:public-key(3:rsa(1:n%d:\x00%s)(1:e%d:%s)))\n", + nLen+1, n, len(e), e), nil + case *ecdsa.PublicKey: + switch k.Curve { + case elliptic.P256(): + q := elliptic.Marshal(k.Curve, k.X, k.Y) + return fmt.Sprintf( + "D (10:public-key(3:ecc(5:curve10:NIST P-256)(1:q%d:%s)))\n", + len(q), q), nil + default: + return "", fmt.Errorf("unsupported curve: %T", k.Curve) + } + default: + return "", nil + } +} diff --git a/internal/assuan/testdata/private-subkeys/foo-sub-ecdsa@example.com.sub-ecdsa.gpg b/internal/assuan/testdata/private-subkeys/foo-sub-ecdsa@example.com.sub-ecdsa.gpg new file mode 100644 index 0000000000000000000000000000000000000000..a068a75c042d689874265c0b1c50aca00b9c2bff GIT binary patch literal 298 zcmV+_0oDGLcLZS&ytNYuDuzhTJp%y;0s{oU2CS7at^|{qAp?UoolaPiqUj_rc2s9Y z@HHs(M-*8m4EY-*5NikHGX+e`mg3!()DJ#O*JniAf9e^Z!L3mVl8E3WOHFaWq4t2aBO8RV{dJU zlmrtK2mm}5Ap|mm9fEG}asa?Bpi5Kj3$_RBr^^KbVGz8v0viJb2?N4s00j#P2nPZN z6$%Lm3jzWb00JHX0vCV)3JDNX?F+UC?5E4Qa{&Ko?mi2+PrEAO_s&63Csb3;@342NOK5*AV30gdoF~y4>IiZO*^KRpTT}mUi{o&jMJ# zX%^2aq}+^xSFlj%Yf$jQXVY0tE_tz>8NUL#!<0! z0;LmJW+glA<72XVkCU4$1l{&lsj@&cshj5EGh5=Q5*SEMrnlX{@X~M?d!5tlRbRL0 z2O5wbOPLX zzc{^VG)6$sO)Ky%+zG=}P=Tq#&44-&B1c|(O>66d8*CqTh@nvKnG+hcGn*31JaPq6 zy3r&2(Yq4#CAsZRw7(zZgb{#M)Xa;14@Zh-Up&zd$>J!#1T# zj3ac}7W6NMWct$|FuOW&f65g>;&C?6hBj%aSv>(Qk220Qp+x{8hwTq)mCa=IRAaS_ z?_p7omH-_(XZv$2VFm!xsB%r0Y-3Gp;U^g7`b5jYKzZ}!@bScY*x<~<{lGHhKuaqb zpcE@RClh9Hm=%>vvBBvv$zlJZ?ID-bBH8_9*Y5uI9b(!r@r-iV%v?U>XAsi6n?n@c zt_K^oU;pD`p@bkh-MRoi4}*^kPU9d42+^6z)BHe+S??EqHX9v55$Y0VkDNCNM``{; zvRSN_FYA#M1XFGMHn?7Y(crwFBwc7&G5d*}n}pa&@C3mZf-mxgW?4Fa^`G&Kql?)F z0OsJAEbs20v}sJp;SA#9%J{9u5iIGkHmLLf#UUB=bS_V zt}Rq@O(@W*J&Z_OJDoiCGIUh+y@5VwS~0b7+c&b{z^ANwdX`14j};?dGOUlW6K=nP z*+hLWy}9;mdLo=N#xetIS~UTs*PF6-DpuC53O5Z?zSt@UAqD*x|7SpstF3sD)|Cdf zdZX~9_beTLpgBcph{nUpSt)C2#Gu+*(?dJbe6v9)Y3!YB=!SVZ zCL91@JX_U&yE?A&S?h&Fk!W_e)rMl(>Ca$l`$o`m@1^y*2xQ zW@r!wV`Ccv&Xj{!-euK7K?kzh5Jv3X@{@sigmVyXfO*V+8)GXr1lUcuvSD3jm-J37 zuf6>5IO^lby4fiU+Es7L*nY!f6Hsy9F|-qAZ*MJgbz(qecwudDY-KKEZ*7SI(gYI$ z2mm}5Aq1XhpuN;Ddrn`E*Eh^IB#8g?GHV3_VGytm0vitn2?N4s00j#P2nPZN6$%Lm z3jzWb00JHX0vCV)3JDO*HYAAu^fGHYAq)K;kcpcMb3EFA>0*YE6QxUt0Mlq`a37V? zi@Z>_^$SgSAP$GDWrkP8>Q|^zbF!65x*;rofI$&*VCmuS&tdHPftJ7{jmN(6R@lpYqi(`v>2zfSSV(?C znfe>;9*XU-lldwm78~*W#qv;(FBYRlIPva#BvgGU_*2I91eVw)9fb8Al?H%w^||

_@1Y{i6(k5(u&=c<@h7h_{-Qz6HB#JQr literal 0 HcmV?d00001 diff --git a/internal/assuan/testdata/private-subkeys/foo@example.com.primary-rsa.gpg b/internal/assuan/testdata/private-subkeys/foo@example.com.primary-rsa.gpg new file mode 100644 index 0000000000000000000000000000000000000000..6e4862af564ec386130f4a65965fd3c0c85c6014 GIT binary patch literal 1857 zcmV-H2fp}~1y}@O4vP{23;?#p{qGc7JG3yoWwV&c^;l1rtBEN?ML|kH6XSv05eape zLj6vWe`Epz1S$qP<)_V$&Y4Xs@Wjv~SLBUPA}1cKQj+?GL;)H0DIcp?Wl7#A-UMHa z@0a9922y|ICa7X7t&#YIBdo&K{dVTVJC!0~cZpCB2C zfRhPOJ6Gb2R?I!5@vn=JH(smCL^jgh6YAys^swmubn73>mCTc%5#U9=)BBAIc`oMI zUjJ1B>W)~Tcr@iI?Tr?Kh=Js|ddLwr*Ro3 zaD|3Y3B(NRpV`)fRLLs|80?X}joN*7$-D4hTi0RRC23;Yd&AMzdO zp<4h*aiV!}wq*r>l!W=F64090>x&Irq8JQ}lm6znQrS>jSIWm#83@QaI|OHlx-G$@ z0BNO`t5e$7f0=}*BqWLlt0NT^>QLQJNVC+c?v`VPp2Q|Sr&)BzF^L~iq_ zrq(EyD4ep``0Z^su&v%8E?Fno1AfGzM05`!V9Z4;Ru)q{0PppVZj(>cWW9bH*E*EG z8G?Q~YtN|ZNmUuIzZGONTH2-74gqnDhokQlx zpAF)EF9vbDeVn5oF{aPliE_Nh_+)a%DDQRw#D)%je%a1{cuq=+K z@iFV-$4z(-k%ezg51F%0#KA!#nGBxR^ditsg9Ihr##2QP3 z1_0c^J7AV;N4`(6jT=qju#u54W5(j3F^edyq)dnV7{sVYoU%!_Q zj3Ujk$_1_RnW#+d)5niI_LqoVJ2I~6!4!e|9m*NBINR!N&0}jkD~ZuGyY(5p2Aj6Q zS;3#kITFvp$r@%ugZl)m;IR1(s8GJJ!#`34a`%Oo@aS2qsq~MsD0$~VYb?74twLZS zXy9BIhxg?S;_UqqAzrVO2Z*e#AB3~nkW9y8HvV>fwi2i6;a>&-f+O7JED>$RRI>5& z;1y??Yo6w2L}+WPU|4p6*`N@Vt3llPW_|$1J)S?aw*Ulymp_A-g&u;(+1Ah*DI1m% zxA)i?cn-;hwk9AXe1O^H{O=57Tid}YW66Fp5LY6?HYoVW$NkerLoATmsxt3|4ECiD z>yNYuXprr$nGa`KOjyx2%zvoi?!0%eUD?1wUFD>UO ziAX6-UjizC$54X5i>lF0!XJ~r93QH`+M=`%W^ZpmWq4t2aBO8RV{dJV0n!8$0SEv* z79j*`D)xSvAftfOSS1~;0!dX#T80Y+0$~n|5&|0!1qlPfX8;8Y2?z%Q1{Dek2nzxN z761Ys0Rk6*0162ZtpZ6^Nm_;r1|vZBZCly@L=Mf z3d+0)m@0~0O&mX-^c(YKOm2qkkF2IjujywgSgRzj>&m9^Aqac*W*>LOozLUDp7tA zk7SDfs1NM>rQYQKYBl^I>ZBRj(v>9`+|MJcD0D(PfXeSz=1jN0H!p}tw^ZvJg6>>a zI-8DH2Hp&ZGee", pk.uid.Name, pk.uid.Comment, pk.uid.Email), - fmt.Sprintf("%X %X %X %X", k.Fingerprint[:5], k.Fingerprint[5:10], k.Fingerprint[10:15], k.Fingerprint[15:])) - if err != nil { - return nil, fmt.Errorf("couldn't get passphrase for key %s: %v", - k.KeyIdString(), err) - } - g.passphrases = append(g.passphrases, pass) - if err = k.Decrypt(pass); err != nil { - return nil, fmt.Errorf("couldn't decrypt key %s: %v", - k.KeyIdString(), err) - } - g.log.Debug("decrypted using passphrase", - zap.String("fingerprint", k.KeyIdString())) + err = g.decryptPrivateKey(k, + fmt.Sprintf("%s (%s) <%s>", + pk.uid.Name, pk.uid.Comment, pk.uid.Email)) + if err != nil { + return nil, err } privKey, ok := k.PrivateKey.(*rsa.PrivateKey) if !ok { @@ -118,20 +132,66 @@ func (g *KeyService) getKey(keygrip []byte) (*rsa.PrivateKey, error) { return nil, nil } +// getECDSAKey returns a matching private RSA key if the keygrip matches. If a key +// is returned err will be nil. If no key is found, both values may be nil. +func (g *KeyService) getECDSAKey(keygrip []byte) (*ecdsa.PrivateKey, error) { + for _, pk := range g.privKeys { + for _, k := range pk.keys { + pubKey, ok := k.PublicKey.PublicKey.(*ecdsa.PublicKey) + if !ok { + continue + } + pubKeygrip, err := KeygripECDSA(pubKey) + if err != nil { + return nil, fmt.Errorf("couldn't get ECDSA keygrip: %v", err) + } + if !bytes.Equal(keygrip, pubKeygrip) { + continue + } + err = g.decryptPrivateKey(k, + fmt.Sprintf("%s (%s) <%s>", + pk.uid.Name, pk.uid.Comment, pk.uid.Email)) + if err != nil { + return nil, err + } + privKey, ok := k.PrivateKey.(*ecdsa.PrivateKey) + if !ok { + return nil, fmt.Errorf("not an ECDSA key %s: %v", + k.KeyIdString(), err) + } + return privKey, nil + } + } + return nil, nil +} + // GetSigner returns a crypto.Signer associated with the given keygrip. func (g *KeyService) GetSigner(keygrip []byte) (crypto.Signer, error) { - rsaPrivKey, err := g.getKey(keygrip) + rsaPrivKey, err := g.getRSAKey(keygrip) if err != nil { - return nil, fmt.Errorf("couldn't getKey: %v", err) + return nil, fmt.Errorf("couldn't getRSAKey: %v", err) } - return &RSAKey{rsa: rsaPrivKey}, nil + if rsaPrivKey != nil { + return &RSAKey{rsa: rsaPrivKey}, nil + } + ecdsaPrivKey, err := g.getECDSAKey(keygrip) + if err != nil { + return nil, fmt.Errorf("couldn't getECDSAKey: %v", err) + } + if ecdsaPrivKey != nil { + return ecdsaPrivKey, nil + } + return nil, fmt.Errorf("couldn't get signer for keygrip %X", keygrip) } // GetDecrypter returns a crypto.Decrypter associated with the given keygrip. func (g *KeyService) GetDecrypter(keygrip []byte) (crypto.Decrypter, error) { - rsaPrivKey, err := g.getKey(keygrip) + rsaPrivKey, err := g.getRSAKey(keygrip) if err != nil { - return nil, fmt.Errorf("couldn't getKey: %v", err) + return nil, fmt.Errorf("couldn't getRSAKey: %v", err) + } + if rsaPrivKey == nil { + return nil, fmt.Errorf("couldn't get decrypter for keygrip %X", keygrip) } return &RSAKey{rsa: rsaPrivKey}, nil } diff --git a/internal/keyservice/gpg/rsakey.go b/internal/keyservice/gpg/rsakey.go index 1ba1b6e..db5d6c3 100644 --- a/internal/keyservice/gpg/rsakey.go +++ b/internal/keyservice/gpg/rsakey.go @@ -7,13 +7,21 @@ import ( "math/big" ) -// RSAKey represents a GPG loaded from a keyfile. +// RSAKey represents a GPG key loaded from a keyfile. // It implements the crypto.Decrypter and crypto.Signer interfaces. type RSAKey struct { rsa *rsa.PrivateKey } // Decrypt performs RSA decryption as per gpg-agent. +// +// Terrible things about this function (not exhaustive): +// * rolling my own crypto +// * makes well-known RSA implementation mistakes +// * RSA in 2021 +// +// I'd love to not have to do this, but hey, it's for gnupg compatibility. +// Get in touch if you know how to improve this function. func (k *RSAKey) Decrypt(_ io.Reader, ciphertext []byte, _ crypto.DecrypterOpts) ([]byte, error) { c := new(big.Int) From 676c673b97ec3ef52ef1f389e1dbbd5d5faae42b Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Fri, 13 Aug 2021 00:20:28 +0800 Subject: [PATCH 14/20] feat: implement subkey signing support --- cmd/piv-agent/serve.go | 2 +- internal/assuan/assuan.go | 48 ++++++++++++++++++++-------------- internal/assuan/fsm.go | 8 ++++++ internal/assuan/readkey.go | 4 +-- internal/securitykey/string.go | 1 + internal/server/gpg.go | 4 +-- 6 files changed, 42 insertions(+), 25 deletions(-) diff --git a/cmd/piv-agent/serve.go b/cmd/piv-agent/serve.go index bb875de..cd66c97 100644 --- a/cmd/piv-agent/serve.go +++ b/cmd/piv-agent/serve.go @@ -80,7 +80,7 @@ func (cmd *ServeCmd) Run(log *zap.Logger) error { if err != nil { log.Warn("couldn't determine $HOME", zap.Error(err)) } - fallbackKeys := filepath.Join(home, ".gnupg", "piv-agent.secring.gpg") + fallbackKeys := filepath.Join(home, ".gnupg", "piv-agent.secring") if _, ok := cmd.AgentTypes["gpg"]; ok { log.Debug("starting GPG server") g.Go(func() error { diff --git a/internal/assuan/assuan.go b/internal/assuan/assuan.go index cb92fef..6b8f780 100644 --- a/internal/assuan/assuan.go +++ b/internal/assuan/assuan.go @@ -33,7 +33,7 @@ var ciphertextRegex = regexp.MustCompile( // It returns a *fsm.Machine configured in the ready state. func New(rw io.ReadWriter, log *zap.Logger, ks ...KeyService) *Assuan { var keyFound bool - var keygrip, signature []byte + var signature []byte var keygrips, hash [][]byte assuan := Assuan{ reader: bufio.NewReader(rw), @@ -86,25 +86,7 @@ func New(rw io.ReadWriter, log *zap.Logger, ks ...KeyService) *Assuan { _, err = io.WriteString(rw, "No_Secret_Key\n") } case keyinfo: - // KEYINFO arguments are a list of keygrips - // if _any_ key is available, we return info, otherwise - // No_Secret_Key. - keygrips, err = hexDecode(assuan.data...) - if err != nil { - return fmt.Errorf("couldn't decode keygrips: %v", err) - } - keyFound, keygrip, err = haveKey(ks, keygrips) - if err != nil { - _, _ = io.WriteString(rw, "ERR 1 couldn't match keygrip\n") - return fmt.Errorf("couldn't match keygrip: %v", err) - } - if keyFound { - _, err = io.WriteString(rw, - fmt.Sprintf("S KEYINFO %s D - - - - - - -\nOK\n", - strings.ToUpper(hex.EncodeToString(keygrip)))) - } else { - _, err = io.WriteString(rw, "No_Secret_Key\n") - } + err = doKeyinfo(rw, assuan.data, ks) case scd: // ignore scdaemon requests _, err = io.WriteString(rw, "ERR 100696144 No such device \n") @@ -213,6 +195,8 @@ func New(rw io.ReadWriter, log *zap.Logger, ks ...KeyService) *Assuan { return fmt.Errorf("couldn't write newline: %v", err) } _, err = io.WriteString(rw, "OK\n") + case keyinfo: + err = doKeyinfo(rw, assuan.data, ks) default: return fmt.Errorf("unknown event: %v", Event(e)) } @@ -315,6 +299,30 @@ func New(rw io.ReadWriter, log *zap.Logger, ks ...KeyService) *Assuan { return &assuan } +// doKeyinfo checks for key availability by keygrip, writing the result to rw. +func doKeyinfo(rw io.ReadWriter, data [][]byte, ks []KeyService) error { + // KEYINFO arguments are a list of keygrips + // if _any_ key is available, we return info, otherwise + // No_Secret_Key. + keygrips, err := hexDecode(data...) + if err != nil { + return fmt.Errorf("couldn't decode keygrips: %v", err) + } + keyFound, keygrip, err := haveKey(ks, keygrips) + if err != nil { + _, _ = io.WriteString(rw, "ERR 1 couldn't match keygrip\n") + return fmt.Errorf("couldn't match keygrip: %v", err) + } + if keyFound { + _, err = io.WriteString(rw, + fmt.Sprintf("S KEYINFO %s D - - - - - - -\nOK\n", + strings.ToUpper(hex.EncodeToString(keygrip)))) + return err + } + _, err = io.WriteString(rw, "No_Secret_Key\n") + return err +} + // haveKey returns true if any of the keygrips refer to keys known locally, and // false otherwise. // It takes keygrips in raw byte format, so keygrip in hex-encoded form must diff --git a/internal/assuan/fsm.go b/internal/assuan/fsm.go index 48c28d2..443fade 100644 --- a/internal/assuan/fsm.go +++ b/internal/assuan/fsm.go @@ -134,6 +134,14 @@ var assuanTransitions = []fsm.Transition{ Src: fsm.State(hashIsSet), Event: fsm.Event(pksign), Dst: fsm.State(hashIsSet), + }, { + Src: fsm.State(hashIsSet), + Event: fsm.Event(keyinfo), + Dst: fsm.State(hashIsSet), + }, { + Src: fsm.State(hashIsSet), + Event: fsm.Event(reset), + Dst: fsm.State(connected), }, // decrypting transitions { diff --git a/internal/assuan/readkey.go b/internal/assuan/readkey.go index c658c66..5d7ffa9 100644 --- a/internal/assuan/readkey.go +++ b/internal/assuan/readkey.go @@ -21,14 +21,14 @@ func readKeyData(pub crypto.PublicKey) (string, error) { ei.SetInt64(int64(k.E)) e := ei.Bytes() // prefix the key with a null byte for compatibility - return fmt.Sprintf("D (10:public-key(3:rsa(1:n%d:\x00%s)(1:e%d:%s)))\n", + return fmt.Sprintf("D (10:public-key(3:rsa(1:n%d:\x00%s)(1:e%d:%s)))\nOK\n", nLen+1, n, len(e), e), nil case *ecdsa.PublicKey: switch k.Curve { case elliptic.P256(): q := elliptic.Marshal(k.Curve, k.X, k.Y) return fmt.Sprintf( - "D (10:public-key(3:ecc(5:curve10:NIST P-256)(1:q%d:%s)))\n", + "D (10:public-key(3:ecc(5:curve10:NIST P-256)(1:q%d:%s)))\nOK\n", len(q), q), nil default: return "", fmt.Errorf("unsupported curve: %T", k.Curve) diff --git a/internal/securitykey/string.go b/internal/securitykey/string.go index 57d39ee..9e5f948 100644 --- a/internal/securitykey/string.go +++ b/internal/securitykey/string.go @@ -74,6 +74,7 @@ func (k *SecurityKey) synthesizeEntities(name, email string) ([]Entity, error) { CreationTime: now, SigType: packet.SigTypePositiveCert, // TODO: determine the key type + // TODO: support ECDH PubKeyAlgo: packet.PubKeyAlgoECDSA, Hash: crypto.SHA256, IssuerKeyId: &pub.KeyId, diff --git a/internal/server/gpg.go b/internal/server/gpg.go index 4494672..9cde7ca 100644 --- a/internal/server/gpg.go +++ b/internal/server/gpg.go @@ -48,8 +48,8 @@ func (g *GPG) Serve(ctx context.Context, l net.Listener, exit *time.Ticker, } // reset the exit timer exit.Reset(timeout) - // if the client stops responding for 60 seconds, give up. - if err := conn.SetDeadline(time.Now().Add(60 * time.Second)); err != nil { + // if the client stops responding for 300 seconds, give up. + if err := conn.SetDeadline(time.Now().Add(300 * time.Second)); err != nil { return fmt.Errorf("couldn't set deadline: %v", err) } // init protocol state machine From cac25d344fb1b43642bca402db6b76f74cb8de40 Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Fri, 13 Aug 2021 20:59:48 +0800 Subject: [PATCH 15/20] fix: avoid overly verbose stack traces on warn level logs --- cmd/piv-agent/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/piv-agent/main.go b/cmd/piv-agent/main.go index 6afbd3c..80e2c79 100644 --- a/cmd/piv-agent/main.go +++ b/cmd/piv-agent/main.go @@ -28,7 +28,7 @@ func main() { var log *zap.Logger var err error if cli.Debug { - log, err = zap.NewDevelopment() + log, err = zap.NewDevelopment(zap.AddStacktrace(zap.ErrorLevel)) } else { log, err = zap.NewProduction() } From 8f3d1876a719e908fc816652ce8bd9533a8b2e7e Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Sat, 14 Aug 2021 23:24:41 +0800 Subject: [PATCH 16/20] fix: update .socket and .service files These now support gpg-agent and ssh-agent functionality. --- deploy/piv-agent.service | 2 +- deploy/piv-agent.socket | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/deploy/piv-agent.service b/deploy/piv-agent.service index ceb88f9..4a644d3 100644 --- a/deploy/piv-agent.service +++ b/deploy/piv-agent.service @@ -2,4 +2,4 @@ Description=piv-agent service [Service] -ExecStart=%h/go/bin/piv-agent serve --debug +ExecStart=piv-agent serve --debug --agent-types=ssh=0;gpg=1 diff --git a/deploy/piv-agent.socket b/deploy/piv-agent.socket index 5d18c7b..3487545 100644 --- a/deploy/piv-agent.socket +++ b/deploy/piv-agent.socket @@ -1,8 +1,9 @@ [Unit] -Description=piv-agent socket +Description=piv-agent socket activation [Socket] ListenStream=%t/piv-agent/ssh.socket +ListenStream=%t/gnupg/S.gpg-agent [Install] WantedBy=sockets.target From 66fe955165bdb4bcf2e7ba5329a087e826f88a59 Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Sat, 14 Aug 2021 23:25:25 +0800 Subject: [PATCH 17/20] chore: update README --- README.md | 243 +++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 196 insertions(+), 47 deletions(-) diff --git a/README.md b/README.md index 031ed2b..c29d904 100644 --- a/README.md +++ b/README.md @@ -5,78 +5,143 @@ ## About -An SSH agent which you can use with your PIV smartcard / security key. +* `piv-agent` is a replacement for `ssh-agent` and `gpg-agent` which you can use with your smartcard or security key that implements [PIV](https://csrc.nist.gov/projects/piv/piv-standards-and-supporting-documentation) (e.g. a [Yubikey](https://developers.yubico.com/yubico-piv-tool/YubiKey_PIV_introduction.html)). +* `piv-agent` originated as a reimplementation of [yubikey-agent](https://github.com/FiloSottile/yubikey-agent) because I wanted a couple of extra features and also to gain a better understanding of the PIV applet on security keys, and the Go [`x/crypto/ssh/agent`](https://pkg.go.dev/golang.org/x/crypto/ssh/agent) package. It has since grown in features (good) and complexity (bad). +* `piv-agent` is built on Go standard library and supplementary `crypto` packages, as well as [`piv-go`](https://github.com/go-piv/piv-go/) and [`pcsclite`](https://pcsclite.apdu.fr/). Thanks for the great software! -`piv-agent` is based almost entirely on ideas and cryptography from https://github.com/FiloSottile/yubikey-agent. +--- +**DISCLAIMER** -**IMPORTANT NOTE**: I am not a cryptographer and I make no assertion about the security or otherwise of this software. +I make no assertion about the security or otherwise of this software and I am not a cryptographer. +If you are, please take a look at the code and send PRs or issues. :green_heart: -### What is wrong with yubikey-agent? +--- -Nothing! -I just wanted to gain a better understanding of how the PIV applet on security keys works, and how the Go ssh-agent library works. -I also added a couple of features that I wanted that yubikey-agent lacks, such as: +### Some features of piv-agent +* implements both `ssh-agent` and `gpg-agent` functionality * support for multiple security keys * support for multiple slots in those keys * support for multiple touch policies -* a way to list existing SSH keys -* support loading key files from disk +* list existing keys on a security key in SSH and OpenPGP format * socket activation (systemd-compatible) - * as a result, automatically drop the transaction on the security key after some period of disuse + * as a result, automatically drop the transaction on the security key and cached passphrases after some period of disuse +* provides "fall-back" to traditional SSH and OpenPGP keyfiles -### Philosophy +### Design philosophy This agent should require no interaction and in general do the right thing when security keys are plugged/unplugged, laptop is power cycled, etc. It is highly opinionated: -* Only supports elliptic curve crypto * Only supports 256-bit EC keys on hardware tokens -* Only supports ed25519 ssh keys on disk (`~/.ssh/id_ed25519`) -* Assumes socket activation +* Only supports ed25519 SSH keys on disk (`~/.ssh/id_ed25519`) +* Requires socket activation + +It makes some concession to practicality with OpenPGP: + +* Supports RSA signing and decryption for OpenPGP keyfiles. + RSA OpenPGP keys are widespread and Debian in particular [only documents RSA keys](https://wiki.debian.org/Keysigning). + +It tries to strike a balance between security and usability: + +* Takes a persistent transaction on the hardware token, effectively caching the PIN. +* Caches passphrases for on-disk keys (i.e. `~/.ssh/id_ed25519`) in memory, so these only need to be provided once after the agent starts. +* After a period of inactivity (32 minutes by default) it exits, dropping both of these. + Socket activation restarts it automatically as required. ### Hardware support Tested with: -* YubiKey 5C, firmware 5.2.4 +* [YubiKey 5C](https://www.yubico.com/au/product/yubikey-5c/), firmware 5.2.4 + +Will be tested with (once it ships!): + +* [Solo V2](https://www.kickstarter.com/projects/conorpatrick/solo-v2-safety-net-against-phishing/) + +Any device implementing the SCard API (PC/SC), and supported by [`piv-go`](https://github.com/go-piv/piv-go/) / [`pcsclite`](https://pcsclite.apdu.fr/) may work. +If you have tested another device with `piv-agent` successfully, please send a PR adding it to this list. ### Platform support -Currently tested on Linux and systemd. -The macOS binaries built for releases are experimental, and not tested. +Currently tested on Linux with `systemd`. + +If you have a Mac, I'd love to add support for `launchd` socket activation. See issue https://github.com/smlx/piv-agent/issues/12. + +### Protocol / Encryption Algorithm support + +| Supported | Not Supported | Support Planned (maybe) | +| --- | --- | --- | +| ✅ | ❌ | ⏳ | + +#### ssh-agent + +| | Security Key | Keyfile | +| --- | --- | --- | +| ecdsa-sha2-nistp256 | ✅ | ❌ | +| ssh-ed25519 | ⏳ | ✅ | + + +#### gpg-agent + +| | Security Key | Keyfile | +| --- | --- | --- | +| ECDSA Sign | ✅ | ✅ | +| ECDH Encrypt | ⏳ | ❌ | +| ECDH Decrypt | ⏳ | ❌ | +| RSA Sign | ❌ | ✅ | +| RSA Encrypt | ❌ | ❌ | +| RSA Decrypt | ❌ | ✅ | ## Install ### Prerequisites -`piv-agent` uses [`piv-go`](https://github.com/go-piv/piv-go#installation), so has dependencies on [`pcsclite`](https://pcsclite.apdu.fr/). +#### Consider redundancy + +It is important to understand that if you lose access to your security key there is no way to regain the keys stored on it. +For that reason it is highly recommended that you use fallback keyfiles with `piv-agent`. + +#### Install pcsclite + +`piv-agent` has transitive dependencies through [`piv-go`](https://github.com/go-piv/piv-go#installation), on [`pcsclite`](https://pcsclite.apdu.fr/). ``` -# debian/ubuntu -sudo apt install pcscd +# debian / ubuntu +sudo apt install libpcsclite1 ``` -### `piv-agent` - -`piv-agent` currently requires systemd socket activation. -Similar configuration may be possible on macOS (see [issue #12](https://github.com/smlx/piv-agent/issues/12)) or other systems. PRs welcome! +### piv-agent -`piv-agent.service` looks for `$HOME/go/bin/piv-agent` by default. -If the binary is in a different location you'll have to edit the service file. +Download the latest [release](https://github.com/smlx/piv-agent/releases), and extract it to a temporary location. +Copy the `piv-agent` binary into your `$PATH`, and the systemd unit files to the correct location: ``` cp deploy/piv-agent.{socket,service} ~/.config/systemd/user/ systemctl --user daemon-reload -systemctl --user enable --now piv-agent.socket ``` -## Set up security key +--- +**NOTE** + +`ssh-agent` and `gpg-agent` functionality are enabled by default. +Edit the systemd unit files to disable one or the other. + +--- + +## Setup + +### Hardware + +--- +**NOTE** + +This procedure is only required once per security key, and wipes any existing keys from PIV slots. -IMPORTANT NOTE: This procedure generally is only required once per security key, and wipes any existing keys from PIV slots. +--- -By default, `piv-agent` uses three slots on your security key to set up keys with different touch policies: never required, cached (required once per transaction), and always. +By default, `piv-agent` uses three slots on your security key to set up keys with different [touch policies](https://docs.yubico.com/yesdk/users-manual/application-piv/pin-touch-policies.html): never required, cached (for 15 seconds), and always. ``` # find the name of the security keys (cards) @@ -87,11 +152,20 @@ piv-agent setup --pin=123456 --card='Yubico YubiKey FIDO+CCID 01 00' --reset-sec piv-agent list ``` -## Use +### SSH + +#### List keys -Generally, add the SSH key from the security token(s) _and_ the your key file SSH key to all services for redundancy. +List your hardware SSH keys: -### Set `SSH_AUTH_SOCK` +``` +piv-agent list +``` + +Add the SSH key with the touch policy you want from the list, to any SSH service. +It's a good idea to generate an `ed25519` keyfile and add that to all SSH services too for redundancy. + +#### Set `SSH_AUTH_SOCK` Export the `SSH_AUTH_SOCK` variable in your shell. @@ -99,41 +173,116 @@ Export the `SSH_AUTH_SOCK` variable in your shell. export SSH_AUTH_SOCK=$XDG_RUNTIME_DIR/piv-agent/ssh.socket ``` -### Prefer the SSH keys on the hardware token +#### Prefer keys on the security key By default, `ssh` will offer [keyfiles it finds on disk](https://manpages.debian.org/testing/openssh-client/ssh_config.5.en.html#IdentityFile) _before_ those from the agent. This is a problem because `piv-agent` is designed to offer keys from the hardware token first, and only fall back to local keyfiles if token keys are refused. -To get `ssh` to ignore local keyfiles and only talk to `piv-agent`, add this line to your `ssh_config`. +To get `ssh` to ignore local keyfiles and only talk to `piv-agent`, add this line to your `ssh_config`, for all hosts: ``` IdentityFile /dev/null ``` -### PIN / Passphrase caching +### GPG + +#### Import public keys + +`gpg` requires public keys to be imported for any private keys stored by the agent, so the `list` command will synthesize a public key based on the private key stored on the hardware. +This public key contains a [User ID packet](https://datatracker.ietf.org/doc/html/rfc4880#section-5.11), which must be signed by the private key, so: + +* you should provide a name and email which will be embedded in the synthesized public key +* `list --key-formats=gpg` requires a touch of the security key to perform signing on the keys associated with those slots + +``` +piv-agent list --key-formats=ssh,gpg --pgp-name='Scott Leggett' --pgp-email='scott@sl.id.au' +``` + +Paste these public keys into a `key.asc` file, and run `gpg --import key.asc`. + +#### Export fallback keys + +--- +**NOTE** + +This step requires `gpg-agent` to be running, not `piv-agent`. + +--- + +Private keys to be used by `piv-agent` must be exported to `~/.gnupg/piv-agent.secring/`: + +``` +# set umask for user-only permissions +umask 77 +mkdir -p ~/.gnupg/piv-agent.secring +gpg --export-secret-key 0xB346A434C7652C02 > ~/.gnupg/piv-agent.secring/key@example.com.gpg +``` + +#### Disable gpg-agent + +Because `piv-agent` takes over the role of `gpg-agent`, the latter should be disabled: + +* Add `no-autostart` to `~/.gnupg/gpg.conf`. +* `systemctl --user disable --now gpg-agent.socket gpg-agent.service; pkill gpg-agent` + +## Use -`piv-agent` is designed to minimise the need to store secret keys permanently in memory while also being highly usable: +Start the agent sockets, and test: -* it takes a persistent transaction on the hardware token, effectively caching the PIN. -* it also caches passphrases for on-disk keys (i.e. `~/.ssh/id_ed25519`). +``` +systemctl --user enable --now piv-agent.socket +ssh-add -l +gpg -K +``` -After a period of inactivity (32 min by default) it exits, dropping both of these. -Socket activation restarts it automatically as required. +#### PIN / Passphrase caching -If your pinentry supports storing credentials I recommend storing the PIN, but not the passphrase, as a decent usability/security tradeoff. +If your pinentry supports storing credentials I recommend storing the PIN of the security key, but not the passphrase of any fallback keys, as a decent usability/security tradeoff. This ensures that at least the encrypted key file and its passphrase aren't stored together. -It also has the advantage of ensuring that you don't forget your passphrase. +It also has the advantage of ensuring that you don't forget your keyfile passphrase. But you might forget your PIN, so maybe don't store that either if you're concerned about that possibility? 🤷 -## Build and Test +#### Add Security Key as a OpenPGP signing subkey + +--- +**NOTE** + +There is currently a [bug](https://dev.gnupg.org/T5555) in GnuPG which doesn't allow ECDSA keys to be added as subkeys correctly. +For now you need to apply the patch described in the bug report to work around this limitation. + +--- + +Adding a `piv-agent` OpenPGP key as a signing subkey of an existing OpenPGP key is a convenient way to integrate a physical Security Key with your existing `gpg` workflow. +This allows you to do things like sign `git` commits using your Yubikey, while keeping the same OpenPGP key ID. +Adding a subkey requires cross-signing, so you need to export the master secret key of your existing OpenPGP key as described above to make it available to `piv-agent`. +There are instructions for adding an existing key as a subkey [here](https://security.stackexchange.com/a/160847). + +--- +**NOTE** + +`gpg` will choose the _newest_ available subkey to perform an action. So it will automatically prefer a newly added `piv-agent` subkey over any existing keyfile subkeys, but fall back to keyfiles if e.g. the Yubikey is not plugged in. -`piv-agent` has dependencies through [`piv-go`](https://github.com/go-piv/piv-go#installation). +--- + +## Develop + +### Prerequisites + +Install build dependencies: ``` # debian/ubuntu -sudo apt install libpcsclite-dev pcscd +sudo apt install libpcsclite-dev ``` -The dbus variable is required for `pinentry` to use a graphical prompt. +### Build and test + +``` +make +``` + +### Build and test manually + +This D-Bus variable is required for `pinentry` to use a graphical prompt: ``` go build ./cmd/piv-agent && systemd-socket-activate -l /tmp/piv-agent.sock -E DBUS_SESSION_BUS_ADDRESS ./piv-agent serve --debug From 8c828c43495d7f99c4141704ffdf15cf946be440 Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Sun, 15 Aug 2021 21:27:50 +0800 Subject: [PATCH 18/20] fix: make ssh-add -l print something sensible --- internal/keyservice/piv/list.go | 4 ++-- internal/mock/mock_pivservice.go | 22 ++++++++++++++++++---- internal/securitykey/string.go | 9 +++++++-- internal/ssh/agent.go | 10 +++------- 4 files changed, 30 insertions(+), 15 deletions(-) diff --git a/internal/keyservice/piv/list.go b/internal/keyservice/piv/list.go index 7979247..bb995a2 100644 --- a/internal/keyservice/piv/list.go +++ b/internal/keyservice/piv/list.go @@ -19,8 +19,8 @@ type SecurityKey interface { AttestationCertificate() (*x509.Certificate, error) Card() string Close() error - PrivateKey(s *securitykey.SigningKey) (crypto.PrivateKey, error) - Serial() uint32 + Comment(*securitykey.SlotSpec) string + PrivateKey(*securitykey.SigningKey) (crypto.PrivateKey, error) SigningKeys() []securitykey.SigningKey StringsGPG(string, string) ([]string, error) StringsSSH() []string diff --git a/internal/mock/mock_pivservice.go b/internal/mock/mock_pivservice.go index 373ae73..f3cba20 100644 --- a/internal/mock/mock_pivservice.go +++ b/internal/mock/mock_pivservice.go @@ -79,19 +79,33 @@ func (mr *MockSecurityKeyMockRecorder) Close() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Close", reflect.TypeOf((*MockSecurityKey)(nil).Close)) } +// Comment mocks base method. +func (m *MockSecurityKey) Comment(arg0 *securitykey.SlotSpec) string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Comment", arg0) + ret0, _ := ret[0].(string) + return ret0 +} + +// Comment indicates an expected call of Comment. +func (mr *MockSecurityKeyMockRecorder) Comment(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Comment", reflect.TypeOf((*MockSecurityKey)(nil).Comment), arg0) +} + // PrivateKey mocks base method. -func (m *MockSecurityKey) PrivateKey(s *securitykey.SigningKey) (crypto.PrivateKey, error) { +func (m *MockSecurityKey) PrivateKey(arg0 *securitykey.SigningKey) (crypto.PrivateKey, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "PrivateKey", s) + ret := m.ctrl.Call(m, "PrivateKey", arg0) ret0, _ := ret[0].(crypto.PrivateKey) ret1, _ := ret[1].(error) return ret0, ret1 } // PrivateKey indicates an expected call of PrivateKey. -func (mr *MockSecurityKeyMockRecorder) PrivateKey(s interface{}) *gomock.Call { +func (mr *MockSecurityKeyMockRecorder) PrivateKey(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "PrivateKey", reflect.TypeOf((*MockSecurityKey)(nil).PrivateKey), s) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "PrivateKey", reflect.TypeOf((*MockSecurityKey)(nil).PrivateKey), arg0) } // Serial mocks base method. diff --git a/internal/securitykey/string.go b/internal/securitykey/string.go index 9e5f948..2da8b78 100644 --- a/internal/securitykey/string.go +++ b/internal/securitykey/string.go @@ -29,14 +29,19 @@ type Entity struct { SigningKey } +// Comment returns a comment suitable for e.g. the SSH public key format +func (k *SecurityKey) Comment(ss *SlotSpec) string { + return fmt.Sprintf("%v #%v, touch policy: %s", k.card, k.serial, + touchStringMap[ss.TouchPolicy]) +} + // StringsSSH returns an array of commonly formatted SSH keys as strings. func (k *SecurityKey) StringsSSH() []string { var ss []string for _, s := range k.SigningKeys() { ss = append(ss, fmt.Sprintf("%s %s\n", strings.TrimSuffix(string(ssh.MarshalAuthorizedKey(s.PubSSH)), "\n"), - fmt.Sprintf("%v #%v, touch policy: %s", k.card, k.serial, - touchStringMap[s.SlotSpec.TouchPolicy]))) + k.Comment(s.SlotSpec))) } return ss } diff --git a/internal/ssh/agent.go b/internal/ssh/agent.go index d8cbf5a..37818d4 100644 --- a/internal/ssh/agent.go +++ b/internal/ssh/agent.go @@ -71,13 +71,9 @@ func (a *Agent) securityKeyIDs() ([]*agent.Key, error) { for _, k := range securityKeys { for _, s := range k.SigningKeys() { keys = append(keys, &agent.Key{ - Format: s.PubSSH.Type(), - Blob: s.PubSSH.Marshal(), - Comment: fmt.Sprintf( - `Security Key "%s" #%d PIV Slot %x`, - s.PubSSH, - k.Serial(), - s.SlotSpec.Slot.Key), + Format: s.PubSSH.Type(), + Blob: s.PubSSH.Marshal(), + Comment: k.Comment(s.SlotSpec), }) } } From e840c236d96510c276d170c16ee5726bda80be28 Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Sun, 15 Aug 2021 21:34:04 +0800 Subject: [PATCH 19/20] chore: add deploy/* to release archive --- .goreleaser.ubuntu-latest.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.goreleaser.ubuntu-latest.yml b/.goreleaser.ubuntu-latest.yml index cdfa692..561d2a5 100644 --- a/.goreleaser.ubuntu-latest.yml +++ b/.goreleaser.ubuntu-latest.yml @@ -1,3 +1,8 @@ +archives: +- files: + - deploy/* + - LICENSE + - README.md builds: - dir: cmd/piv-agent goos: From 13f24940c5850e70034f2019e788629d932c3e2e Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Sun, 15 Aug 2021 22:26:59 +0800 Subject: [PATCH 20/20] fix: avoid ineffectual assign caught by the CI linter --- internal/assuan/assuan.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/assuan/assuan.go b/internal/assuan/assuan.go index 6b8f780..31c0389 100644 --- a/internal/assuan/assuan.go +++ b/internal/assuan/assuan.go @@ -108,12 +108,13 @@ func New(rw io.ReadWriter, log *zap.Logger, ks ...KeyService) *Assuan { _, _ = io.WriteString(rw, "ERR 1 couldn't match keygrip\n") return fmt.Errorf("couldn't match keygrip: %v", err) } - readKeyData, err := readKeyData(signer.Public()) + var data string + data, err = readKeyData(signer.Public()) if err != nil { _, _ = io.WriteString(rw, "ERR 1 couldn't get key data\n") return fmt.Errorf("couldn't get key data: %v", err) } - _, err = io.WriteString(rw, readKeyData) + _, err = io.WriteString(rw, data) case setkeydesc: // ignore this event since we don't currently use the client's // description in the prompt