From f9a9c46965241df271da2410f00e90b1fbcd6235 Mon Sep 17 00:00:00 2001 From: drew Date: Sun, 31 May 2026 12:15:25 +0400 Subject: [PATCH] chore: use new library cardhl Signed-off-by: drew --- go.mod | 7 +- go.sum | 6 +- pgp/yubikey.go | 422 ++++---------------------------------------- pgp/yubikey_test.go | 69 +------- 4 files changed, 43 insertions(+), 461 deletions(-) diff --git a/go.mod b/go.mod index 3fde6852..f8b714d5 100644 --- a/go.mod +++ b/go.mod @@ -6,20 +6,18 @@ require ( charm.land/bubbles/v2 v2.1.0 charm.land/bubbletea/v2 v2.0.6 charm.land/lipgloss/v2 v2.0.3 - cunicu.li/go-iso7816 v0.8.8 - cunicu.li/go-openpgp-card v0.3.11 git.sr.ht/~rockorager/go-jmap v0.5.3 github.com/ProtonMail/go-crypto v1.4.1 github.com/PuerkitoBio/goquery v1.12.0 github.com/arran4/golang-ical v0.3.5 github.com/charmbracelet/x/ansi v0.11.7 - github.com/ebfe/scard v0.0.0-20241214075232-7af069cabc25 github.com/emersion/go-imap/v2 v2.0.0-beta.8 github.com/emersion/go-maildir v0.6.0 github.com/emersion/go-message v0.18.2 github.com/emersion/go-pgpmail v0.2.2 github.com/emersion/go-sasl v0.0.0-20241020182733-b788ff22d5a6 github.com/floatpane/bubble-overlay v0.0.1 + github.com/floatpane/go-openpgp-card-hl v0.0.1 github.com/floatpane/go-uds-jsonrpc v0.0.1 github.com/floatpane/jwz-go v0.0.1 github.com/floatpane/termimage v0.2.1 @@ -38,6 +36,8 @@ require ( ) require ( + cunicu.li/go-iso7816 v0.8.8 // indirect + cunicu.li/go-openpgp-card v0.3.11 // indirect github.com/andybalholm/cascadia v1.3.3 // indirect github.com/atotto/clipboard v0.1.4 // indirect github.com/aymerick/douceur v0.2.0 // indirect @@ -50,6 +50,7 @@ require ( github.com/clipperhouse/uax29/v2 v2.7.0 // indirect github.com/cloudflare/circl v1.6.3 // indirect github.com/danieljoos/wincred v1.2.3 // indirect + github.com/ebfe/scard v0.0.0-20241214075232-7af069cabc25 // indirect github.com/godbus/dbus/v5 v5.2.2 // indirect github.com/gorilla/css v1.0.1 // indirect github.com/landlock-lsm/go-landlock v0.8.1 // indirect diff --git a/go.sum b/go.sum index 70b51480..abe165b7 100644 --- a/go.sum +++ b/go.sum @@ -70,10 +70,12 @@ github.com/emersion/go-sasl v0.0.0-20241020182733-b788ff22d5a6/go.mod h1:iL2twTe github.com/emersion/go-textwrapper v0.0.0-20200911093747-65d896831594/go.mod h1:aqO8z8wPrjkscevZJFVE1wXJrLpC5LtJG7fqLOsPb2U= github.com/floatpane/bubble-overlay v0.0.1 h1:5xU8cNigDPYegvgGMfOG23fIDXhrqXPvLTaEB7uHGK4= github.com/floatpane/bubble-overlay v0.0.1/go.mod h1:Csi1byxb9L8EAb8X13XdWF5aX5YiBD5C9WEWACyGa8A= -github.com/floatpane/jwz-go v0.0.1 h1:OAl/vaUYn+/8zFR47WaCybBGoQItb1ZkFplNrmeO3ps= -github.com/floatpane/jwz-go v0.0.1/go.mod h1:1b/JE4K+LBIBGtWbFdM1BXpN6ZbGWIv7IXPRbXD4oRE= +github.com/floatpane/go-openpgp-card-hl v0.0.1 h1:1DYmzwGDb8eneZxbc/xtwjXeFY8DFL3eYnUooMT0L0w= +github.com/floatpane/go-openpgp-card-hl v0.0.1/go.mod h1:Mrx+ukCnpEpMAxyB0p8Ch2gu78Q3Ir40BxBybb2jirw= github.com/floatpane/go-uds-jsonrpc v0.0.1 h1:/sBlCXVAP9SyLWLj0wlFI07dX/SfXeUM67B4tRwK2QA= github.com/floatpane/go-uds-jsonrpc v0.0.1/go.mod h1:G/YeDIocGkPIU+uyhJ/e8ynn9wIEMIkJ74d3VUuC4rM= +github.com/floatpane/jwz-go v0.0.1 h1:OAl/vaUYn+/8zFR47WaCybBGoQItb1ZkFplNrmeO3ps= +github.com/floatpane/jwz-go v0.0.1/go.mod h1:1b/JE4K+LBIBGtWbFdM1BXpN6ZbGWIv7IXPRbXD4oRE= github.com/floatpane/termimage v0.2.1 h1:jYPBg+SSl5PHFpN96tccBYXG4ZERoJ61ALyRDJMqonU= github.com/floatpane/termimage v0.2.1/go.mod h1:j8AaqyNtVAsPgbGblXprh33G0/5/CopWWgkXi3QB09I= github.com/godbus/dbus/v5 v5.2.2 h1:TUR3TgtSVDmjiXOgAAyaZbYmIeP3DPkld3jgKGV8mXQ= diff --git a/pgp/yubikey.go b/pgp/yubikey.go index 3c780cc3..680bd381 100644 --- a/pgp/yubikey.go +++ b/pgp/yubikey.go @@ -2,285 +2,82 @@ package pgp import ( "bytes" - "crypto" "crypto/rand" - "encoding/binary" "fmt" - "io" - "math/big" - "os" - "strings" "time" - pgpcrypto "github.com/ProtonMail/go-crypto/openpgp" - "github.com/ProtonMail/go-crypto/openpgp/armor" - "github.com/ProtonMail/go-crypto/openpgp/packet" - "github.com/ebfe/scard" - - iso "cunicu.li/go-iso7816" - "cunicu.li/go-iso7816/drivers/pcsc" - "cunicu.li/go-iso7816/filter" - - openpgp "cunicu.li/go-openpgp-card" + cardhl "github.com/floatpane/go-openpgp-card-hl" ) var randRead = rand.Read -// openCard connects to the first available OpenPGP smartcard via PC/SC. -func openCard() (*openpgp.Card, error) { - ctx, err := scard.EstablishContext() - if err != nil { - return nil, fmt.Errorf( - "failed to connect to PC/SC daemon: %w\n"+ - "Make sure pcscd is running:\n"+ - " sudo systemctl enable --now pcscd.socket\n"+ - "You may also need the ccid package for USB smartcard support", - err, - ) - } - - pcscCard, err := pcsc.OpenFirstCard(ctx, filter.HasApplet(iso.AidOpenPGP), true) - if err != nil { - ctx.Release() //nolint:errcheck,gosec - return nil, fmt.Errorf( - "no OpenPGP smartcard found: %w\n"+ - "Make sure your YubiKey is plugged in and has an OpenPGP key configured", - err, - ) - } - - isoCard := iso.NewCard(pcscCard) - card, err := openpgp.NewCard(isoCard) - if err != nil { - pcscCard.Close() //nolint:errcheck,gosec - ctx.Release() //nolint:errcheck,gosec - return nil, fmt.Errorf("failed to initialize OpenPGP card: %w", err) - } - - return card, nil -} - // BuildPGPSignedMessage creates a multipart/signed MIME message using a YubiKey. // publicKeyPath is the path to the account's PGP public key file, used to read // key metadata (fingerprint, key ID, algorithm) for building a valid OpenPGP // signature packet. +// +// The card session, signature packet construction, and ASCII armoring are +// handled by github.com/floatpane/go-openpgp-card-hl; this function owns only +// the MIME multipart/signed framing on top of the detached signature. func BuildPGPSignedMessage(payload []byte, pin string, publicKeyPath string) ([]byte, error) { - card, err := openCard() + card, err := cardhl.Open() if err != nil { return nil, err } defer card.Close() //nolint:errcheck - // Verify PIN (PW1 for signing operations) - if err := card.VerifyPassword(openpgp.PW1, pin); err != nil { - return nil, fmt.Errorf("PIN verification failed: %w", err) - } - - // Get the signing private key from the card. - privKey, err := card.PrivateKey(openpgp.KeySign, nil) - if err != nil { - return nil, fmt.Errorf("failed to get signing key from card: %w", err) - } - - signer, ok := privKey.(crypto.Signer) - if !ok { - return nil, fmt.Errorf("signing key does not implement crypto.Signer") - } - - // Load the public key entity to get metadata for the signature packet - signingKey, err := loadSigningPublicKey(publicKeyPath) + // Load the public key entity to get metadata for the signature packet. + pub, err := cardhl.LoadPublicKey(publicKeyPath) if err != nil { return nil, fmt.Errorf("failed to load public key: %w", err) } - // Split payload into headers and body for MIME structure + // Split payload into headers and body for MIME structure. headers, body := splitPayload(payload) - // Build the signed body part (this is what gets hashed) + // Build the signed body part (this is what gets hashed and signed). boundary := generateMIMEBoundary() signedPart := buildSignedPart(headers, body, boundary) - // Build the OpenPGP signature packet - sigPacket, err := buildSignaturePacket(signedPart, signer, signingKey) + // Produce a detached, ASCII-armored signature over the signed part. + armoredSig, err := card.Sign(signedPart, pin, pub) if err != nil { - return nil, fmt.Errorf("failed to build signature: %w", err) - } - - // Armor the signature - armoredSig, err := armorSignature(sigPacket) - if err != nil { - return nil, fmt.Errorf("failed to armor signature: %w", err) + return nil, err } return buildMultipartSigned(headers, body, boundary, armoredSig), nil } -func generateMIMEBoundary() string { - var buf [16]byte - if n, err := randRead(buf[:]); err == nil && n == len(buf) { - return fmt.Sprintf("----=_Part_%x", buf[:]) - } - return fmt.Sprintf("----=_Part_%d", time.Now().UnixNano()) -} - -// loadSigningPublicKey reads a PGP public key file and returns the signing -// subkey's PublicKey (or the primary key if no signing subkey exists). -func loadSigningPublicKey(path string) (*packet.PublicKey, error) { - keyData, err := os.ReadFile(path) - if err != nil { - return nil, err - } - - entities, err := pgpcrypto.ReadArmoredKeyRing(bytes.NewReader(keyData)) +// VerifyYubiKeyAvailable checks if a YubiKey with OpenPGP support is connected. +func VerifyYubiKeyAvailable() error { + card, err := cardhl.Open() if err != nil { - entities, err = pgpcrypto.ReadKeyRing(bytes.NewReader(keyData)) - if err != nil { - return nil, fmt.Errorf("failed to parse PGP key: %w", err) - } - } - if len(entities) == 0 { - return nil, fmt.Errorf("no keys found in keyring") - } - - entity := entities[0] - - // Look for a signing subkey first - now := time.Now() - for _, subkey := range entity.Subkeys { - if subkey.Sig != nil && subkey.Sig.FlagsValid && subkey.Sig.FlagSign && !subkey.PublicKey.KeyExpired(subkey.Sig, now) { - return subkey.PublicKey, nil - } + return err } - - // Fall back to primary key - return entity.PrimaryKey, nil + return card.Close() } -// buildSignaturePacket creates a valid OpenPGP v4 signature packet. -func buildSignaturePacket(signedContent []byte, signer crypto.Signer, pubKey *packet.PublicKey) ([]byte, error) { - now := time.Now() - hashAlgo := crypto.SHA256 - hashAlgoID := byte(8) // SHA-256 in OpenPGP - - // Build hashed subpackets - var hashedSubpackets bytes.Buffer - - // Subpacket: signature creation time (type 2) - writeSubpacket(&hashedSubpackets, 2, func(buf *bytes.Buffer) { - ts := make([]byte, 4) - binary.BigEndian.PutUint32(ts, uint32(now.Unix())) - buf.Write(ts) - }) - - // Subpacket: issuer key ID (type 16) - writeSubpacket(&hashedSubpackets, 16, func(buf *bytes.Buffer) { - kid := make([]byte, 8) - binary.BigEndian.PutUint64(kid, pubKey.KeyId) - buf.Write(kid) - }) - - // Subpacket: issuer fingerprint (type 33) - writeSubpacket(&hashedSubpackets, 33, func(buf *bytes.Buffer) { - buf.WriteByte(byte(pubKey.Version)) - buf.Write(pubKey.Fingerprint) - }) - - // Build hash suffix (RFC 4880, Section 5.2.4) - var hashSuffix bytes.Buffer - hashSuffix.WriteByte(4) // version - hashSuffix.WriteByte(0x00) // signature type: binary - hashSuffix.WriteByte(byte(pubKey.PubKeyAlgo)) // public key algorithm - hashSuffix.WriteByte(hashAlgoID) // hash algorithm - hsLen := hashedSubpackets.Len() - hashSuffix.WriteByte(byte(hsLen >> 8)) - hashSuffix.WriteByte(byte(hsLen)) - hashSuffix.Write(hashedSubpackets.Bytes()) - - // V4 hash trailer - trailer := hashSuffix.Bytes() - var hashTrailer bytes.Buffer - hashTrailer.WriteByte(4) // version - hashTrailer.WriteByte(0xff) // marker - tLen := make([]byte, 4) - binary.BigEndian.PutUint32(tLen, uint32(len(trailer))) - hashTrailer.Write(tLen) - - // Hash the signed content + hash suffix + trailer - hasher := hashAlgo.New() - hasher.Write(signedContent) - hasher.Write(trailer) - hasher.Write(hashTrailer.Bytes()) - digest := hasher.Sum(nil) - - // Sign with the YubiKey - rawSig, err := signer.Sign(nil, digest, hashAlgo) +// GetYubiKeyInfo returns human-readable information about the connected card. +func GetYubiKeyInfo() (string, error) { + card, err := cardhl.Open() if err != nil { - return nil, fmt.Errorf("signing failed: %w", err) + return "", err } + defer card.Close() //nolint:errcheck - // Build the complete signature packet body - var body bytes.Buffer - body.Write(trailer) // version + sig type + algo + hash algo + hashed subpackets - - // Unhashed subpackets (empty) - body.WriteByte(0) - body.WriteByte(0) - - // Hash tag (first 2 bytes of digest) - body.WriteByte(digest[0]) - body.WriteByte(digest[1]) - - // Encode the signature MPIs based on algorithm - switch pubKey.PubKeyAlgo { //nolint:exhaustive - case packet.PubKeyAlgoEdDSA: - // EdDSA: raw signature is r || s, 32 bytes each - if len(rawSig) != 64 { - return nil, fmt.Errorf("unexpected EdDSA signature length: %d", len(rawSig)) - } - writeMPI(&body, rawSig[:32]) // r - writeMPI(&body, rawSig[32:]) // s - - case packet.PubKeyAlgoRSA, packet.PubKeyAlgoRSASignOnly: - // RSA: single MPI - writeMPI(&body, rawSig) - - case packet.PubKeyAlgoECDSA: - // ECDSA: card returns ASN.1 DER encoded (R, S) - r, s, err := parseASN1Signature(rawSig) - if err != nil { - return nil, fmt.Errorf("failed to parse ECDSA signature: %w", err) - } - writeMPI(&body, r) - writeMPI(&body, s) - - default: - return nil, fmt.Errorf("unsupported key algorithm: %d", pubKey.PubKeyAlgo) + info, err := card.Info() + if err != nil { + return "", err } - - // Wrap in an OpenPGP packet (new-format header) - var pkt bytes.Buffer - bodyBytes := body.Bytes() - pkt.WriteByte(0xC2) // new-format packet tag for signature (type 2) - writeNewFormatLength(&pkt, len(bodyBytes)) - pkt.Write(bodyBytes) - - return pkt.Bytes(), nil + return info.String(), nil } -// armorSignature wraps a binary OpenPGP signature in ASCII armor. -func armorSignature(sigPacket []byte) ([]byte, error) { - var buf bytes.Buffer - w, err := armor.Encode(&buf, "PGP SIGNATURE", nil) - if err != nil { - return nil, err - } - if _, err := w.Write(sigPacket); err != nil { - return nil, err - } - if err := w.Close(); err != nil { - return nil, err +func generateMIMEBoundary() string { + var buf [16]byte + if n, err := randRead(buf[:]); err == nil && n == len(buf) { + return fmt.Sprintf("----=_Part_%x", buf[:]) } - return buf.Bytes(), nil + return fmt.Sprintf("----=_Part_%d", time.Now().UnixNano()) } // splitPayload splits a MIME message into headers and body. @@ -365,158 +162,3 @@ func buildMultipartSigned(headers, body []byte, boundary string, armoredSig []by return result.Bytes() } - -// writeSubpacket writes a single OpenPGP subpacket. -func writeSubpacket(w *bytes.Buffer, typ byte, writeContent func(*bytes.Buffer)) { - var content bytes.Buffer - writeContent(&content) - length := content.Len() + 1 // +1 for type byte - if length < 192 { - w.WriteByte(byte(length)) - } else { - // Two-octet length - length -= 192 - w.WriteByte(byte(length>>8) + 192) - w.WriteByte(byte(length)) - } - w.WriteByte(typ) - w.Write(content.Bytes()) -} - -// writeMPI writes a big-endian integer as an OpenPGP MPI (2-byte bit count + data). -func writeMPI(w io.Writer, data []byte) { - // Strip leading zero bytes - for len(data) > 0 && data[0] == 0 { - data = data[1:] - } - if len(data) == 0 { - data = []byte{0} - } - bitLen := uint16((len(data)-1)*8 + bitLength(data[0])) - buf := make([]byte, 2) - binary.BigEndian.PutUint16(buf, bitLen) - w.Write(buf) //nolint:errcheck,gosec - w.Write(data) //nolint:errcheck,gosec -} - -// bitLength returns the number of significant bits in a byte. -func bitLength(b byte) int { - n := 0 - for b > 0 { - n++ - b >>= 1 - } - return n -} - -// writeNewFormatLength writes an OpenPGP new-format packet body length. -func writeNewFormatLength(w *bytes.Buffer, length int) { - switch { - case length < 192: - w.WriteByte(byte(length)) - case length < 8384: - length -= 192 - w.WriteByte(byte(length>>8) + 192) - w.WriteByte(byte(length)) - default: - w.WriteByte(255) - buf := make([]byte, 4) - binary.BigEndian.PutUint32(buf, uint32(length)) - _, _ = w.Write(buf) - } -} - -// parseASN1Signature extracts r and s from an ASN.1 DER encoded ECDSA signature. -// -// Each intermediate slice access is bounds-checked against len(der). A truncated -// or malformed signature produces a typed error rather than an index-out-of-range -// panic; the minimum-length check up front only rules out obvious runts (#613). -func parseASN1Signature(der []byte) (r, s []byte, err error) { - // ASN.1 SEQUENCE { INTEGER r, INTEGER s } - if len(der) < 6 || der[0] != 0x30 { - return nil, nil, fmt.Errorf("invalid ASN.1 signature") - } - - pos := 2 // skip SEQUENCE tag and length - - // Parse R - if pos >= len(der) || der[pos] != 0x02 { - return nil, nil, fmt.Errorf("expected INTEGER tag for R") - } - pos++ - if pos >= len(der) { - return nil, nil, fmt.Errorf("ASN.1 signature truncated before R length") - } - rLen := int(der[pos]) - pos++ - if pos+rLen > len(der) { - return nil, nil, fmt.Errorf("ASN.1 signature truncated: R length overflow") - } - rVal := new(big.Int).SetBytes(der[pos : pos+rLen]) - pos += rLen - - // Parse S - if pos >= len(der) || der[pos] != 0x02 { - return nil, nil, fmt.Errorf("expected INTEGER tag for S") - } - pos++ - if pos >= len(der) { - return nil, nil, fmt.Errorf("ASN.1 signature truncated before S length") - } - sLen := int(der[pos]) - pos++ - if pos+sLen > len(der) { - return nil, nil, fmt.Errorf("ASN.1 signature truncated: S length overflow") - } - sVal := new(big.Int).SetBytes(der[pos : pos+sLen]) - - return rVal.Bytes(), sVal.Bytes(), nil -} - -// VerifyYubiKeyAvailable checks if a YubiKey with OpenPGP support is connected. -func VerifyYubiKeyAvailable() error { - card, err := openCard() - if err != nil { - return err - } - card.Close() //nolint:errcheck,gosec - return nil -} - -// GetYubiKeyInfo returns human-readable information about the connected card. -func GetYubiKeyInfo() (string, error) { - card, err := openCard() - if err != nil { - return "", err - } - defer card.Close() //nolint:errcheck - - var info strings.Builder - - aid := card.AID - fmt.Fprintf(&info, "Manufacturer: %s\n", aid.Manufacturer) - fmt.Fprintf(&info, "Serial: %X\n", aid.Serial) - fmt.Fprintf(&info, "Version: %s\n", aid.Version) - - ch, err := card.GetCardholder() - if err == nil && ch.Name != "" { - fmt.Fprintf(&info, "Cardholder: %s\n", ch.Name) - } - - if keys := card.Keys; keys != nil { - if ki, ok := keys[openpgp.KeySign]; ok { - fmt.Fprintf(&info, "Sign Key: %s", ki.AlgAttrs) - switch ki.Status { - case openpgp.KeyGenerated: - info.WriteString(" (generated)") - case openpgp.KeyImported: - info.WriteString(" (imported)") - case openpgp.KeyNotPresent: - // no key on card - } - info.WriteString("\n") - } - } - - return info.String(), nil -} diff --git a/pgp/yubikey_test.go b/pgp/yubikey_test.go index ae574194..ddd1d12a 100644 --- a/pgp/yubikey_test.go +++ b/pgp/yubikey_test.go @@ -7,72 +7,9 @@ import ( "testing" ) -// TestParseASN1Signature_TruncatedDoesNotPanic covers the bounds-check path -// added for #613. Each input would have panicked in the original parser -// with "index out of range"; here we expect a typed error instead. -func TestParseASN1Signature_TruncatedDoesNotPanic(t *testing.T) { - cases := []struct { - name string - der []byte - wantErr string - }{ - { - // Length byte declares 0x10 bytes of R but only 1 byte follows. - name: "R length overruns buffer", - der: []byte{0x30, 0x06, 0x02, 0x10, 0xAA, 0x00}, - wantErr: "R length overflow", - }, - { - // Length byte declares 0x10 bytes of S but only 1 byte follows. - name: "S length overruns buffer", - der: []byte{0x30, 0x06, 0x02, 0x01, 0x01, 0x02, 0x10, 0xAA}, - wantErr: "S length overflow", - }, - { - // Valid R, then no S block at all. - name: "missing S after R", - der: []byte{0x30, 0x06, 0x02, 0x01, 0x01, 0x00}, - wantErr: "expected INTEGER tag for S", - }, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - // The test must not panic: the fix replaces panics with errors. - defer func() { - if r := recover(); r != nil { - t.Fatalf("parseASN1Signature panicked: %v", r) - } - }() - _, _, err := parseASN1Signature(tc.der) - if err == nil { - t.Fatalf("want error, got nil") - } - if !strings.Contains(err.Error(), tc.wantErr) { - t.Fatalf("error = %q, want it to mention %q", err.Error(), tc.wantErr) - } - }) - } -} - -// TestParseASN1Signature_WellFormed guards against regressions in the -// happy path: a minimal SEQUENCE { INTEGER, INTEGER } must still decode -// to the original r and s bytes. -func TestParseASN1Signature_WellFormed(t *testing.T) { - // SEQUENCE (6 bytes) { INTEGER 0x01, INTEGER 0x02 } - der := []byte{0x30, 0x06, 0x02, 0x01, 0x01, 0x02, 0x01, 0x02} - - r, s, err := parseASN1Signature(der) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if len(r) != 1 || r[0] != 0x01 { - t.Errorf("r = %x, want 01", r) - } - if len(s) != 1 || s[0] != 0x02 { - t.Errorf("s = %x, want 02", s) - } -} +// The OpenPGP signature packet construction and its ASN.1 / MPI helpers now +// live in github.com/floatpane/go-openpgp-card-hl and are tested there. What +// remains here is matcha's own MIME multipart/signed framing. func TestGenerateMIMEBoundaryUsesCryptoRandomBytes(t *testing.T) { oldRandRead := randRead