diff --git a/sign/keysize.go b/sign/keysize.go new file mode 100644 index 0000000..4104d9d --- /dev/null +++ b/sign/keysize.go @@ -0,0 +1,105 @@ +package sign + +import ( + "crypto" + "crypto/ecdsa" + "crypto/ed25519" + "crypto/rsa" + "crypto/x509" + "errors" + "fmt" +) + +var ( + ErrNilSigner = errors.New("signer cannot be nil") + ErrNilPublicKey = errors.New("public key cannot be nil") + ErrNilCertificate = errors.New("certificate cannot be nil") + ErrUnsupportedKey = errors.New("unsupported key type") + ErrKeyMismatch = errors.New("signer public key does not match certificate") +) + +// SignatureSize returns the maximum signature size in bytes for the given signer. +// Do not use Certificate.SignatureAlgorithm for this - that's how the CA signed +// the cert, not the size of signatures this key produces. +func SignatureSize(signer crypto.Signer) (int, error) { + if signer == nil { + return 0, ErrNilSigner + } + + pub := signer.Public() + if pub == nil { + return 0, ErrNilPublicKey + } + + return PublicKeySignatureSize(pub) +} + +// PublicKeySignatureSize returns the maximum signature size for a public key. +func PublicKeySignatureSize(pub crypto.PublicKey) (int, error) { + if pub == nil { + return 0, ErrNilPublicKey + } + + switch k := pub.(type) { + case *rsa.PublicKey: + if k.N == nil { + return 0, fmt.Errorf("%w: RSA key has nil modulus", ErrUnsupportedKey) + } + return k.Size(), nil + + case *ecdsa.PublicKey: + if k.Curve == nil { + return 0, fmt.Errorf("%w: ECDSA key has nil curve", ErrUnsupportedKey) + } + // ECDSA signatures are DER-encoded as SEQUENCE { r INTEGER, s INTEGER } per RFC 3279 Section 2.2.3. + // Max size: 2 coords + 9 bytes overhead (SEQUENCE tag/len, two INTEGER tag/len, two padding bytes) + coordSize := (k.Curve.Params().BitSize + 7) / 8 + return 2*coordSize + 9, nil + + case ed25519.PublicKey: + return ed25519.SignatureSize, nil + + default: + return 0, fmt.Errorf("%w: %T", ErrUnsupportedKey, pub) + } +} + +// DefaultSignatureSize is the fallback for unrecognized key types. +const DefaultSignatureSize = 8192 + +// ValidateSignerCertificateMatch checks that the signer's public key matches the certificate. +func ValidateSignerCertificateMatch(signer crypto.Signer, cert *x509.Certificate) error { + if signer == nil { + return ErrNilSigner + } + if cert == nil { + return ErrNilCertificate + } + + signerPub := signer.Public() + if signerPub == nil { + return ErrNilPublicKey + } + + signerPubBytes, err := x509.MarshalPKIXPublicKey(signerPub) + if err != nil { + return fmt.Errorf("failed to marshal signer public key: %w", err) + } + + certPubBytes, err := x509.MarshalPKIXPublicKey(cert.PublicKey) + if err != nil { + return fmt.Errorf("failed to marshal certificate public key: %w", err) + } + + if len(signerPubBytes) != len(certPubBytes) { + return ErrKeyMismatch + } + + for i := range signerPubBytes { + if signerPubBytes[i] != certPubBytes[i] { + return ErrKeyMismatch + } + } + + return nil +} diff --git a/sign/keysize_test.go b/sign/keysize_test.go new file mode 100644 index 0000000..51a004b --- /dev/null +++ b/sign/keysize_test.go @@ -0,0 +1,266 @@ +package sign + +import ( + "crypto" + "crypto/ecdsa" + "crypto/ed25519" + "crypto/elliptic" + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "crypto/x509/pkix" + "errors" + "math/big" + "testing" + "time" +) + +func TestSignatureSize(t *testing.T) { + tests := []struct { + name string + keyBits int + keyType string + wantSize int + }{ + {"RSA-1024", 1024, "RSA", 128}, + {"RSA-2048", 2048, "RSA", 256}, + {"RSA-3072", 3072, "RSA", 384}, + {"RSA-4096", 4096, "RSA", 512}, + {"ECDSA-P256", 256, "ECDSA", 73}, // 2*32 + 9 = 73 (DER overhead) + {"ECDSA-P384", 384, "ECDSA", 105}, // 2*48 + 9 = 105 + {"ECDSA-P521", 521, "ECDSA", 141}, // 2*66 + 9 = 141 + {"Ed25519", 0, "Ed25519", 64}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var signer crypto.Signer + var err error + + switch tt.keyType { + case "RSA": + signer, err = rsa.GenerateKey(rand.Reader, tt.keyBits) + case "ECDSA": + var curve elliptic.Curve + switch tt.keyBits { + case 256: + curve = elliptic.P256() + case 384: + curve = elliptic.P384() + case 521: + curve = elliptic.P521() + } + signer, err = ecdsa.GenerateKey(curve, rand.Reader) + case "Ed25519": + _, signer, err = ed25519.GenerateKey(rand.Reader) + } + + if err != nil { + t.Fatalf("key generation failed: %v", err) + } + + gotSize, err := SignatureSize(signer) + if err != nil { + t.Fatalf("SignatureSize failed: %v", err) + } + + if gotSize != tt.wantSize { + t.Errorf("SignatureSize() = %d, want %d", gotSize, tt.wantSize) + } + }) + } +} + +func TestSignatureSize_Errors(t *testing.T) { + t.Run("nil signer", func(t *testing.T) { + _, err := SignatureSize(nil) + if !errors.Is(err, ErrNilSigner) { + t.Errorf("expected ErrNilSigner, got %v", err) + } + }) + + t.Run("unsupported key type", func(t *testing.T) { + _, err := PublicKeySignatureSize(struct{}{}) + if !errors.Is(err, ErrUnsupportedKey) { + t.Errorf("expected ErrUnsupportedKey, got %v", err) + } + }) + + t.Run("nil public key", func(t *testing.T) { + _, err := PublicKeySignatureSize(nil) + if !errors.Is(err, ErrNilPublicKey) { + t.Errorf("expected ErrNilPublicKey, got %v", err) + } + }) +} + +func TestPublicKeySignatureSize_RSA(t *testing.T) { + // Test that RSA public key size calculation matches key.Size() + for _, bits := range []int{1024, 2048, 3072, 4096} { + t.Run("RSA-"+string(rune('0'+bits/1000))+string(rune('0'+(bits%1000)/100))+string(rune('0'+(bits%100)/10))+string(rune('0'+bits%10)), func(t *testing.T) { + key, err := rsa.GenerateKey(rand.Reader, bits) + if err != nil { + t.Fatalf("key generation failed: %v", err) + } + + size, err := PublicKeySignatureSize(&key.PublicKey) + if err != nil { + t.Fatalf("PublicKeySignatureSize failed: %v", err) + } + + if size != key.Size() { + t.Errorf("PublicKeySignatureSize() = %d, want %d (key.Size())", size, key.Size()) + } + + if size != bits/8 { + t.Errorf("PublicKeySignatureSize() = %d, want %d (bits/8)", size, bits/8) + } + }) + } +} + +func createTestCertificate(t *testing.T, key crypto.Signer) *x509.Certificate { + t.Helper() + + template := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{ + CommonName: "Test Certificate", + }, + NotBefore: time.Now(), + NotAfter: time.Now().Add(24 * time.Hour), + KeyUsage: x509.KeyUsageDigitalSignature, + BasicConstraintsValid: true, + } + + certDER, err := x509.CreateCertificate(rand.Reader, template, template, key.Public(), key) + if err != nil { + t.Fatalf("failed to create certificate: %v", err) + } + + cert, err := x509.ParseCertificate(certDER) + if err != nil { + t.Fatalf("failed to parse certificate: %v", err) + } + + return cert +} + +func TestValidateSignerCertificateMatch(t *testing.T) { + // Generate matching pair + key1, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("failed to generate key1: %v", err) + } + cert1 := createTestCertificate(t, key1) + + // Generate mismatched pair + key2, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("failed to generate key2: %v", err) + } + + t.Run("matching keys", func(t *testing.T) { + err := ValidateSignerCertificateMatch(key1, cert1) + if err != nil { + t.Errorf("expected no error for matching keys, got %v", err) + } + }) + + t.Run("mismatched keys", func(t *testing.T) { + err := ValidateSignerCertificateMatch(key2, cert1) + if !errors.Is(err, ErrKeyMismatch) { + t.Errorf("expected ErrKeyMismatch, got %v", err) + } + }) + + t.Run("nil signer", func(t *testing.T) { + err := ValidateSignerCertificateMatch(nil, cert1) + if !errors.Is(err, ErrNilSigner) { + t.Errorf("expected ErrNilSigner, got %v", err) + } + }) + + t.Run("nil certificate", func(t *testing.T) { + err := ValidateSignerCertificateMatch(key1, nil) + if !errors.Is(err, ErrNilCertificate) { + t.Errorf("expected ErrNilCertificate, got %v", err) + } + }) +} + +func TestValidateSignerCertificateMatch_DifferentKeyTypes(t *testing.T) { + // Generate ECDSA key and certificate + ecKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("failed to generate EC key: %v", err) + } + ecCert := createTestCertificate(t, ecKey) + + // Generate RSA key + rsaKey, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("failed to generate RSA key: %v", err) + } + + t.Run("RSA signer with EC certificate", func(t *testing.T) { + err := ValidateSignerCertificateMatch(rsaKey, ecCert) + if !errors.Is(err, ErrKeyMismatch) { + t.Errorf("expected ErrKeyMismatch for mismatched key types, got %v", err) + } + }) + + t.Run("EC signer with EC certificate", func(t *testing.T) { + err := ValidateSignerCertificateMatch(ecKey, ecCert) + if err != nil { + t.Errorf("expected no error for matching EC keys, got %v", err) + } + }) +} + +// BenchmarkSignatureSize benchmarks the signature size calculation +func BenchmarkSignatureSize(b *testing.B) { + rsaKey, _ := rsa.GenerateKey(rand.Reader, 4096) + ecKey, _ := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + _, edKey, _ := ed25519.GenerateKey(rand.Reader) + + b.Run("RSA-4096", func(b *testing.B) { + for i := 0; i < b.N; i++ { + _, _ = SignatureSize(rsaKey) + } + }) + + b.Run("ECDSA-P256", func(b *testing.B) { + for i := 0; i < b.N; i++ { + _, _ = SignatureSize(ecKey) + } + }) + + b.Run("Ed25519", func(b *testing.B) { + for i := 0; i < b.N; i++ { + _, _ = SignatureSize(edKey) + } + }) +} + +// BenchmarkValidateSignerCertificateMatch benchmarks the validation function +func BenchmarkValidateSignerCertificateMatch(b *testing.B) { + rsaKey, _ := rsa.GenerateKey(rand.Reader, 2048) + + // Create a certificate manually for benchmarking + template := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: "Test"}, + NotBefore: time.Now(), + NotAfter: time.Now().Add(24 * time.Hour), + KeyUsage: x509.KeyUsageDigitalSignature, + BasicConstraintsValid: true, + } + certDER, _ := x509.CreateCertificate(rand.Reader, template, template, rsaKey.Public(), rsaKey) + cert, _ := x509.ParseCertificate(certDER) + + b.ResetTimer() + for i := 0; i < b.N; i++ { + _ = ValidateSignerCertificateMatch(rsaKey, cert) + } +} diff --git a/sign/keysizes_test.go b/sign/keysizes_test.go new file mode 100644 index 0000000..49c329c --- /dev/null +++ b/sign/keysizes_test.go @@ -0,0 +1,488 @@ +package sign + +// Comprehensive test suite for PDF signing with various key types and sizes. +// This file systematically tests the library's behavior with different +// cryptographic configurations to identify limitations and bugs. +// +// PDF Signing Cryptography Background: +// ==================================== +// In PDF signing, there are TWO separate cryptographic operations: +// +// 1. Certificate Signing (by CA): How the CA signed your certificate +// - Stored in Certificate.SignatureAlgorithm +// - Examples: SHA256-RSA, SHA384-RSA, ECDSA-SHA256 +// - This is INDEPENDENT of your key size +// +// 2. Document Signing (by you): Using your private key to sign the PDF +// - Signature size depends on YOUR key size, not the CA's algorithm +// - RSA: signature size = key size in bytes (e.g., RSA-3072 = 384 bytes) +// - ECDSA: signature size depends on curve (P-256=64, P-384=96, P-521=132 bytes) +// +// The Bug Under Investigation: +// ============================ +// The library uses Certificate.SignatureAlgorithm to estimate signature buffer size. +// This is WRONG because it confuses the CA's signing algorithm with your key size. + +import ( + "crypto" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "crypto/x509/pkix" + "fmt" + "math/big" + "os" + "testing" + "time" + + "github.com/digitorus/pdfsign/revocation" + "github.com/digitorus/pdfsign/verify" +) + +// TestKeySize represents a test case for a specific key configuration +type TestKeySize struct { + Name string + KeyType string // "RSA" or "ECDSA" + KeySize int // bits for RSA, curve size for ECDSA + ExpectedSigSize int // expected signature size in bytes + CASignatureAlgo x509.SignatureAlgorithm + AllocatedByLibrary int // what the library allocates based on CA algo +} + +// Common PDF signing scenarios in the real world +var testKeySizes = []TestKeySize{ + // ========================================================================= + // RSA Keys - Most common in enterprise PDF signing + // ========================================================================= + + // RSA-1024: Legacy, insecure, but still found in old systems + { + Name: "RSA-1024 with SHA1-RSA CA", + KeyType: "RSA", + KeySize: 1024, + ExpectedSigSize: 128, + CASignatureAlgo: x509.SHA1WithRSA, + AllocatedByLibrary: 128, + }, + { + Name: "RSA-1024 with SHA256-RSA CA", + KeyType: "RSA", + KeySize: 1024, + ExpectedSigSize: 128, + CASignatureAlgo: x509.SHA256WithRSA, + AllocatedByLibrary: 256, + }, + + // RSA-2048: Current industry standard minimum + { + Name: "RSA-2048 with SHA1-RSA CA", + KeyType: "RSA", + KeySize: 2048, + ExpectedSigSize: 256, + CASignatureAlgo: x509.SHA1WithRSA, + AllocatedByLibrary: 128, // under-allocated + }, + { + Name: "RSA-2048 with SHA256-RSA CA", + KeyType: "RSA", + KeySize: 2048, + ExpectedSigSize: 256, + CASignatureAlgo: x509.SHA256WithRSA, + AllocatedByLibrary: 256, + }, + { + Name: "RSA-2048 with SHA384-RSA CA", + KeyType: "RSA", + KeySize: 2048, + ExpectedSigSize: 256, + CASignatureAlgo: x509.SHA384WithRSA, + AllocatedByLibrary: 384, + }, + + // RSA-3072: Recommended for security beyond 2030 + { + Name: "RSA-3072 with SHA256-RSA CA", + KeyType: "RSA", + KeySize: 3072, + ExpectedSigSize: 384, + CASignatureAlgo: x509.SHA256WithRSA, + AllocatedByLibrary: 256, // under-allocated + }, + { + Name: "RSA-3072 with SHA384-RSA CA", + KeyType: "RSA", + KeySize: 3072, + ExpectedSigSize: 384, + CASignatureAlgo: x509.SHA384WithRSA, + AllocatedByLibrary: 384, + }, + { + Name: "RSA-3072 with SHA512-RSA CA", + KeyType: "RSA", + KeySize: 3072, + ExpectedSigSize: 384, + CASignatureAlgo: x509.SHA512WithRSA, + AllocatedByLibrary: 512, + }, + + // RSA-4096: High security applications + { + Name: "RSA-4096 with SHA256-RSA CA", + KeyType: "RSA", + KeySize: 4096, + ExpectedSigSize: 512, + CASignatureAlgo: x509.SHA256WithRSA, + AllocatedByLibrary: 256, // under-allocated + }, + { + Name: "RSA-4096 with SHA384-RSA CA", + KeyType: "RSA", + KeySize: 4096, + ExpectedSigSize: 512, + CASignatureAlgo: x509.SHA384WithRSA, + AllocatedByLibrary: 384, // under-allocated + }, + { + Name: "RSA-4096 with SHA512-RSA CA", + KeyType: "RSA", + KeySize: 4096, + ExpectedSigSize: 512, + CASignatureAlgo: x509.SHA512WithRSA, + AllocatedByLibrary: 512, + }, + + // ========================================================================= + // ECDSA Keys - Increasingly common, especially in modern PKI + // ========================================================================= + + // ECDSA P-256: Most widely deployed elliptic curve + { + Name: "ECDSA P-256 with ECDSA-SHA256 CA", + KeyType: "ECDSA", + KeySize: 256, + ExpectedSigSize: 64, + CASignatureAlgo: x509.ECDSAWithSHA256, + AllocatedByLibrary: 256, + }, + + // ECDSA P-384: Higher security curve + { + Name: "ECDSA P-384 with ECDSA-SHA384 CA", + KeyType: "ECDSA", + KeySize: 384, + ExpectedSigSize: 96, + CASignatureAlgo: x509.ECDSAWithSHA384, + AllocatedByLibrary: 384, + }, + + // ECDSA P-521: Highest standard NIST curve + { + Name: "ECDSA P-521 with ECDSA-SHA512 CA", + KeyType: "ECDSA", + KeySize: 521, + ExpectedSigSize: 132, + CASignatureAlgo: x509.ECDSAWithSHA512, + AllocatedByLibrary: 512, + }, +} + +// generateTestCertificate creates a self-signed certificate with the specified key +func generateTestCertificate(keyType string, keySize int, caAlgo x509.SignatureAlgorithm) (crypto.Signer, *x509.Certificate, error) { + var privateKey crypto.Signer + var err error + + // Generate the key + switch keyType { + case "RSA": + privateKey, err = rsa.GenerateKey(rand.Reader, keySize) + case "ECDSA": + var curve elliptic.Curve + switch keySize { + case 256: + curve = elliptic.P256() + case 384: + curve = elliptic.P384() + case 521: + curve = elliptic.P521() + default: + return nil, nil, fmt.Errorf("unsupported ECDSA curve size: %d", keySize) + } + privateKey, err = ecdsa.GenerateKey(curve, rand.Reader) + default: + return nil, nil, fmt.Errorf("unsupported key type: %s", keyType) + } + + if err != nil { + return nil, nil, fmt.Errorf("failed to generate key: %w", err) + } + + // Create certificate template + template := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{ + CommonName: fmt.Sprintf("Test %s-%d Certificate", keyType, keySize), + Organization: []string{"PDF Signing Test"}, + }, + NotBefore: time.Now(), + NotAfter: time.Now().Add(365 * 24 * time.Hour), + KeyUsage: x509.KeyUsageDigitalSignature, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageAny}, + BasicConstraintsValid: true, + SignatureAlgorithm: caAlgo, + } + + // Self-sign the certificate + certDER, err := x509.CreateCertificate(rand.Reader, template, template, privateKey.Public(), privateKey) + if err != nil { + return nil, nil, fmt.Errorf("failed to create certificate: %w", err) + } + + cert, err := x509.ParseCertificate(certDER) + if err != nil { + return nil, nil, fmt.Errorf("failed to parse certificate: %w", err) + } + + return privateKey, cert, nil +} + +// TestResult captures the actual test outcome +type TestResult struct { + Name string + KeyType string + KeySize int + CAAlgo string + Allocated int + Needed int + SignOK bool + VerifyOK bool + RetryTriggered bool + Error string +} + +// TestPDFSigningKeyMatrix runs the comprehensive test matrix and reports actual behavior +func TestPDFSigningKeyMatrix(t *testing.T) { + inputFilePath := "../testfiles/testfile20.pdf" + + // Verify test file exists + if _, err := os.Stat(inputFilePath); os.IsNotExist(err) { + t.Fatalf("test file not found: %s", inputFilePath) + } + + var results []TestResult + + for _, tc := range testKeySizes { + t.Run(tc.Name, func(t *testing.T) { + result := TestResult{ + Name: tc.Name, + KeyType: tc.KeyType, + KeySize: tc.KeySize, + CAAlgo: tc.CASignatureAlgo.String(), + Allocated: tc.AllocatedByLibrary, + Needed: tc.ExpectedSigSize, + } + + // Generate key and certificate + privateKey, cert, err := generateTestCertificate(tc.KeyType, tc.KeySize, tc.CASignatureAlgo) + if err != nil { + result.Error = fmt.Sprintf("cert generation failed: %v", err) + results = append(results, result) + t.Logf("SKIP: %s", result.Error) + return + } + + // Create temp file for output + tmpfile, err := os.CreateTemp("", fmt.Sprintf("pdf_sign_test_%s_%d_*.pdf", tc.KeyType, tc.KeySize)) + if err != nil { + t.Fatalf("failed to create temp file: %v", err) + } + defer func() { _ = os.Remove(tmpfile.Name()) }() + _ = tmpfile.Close() + + // Attempt to sign the PDF + signErr := SignFile(inputFilePath, tmpfile.Name(), SignData{ + Signature: SignDataSignature{ + Info: SignDataSignatureInfo{ + Name: fmt.Sprintf("%s-%d Test", tc.KeyType, tc.KeySize), + Location: "Test Location", + Reason: fmt.Sprintf("Testing %s-%d key", tc.KeyType, tc.KeySize), + ContactInfo: "test@example.com", + Date: time.Now().Local(), + }, + CertType: CertificationSignature, + DocMDPPerm: AllowFillingExistingFormFieldsAndSignaturesPerms, + }, + Signer: privateKey, + DigestAlgorithm: crypto.SHA256, + Certificate: cert, + RevocationData: revocation.InfoArchival{}, + RevocationFunction: DefaultEmbedRevocationStatusFunction, + }) + + if signErr != nil { + result.SignOK = false + result.Error = signErr.Error() + } else { + result.SignOK = true + + // Try to verify the signed PDF + signedFile, err := os.Open(tmpfile.Name()) + if err != nil { + t.Fatalf("failed to open signed file: %v", err) + } + defer func() { _ = signedFile.Close() }() + + _, verifyErr := verifyFile(signedFile) + if verifyErr != nil { + result.VerifyOK = false + // Truncate error for readability + errStr := verifyErr.Error() + if len(errStr) > 100 { + errStr = errStr[:100] + "..." + } + result.Error = errStr + } else { + result.VerifyOK = true + } + } + + results = append(results, result) + + // Report result + status := "✗ FAIL" + if result.SignOK && result.VerifyOK { + status = "✓ PASS" + } + t.Logf("%s | Key: %s-%d | CA: %s | Alloc: %d | Need: %d", + status, tc.KeyType, tc.KeySize, tc.CASignatureAlgo, tc.AllocatedByLibrary, tc.ExpectedSigSize) + if result.Error != "" { + t.Logf(" Error: %s", result.Error) + } + }) + } + + // Print summary at the end + t.Log("") + t.Log("============================================================") + t.Log("SUMMARY: PDF Signing Key Size Compatibility") + t.Log("============================================================") + t.Log("Key | CA Algorithm | Alloc | Need | Sign | Verify") + t.Log("------------|--------------|-------|------|------|-------") + for _, r := range results { + signStatus := "✗" + if r.SignOK { + signStatus = "✓" + } + verifyStatus := "✗" + if r.VerifyOK { + verifyStatus = "✓" + } + t.Logf("%-11s | %-12s | %5d | %4d | %4s | %s", + fmt.Sprintf("%s-%d", r.KeyType, r.KeySize), + r.CAAlgo, r.Allocated, r.Needed, signStatus, verifyStatus) + } +} + +// verifyFile is a helper to verify a signed PDF using the actual verify package +func verifyFile(f *os.File) (bool, error) { + _, err := f.Seek(0, 0) + if err != nil { + return false, err + } + + _, err = verify.VerifyFile(f) + if err != nil { + return false, err + } + + return true, nil +} + +// TestCertificateSizes measures actual certificate sizes for different key types +// This helps understand why the buffer calculation is failing +func TestCertificateSizes(t *testing.T) { + t.Log("Certificate Size Analysis") + t.Log("==========================") + t.Log("Larger keys = larger certificates = larger CMS structures") + t.Log("") + + keySizes := []struct { + keyType string + keySize int + algo x509.SignatureAlgorithm + }{ + {"RSA", 1024, x509.SHA256WithRSA}, + {"RSA", 2048, x509.SHA256WithRSA}, + {"RSA", 3072, x509.SHA256WithRSA}, + {"RSA", 4096, x509.SHA256WithRSA}, + {"ECDSA", 256, x509.ECDSAWithSHA256}, + {"ECDSA", 384, x509.ECDSAWithSHA384}, + {"ECDSA", 521, x509.ECDSAWithSHA512}, + } + + t.Log("| Key Type | Cert Size | PubKey Size | Raw Sig Size |") + t.Log("|-------------|-----------|-------------|--------------|") + + for _, ks := range keySizes { + privateKey, cert, err := generateTestCertificate(ks.keyType, ks.keySize, ks.algo) + if err != nil { + t.Logf("| %-11s | ERROR: %v", fmt.Sprintf("%s-%d", ks.keyType, ks.keySize), err) + continue + } + + // Get raw signature size based on key type + var sigSize int + switch k := privateKey.(type) { + case *rsa.PrivateKey: + sigSize = k.Size() + case *ecdsa.PrivateKey: + // ECDSA signature size is 2 * coordinate size (r, s values) + sigSize = (k.Curve.Params().BitSize + 7) / 8 * 2 + } + + t.Logf("| %-11s | %9d | %11d | %12d |", + fmt.Sprintf("%s-%d", ks.keyType, ks.keySize), + len(cert.Raw), + len(cert.RawSubjectPublicKeyInfo), + sigSize) + } + + t.Log("") + t.Log("Note: CMS structure size = Cert + Signature + Overhead") + t.Log("The library must allocate enough space for the entire CMS structure,") + t.Log("not just the raw signature.") +} + +// TestPDFSigningKeySizeSummary prints a summary table of all test cases +func TestPDFSigningKeySizeSummary(t *testing.T) { + t.Log("PDF Signing Key Size Compatibility Matrix") + t.Log("==========================================") + t.Log("") + t.Log("This table shows how the library allocates buffer space based on") + t.Log("Certificate.SignatureAlgorithm (CA's signing algorithm) vs what's") + t.Log("actually needed based on the certificate's public key size.") + t.Log("") + t.Log("| Key Config | Sig Size | CA Algorithm | Lib Alloc | Risk |") + t.Log("|-------------|----------|---------------|-----------|-------------|") + + for _, tc := range testKeySizes { + risk := "OK" + if tc.AllocatedByLibrary < tc.ExpectedSigSize { + risk = "UNDERALLOC" + } + t.Logf("| %-11s | %4d | %-13s | %5d | %-11s |", + fmt.Sprintf("%s-%d", tc.KeyType, tc.KeySize), + tc.ExpectedSigSize, + tc.CASignatureAlgo, + tc.AllocatedByLibrary, + risk) + } + + t.Log("") + t.Log("Legend:") + t.Log(" - Sig Size: Actual signature size produced by the key (bytes)") + t.Log(" - CA Algorithm: Algorithm used to sign the certificate (Certificate.SignatureAlgorithm)") + t.Log(" - Lib Alloc: Bytes allocated by library based on CA Algorithm") + t.Log(" - UNDERALLOC: Library allocates less than needed, may trigger retry") +} diff --git a/sign/pdfsignature.go b/sign/pdfsignature.go index 216d262..2e1f957 100644 --- a/sign/pdfsignature.go +++ b/sign/pdfsignature.go @@ -443,9 +443,29 @@ func (context *SignContext) replaceSignature() error { hex.Encode(dst, signature) if uint32(len(dst)) > context.SignatureMaxLength { - log.Println("Signature too long, retrying with increased buffer size.") - // set new base and try signing again - context.SignatureMaxLengthBase += (uint32(len(dst)) - context.SignatureMaxLength) + 1 + const maxSignatureRetries = 2 + if context.retryCount >= maxSignatureRetries { + return fmt.Errorf("signature buffer calculation failed after %d attempts: "+ + "signature size %d exceeds allocated %d bytes", + maxSignatureRetries, len(dst), context.SignatureMaxLength) + } + context.retryCount++ + + deficit := uint32(len(dst)) - context.SignatureMaxLength + padding := uint32(2) // Use 2 to ensure we add an even number (hex requires pairs) + if context.retryCount == 2 { + // Add extra padding on second retry to handle timestamp variability + padding = deficit/5 + 256 + // Ensure padding keeps total even + if (deficit+padding)%2 != 0 { + padding++ + } + } + + log.Printf("Signature too long (need %d, have %d), retry %d with +%d bytes", + len(dst), context.SignatureMaxLength, context.retryCount, deficit+padding) + + context.SignatureMaxLengthBase += deficit + padding return context.SignPDF() } diff --git a/sign/sign.go b/sign/sign.go index 4856963..48742ff 100644 --- a/sign/sign.go +++ b/sign/sign.go @@ -86,6 +86,16 @@ func (context *SignContext) SignPDF() error { context.SignData.Appearance.Page = 1 } + // Reset state that accumulates during signing (important for retry) + context.newXrefEntries = nil + context.updatedXrefEntries = nil + context.lastXrefID = 0 + context.ByteRangeValues = nil + context.NewXrefStart = 0 + context.CatalogData = CatalogData{} + context.VisualSignData = VisualSignData{} + context.InfoData = InfoData{} + context.OutputBuffer = filebuffer.New([]byte{}) // Copy old file into new buffer. @@ -111,22 +121,23 @@ func (context *SignContext) SignPDF() error { return fmt.Errorf("certificate is required") } - switch context.SignData.Certificate.SignatureAlgorithm.String() { - case "SHA1-RSA": - case "ECDSA-SHA1": - case "DSA-SHA1": - context.SignatureMaxLength += uint32(hex.EncodedLen(128)) - case "SHA256-RSA": - case "ECDSA-SHA256": - case "DSA-SHA256": - context.SignatureMaxLength += uint32(hex.EncodedLen(256)) - case "SHA384-RSA": - case "ECDSA-SHA384": - context.SignatureMaxLength += uint32(hex.EncodedLen(384)) - case "SHA512-RSA": - case "ECDSA-SHA512": - context.SignatureMaxLength += uint32(hex.EncodedLen(512)) + if context.SignData.Signer != nil { + if err := ValidateSignerCertificateMatch(context.SignData.Signer, context.SignData.Certificate); err != nil { + return fmt.Errorf("signer/certificate validation failed: %w", err) + } + } + + var sigSize int + if context.SignData.SignatureSizeOverride > 0 { + sigSize = int(context.SignData.SignatureSizeOverride) + } else { + var err error + sigSize, err = PublicKeySignatureSize(context.SignData.Certificate.PublicKey) + if err != nil { + sigSize = DefaultSignatureSize + } } + context.SignatureMaxLength += uint32(hex.EncodedLen(sigSize)) // Add size of digest algorithm twice (for file digist and signing certificate attribute) context.SignatureMaxLength += uint32(hex.EncodedLen(context.SignData.DigestAlgorithm.Size() * 2)) @@ -256,11 +267,20 @@ func (context *SignContext) SignPDF() error { return fmt.Errorf("failed to update byte range: %w", err) } + // Track retry count before replaceSignature to detect if retry occurred + retryCountBefore := context.retryCount + // Replace signature if err := context.replaceSignature(); err != nil { return fmt.Errorf("failed to replace signature: %w", err) } + // If retry occurred inside replaceSignature, the recursive SignPDF call + // already wrote the output. Skip writing to avoid duplicate content. + if context.retryCount > retryCountBefore { + return nil + } + // Write final output if _, err := context.OutputBuffer.Seek(0, 0); err != nil { return err diff --git a/sign/sign_test.go b/sign/sign_test.go index a91218f..4607bee 100644 --- a/sign/sign_test.go +++ b/sign/sign_test.go @@ -1,14 +1,20 @@ package sign import ( + "bytes" "crypto" + "crypto/rand" "crypto/rsa" "crypto/x509" + "crypto/x509/pkix" "encoding/pem" "fmt" "io" + "log" + "math/big" "os" "path/filepath" + "strings" "testing" "time" @@ -813,3 +819,260 @@ func TestVisualSignLastPage(t *testing.T) { verifySignedFile(t, tmpfile, originalFileName) } + +// TestRetryMechanismProducesValidPDF tests that when a retry is triggered +// (due to undersized buffer), the resulting PDF is still valid. +// This is a regression test for a bug where retry would corrupt PDFs because +// context state (newXrefEntries, lastXrefID, etc.) wasn't reset between attempts. +func TestRetryMechanismProducesValidPDF(t *testing.T) { + // Capture log output to verify retry happens + var logBuf bytes.Buffer + log.SetOutput(&logBuf) + defer log.SetOutput(os.Stderr) + + // Generate RSA-4096 key - produces 512-byte signatures + privateKey, err := rsa.GenerateKey(rand.Reader, 4096) + if err != nil { + t.Fatalf("failed to generate RSA-4096 key: %v", err) + } + + template := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: "Retry Test"}, + NotBefore: time.Now(), + NotAfter: time.Now().Add(24 * time.Hour), + KeyUsage: x509.KeyUsageDigitalSignature, + BasicConstraintsValid: true, + } + certDER, err := x509.CreateCertificate(rand.Reader, template, template, &privateKey.PublicKey, privateKey) + if err != nil { + t.Fatalf("failed to create certificate: %v", err) + } + cert, err := x509.ParseCertificate(certDER) + if err != nil { + t.Fatalf("failed to parse certificate: %v", err) + } + + inputFilePath := "../testfiles/testfile20.pdf" + tmpfile, err := os.CreateTemp("", "retry_test_*.pdf") + if err != nil { + t.Fatalf("failed to create temp file: %v", err) + } + defer func() { _ = os.Remove(tmpfile.Name()) }() + + // Force a retry by setting SignatureSizeOverride to simulate the old bug. + // Old bug: used Certificate.SignatureAlgorithm (SHA256=256 bits) instead of key size. + // RSA-4096 produces 512-byte signatures, but old code would allocate for 256 bytes. + // This override replicates that bug to test the retry mechanism. + err = SignFile(inputFilePath, tmpfile.Name(), SignData{ + Signature: SignDataSignature{ + Info: SignDataSignatureInfo{ + Name: "Retry Test", + Reason: "Testing retry mechanism with undersized buffer", + Date: time.Now().Local(), + }, + CertType: CertificationSignature, + DocMDPPerm: AllowFillingExistingFormFieldsAndSignaturesPerms, + }, + Signer: privateKey, + DigestAlgorithm: crypto.SHA256, + Certificate: cert, + SignatureSizeOverride: 1, // Way too small! RSA-4096 needs 512 bytes. Forces retry. + }) + if err != nil { + t.Fatalf("signing failed: %v", err) + } + + // Verify retry actually happened + logOutput := logBuf.String() + if !strings.Contains(logOutput, "Signature too long") { + t.Logf("Log output: %s", logOutput) + t.Fatal("Expected retry to be triggered but no retry log message found") + } + t.Logf("Retry was triggered: %s", logOutput) + + // Verify the PDF is valid - this will fail if retry corrupted the PDF + _, err = verify.VerifyFile(tmpfile) + if err != nil { + // Debug: show file size and content around signature + _, _ = tmpfile.Seek(0, 0) + data, _ := io.ReadAll(tmpfile) + t.Logf("Output file size: %d bytes", len(data)) + + // Find /Contents< to see signature area + contentsIdx := bytes.Index(data, []byte("/Contents<")) + if contentsIdx != -1 { + start := contentsIdx + end := contentsIdx + 200 + if end > len(data) { + end = len(data) + } + t.Logf("Signature area at offset %d: %q", contentsIdx, data[start:end]) + } else { + t.Log("Could not find /Contents<") + } + + // Show area around offset 5209 where xref says object 11 is + if len(data) > 5409 { + t.Logf("Content at offset 5209: %q", data[5209:5409]) + } + + t.Fatalf("PDF verification failed after retry: %v", err) + } +} + +// TestSignatureSizeOverride tests that SignatureSizeOverride is used when set. +func TestSignatureSizeOverride(t *testing.T) { + cert, pkey := loadCertificateAndKey(t) + inputFilePath := "../testfiles/testfile20.pdf" + + tmpfile, err := os.CreateTemp("", "sigsize_override_test_*.pdf") + if err != nil { + t.Fatalf("failed to create temp file: %v", err) + } + defer func() { _ = os.Remove(tmpfile.Name()) }() + + // Test with a valid override (larger than needed) + err = SignFile(inputFilePath, tmpfile.Name(), SignData{ + Signature: SignDataSignature{ + Info: SignDataSignatureInfo{ + Name: "Override Test", + Reason: "Testing SignatureSizeOverride", + Date: time.Now().Local(), + }, + CertType: CertificationSignature, + DocMDPPerm: AllowFillingExistingFormFieldsAndSignaturesPerms, + }, + Signer: pkey, + DigestAlgorithm: crypto.SHA256, + Certificate: cert, + SignatureSizeOverride: 512, // Override with larger size + }) + if err != nil { + t.Fatalf("failed to sign PDF with SignatureSizeOverride: %v", err) + } + + verifySignedFile(t, tmpfile, "sigsize_override_test.pdf") +} + +// TestSignatureSizeOverrideTooSmall tests that signing fails gracefully when +// SignatureSizeOverride is set too small. +func TestSignatureSizeOverrideTooSmall(t *testing.T) { + cert, pkey := loadCertificateAndKey(t) + inputFilePath := "../testfiles/testfile20.pdf" + + tmpfile, err := os.CreateTemp("", "sigsize_small_test_*.pdf") + if err != nil { + t.Fatalf("failed to create temp file: %v", err) + } + defer func() { _ = os.Remove(tmpfile.Name()) }() + + // Test with override that's too small - should trigger retry and eventually succeed + // The embedded certificate is RSA-1024 (128 bytes), so 64 is too small + err = SignFile(inputFilePath, tmpfile.Name(), SignData{ + Signature: SignDataSignature{ + Info: SignDataSignatureInfo{ + Name: "Small Override Test", + Reason: "Testing too-small SignatureSizeOverride", + Date: time.Now().Local(), + }, + CertType: CertificationSignature, + DocMDPPerm: AllowFillingExistingFormFieldsAndSignaturesPerms, + }, + Signer: pkey, + DigestAlgorithm: crypto.SHA256, + Certificate: cert, + SignatureSizeOverride: 64, // Too small - will trigger retry + }) + + // This should succeed because the retry mechanism will increase the buffer + if err != nil { + t.Fatalf("signing should have succeeded with retry, got error: %v", err) + } + + verifySignedFile(t, tmpfile, "sigsize_small_test.pdf") +} + +// TestSignPDFWithRSA3072Key tests signing with an RSA-3072 key. +// This is a regression test for a bug where the signature buffer size was +// calculated based on Certificate.SignatureAlgorithm (the algorithm used to +// sign the certificate) rather than the actual public key size. +// +// RSA-3072 produces 384-byte signatures, but a certificate signed with SHA256-RSA +// would cause the code to allocate only 256 bytes, triggering retry logic that +// could corrupt the PDF. +func TestSignPDFWithRSA3072Key(t *testing.T) { + // Generate a 3072-bit RSA key + privateKey, err := rsa.GenerateKey(rand.Reader, 3072) + if err != nil { + t.Fatalf("failed to generate RSA-3072 key: %v", err) + } + + // Create a self-signed certificate with SHA256-RSA signature algorithm + // This is the key part: the certificate is signed with SHA256-RSA, + // but the public key is RSA-3072 which produces 384-byte signatures + template := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{ + CommonName: "RSA-3072 Test Certificate", + Organization: []string{"Test"}, + }, + NotBefore: time.Now(), + NotAfter: time.Now().Add(365 * 24 * time.Hour), + KeyUsage: x509.KeyUsageDigitalSignature, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageAny}, + BasicConstraintsValid: true, + } + + certDER, err := x509.CreateCertificate(rand.Reader, template, template, &privateKey.PublicKey, privateKey) + if err != nil { + t.Fatalf("failed to create certificate: %v", err) + } + + cert, err := x509.ParseCertificate(certDER) + if err != nil { + t.Fatalf("failed to parse certificate: %v", err) + } + + // Verify our test setup: certificate should be signed with SHA256-RSA + // but have an RSA-3072 public key + if cert.SignatureAlgorithm != x509.SHA256WithRSA { + t.Fatalf("expected SHA256WithRSA signature algorithm, got %v", cert.SignatureAlgorithm) + } + if privateKey.Size() != 384 { // 3072 bits = 384 bytes + t.Fatalf("expected 384-byte key size, got %d", privateKey.Size()) + } + + // Now try to sign a PDF with this certificate + inputFilePath := "../testfiles/testfile20.pdf" + tmpfile, err := os.CreateTemp("", "rsa3072_test_*.pdf") + if err != nil { + t.Fatalf("failed to create temp file: %v", err) + } + defer func() { _ = os.Remove(tmpfile.Name()) }() + + err = SignFile(inputFilePath, tmpfile.Name(), SignData{ + Signature: SignDataSignature{ + Info: SignDataSignatureInfo{ + Name: "RSA-3072 Test", + Location: "Test Location", + Reason: "Testing RSA-3072 signature buffer size", + ContactInfo: "test@example.com", + Date: time.Now().Local(), + }, + CertType: CertificationSignature, + DocMDPPerm: AllowFillingExistingFormFieldsAndSignaturesPerms, + }, + Signer: privateKey, + DigestAlgorithm: crypto.SHA256, + Certificate: cert, + RevocationData: revocation.InfoArchival{}, + RevocationFunction: DefaultEmbedRevocationStatusFunction, + }) + if err != nil { + t.Fatalf("failed to sign PDF with RSA-3072 key: %v", err) + } + + // Verify the signed PDF + verifySignedFile(t, tmpfile, "rsa3072_test.pdf") +} diff --git a/sign/types.go b/sign/types.go index 242ffdf..569f439 100644 --- a/sign/types.go +++ b/sign/types.go @@ -35,6 +35,11 @@ type SignData struct { RevocationFunction RevocationFunction Appearance Appearance + // SignatureSizeOverride sets the raw signature size in bytes manually. + // Leave as 0 to auto-detect from the certificate's public key. + // Only needed for unsupported key types (e.g., post-quantum algorithms). + SignatureSizeOverride uint32 + objectId uint32 } @@ -112,4 +117,5 @@ type SignContext struct { lastXrefID uint32 newXrefEntries []xrefEntry updatedXrefEntries []xrefEntry + retryCount int } diff --git a/verify/utils_test.go b/verify/utils_test.go index 51c773d..4314d17 100644 --- a/verify/utils_test.go +++ b/verify/utils_test.go @@ -31,7 +31,7 @@ func TestParseDate(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { result, err := parseDate(test.input) - + if test.expected { if err != nil { t.Errorf("Expected parsing to succeed, but got error: %v", err) @@ -95,18 +95,18 @@ func TestParseKeywords(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { result := parseKeywords(test.input) - + if len(result) != len(test.expected) { t.Errorf("Expected %d keywords, got %d", len(test.expected), len(result)) return } - + for i, keyword := range result { if keyword != test.expected[i] { t.Errorf("Expected keyword %d to be %q, got %q", i, test.expected[i], keyword) } } - + t.Logf("Input: %q -> %v", test.input, result) }) }