fix: randomize pgp boundary (#1191)

Joey Roth created 2 months ago

## What?
- Replace the YubiKey PGP multipart/signed boundary timestamp with 16
bytes from crypto/rand.
- Keep a UnixNano fallback for rare random-source failures.
- Add focused tests for the random path and fallback formatting.

## Why?
MIME boundaries should not be predictable. The existing YubiKey path
used time.Now().Unix(), while other signing paths already prefer
crypto/rand with a timestamp fallback.

Closes #729

Change summary

pgp/yubikey.go      | 13 ++++++++++++-
pgp/yubikey_test.go | 38 ++++++++++++++++++++++++++++++++++++++
2 files changed, 50 insertions(+), 1 deletion(-)

Detailed changes

pgp/yubikey.go

@@ -3,6 +3,7 @@ package pgp
 import (
 	"bytes"
 	"crypto"
+	"crypto/rand"
 	"encoding/binary"
 	"fmt"
 	"io"
@@ -23,6 +24,8 @@ import (
 	openpgp "cunicu.li/go-openpgp-card"
 )
 
+var randRead = rand.Read
+
 // openCard connects to the first available OpenPGP smartcard via PC/SC.
 func openCard() (*openpgp.Card, error) {
 	ctx, err := scard.EstablishContext()
@@ -94,7 +97,7 @@ func BuildPGPSignedMessage(payload []byte, pin string, publicKeyPath string) ([]
 	headers, body := splitPayload(payload)
 
 	// Build the signed body part (this is what gets hashed)
-	boundary := fmt.Sprintf("----=_Part_%d", time.Now().Unix())
+	boundary := generateMIMEBoundary()
 	signedPart := buildSignedPart(headers, body, boundary)
 
 	// Build the OpenPGP signature packet
@@ -112,6 +115,14 @@ func BuildPGPSignedMessage(payload []byte, pin string, publicKeyPath string) ([]
 	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) {

pgp/yubikey_test.go

@@ -1,6 +1,8 @@
 package pgp
 
 import (
+	"errors"
+	"strconv"
 	"strings"
 	"testing"
 )
@@ -72,3 +74,39 @@ func TestParseASN1Signature_WellFormed(t *testing.T) {
 		t.Errorf("s = %x, want 02", s)
 	}
 }
+
+func TestGenerateMIMEBoundaryUsesCryptoRandomBytes(t *testing.T) {
+	oldRandRead := randRead
+	defer func() { randRead = oldRandRead }()
+
+	randRead = func(p []byte) (int, error) {
+		for i := range p {
+			p[i] = byte(i)
+		}
+		return len(p), nil
+	}
+
+	got := generateMIMEBoundary()
+	want := "----=_Part_000102030405060708090a0b0c0d0e0f"
+	if got != want {
+		t.Fatalf("boundary = %q, want %q", got, want)
+	}
+}
+
+func TestGenerateMIMEBoundaryFallsBackToUnixNano(t *testing.T) {
+	oldRandRead := randRead
+	defer func() { randRead = oldRandRead }()
+
+	randRead = func(_ []byte) (int, error) {
+		return 0, errors.New("random source unavailable")
+	}
+
+	const prefix = "----=_Part_"
+	got := generateMIMEBoundary()
+	if !strings.HasPrefix(got, prefix) {
+		t.Fatalf("boundary = %q, want prefix %q", got, prefix)
+	}
+	if _, err := strconv.ParseInt(strings.TrimPrefix(got, prefix), 10, 64); err != nil {
+		t.Fatalf("fallback boundary suffix is not a UnixNano timestamp: %v", err)
+	}
+}