Skip to content

Conversation

@edgarsj
Copy link
Owner

@edgarsj edgarsj commented Jan 4, 2026

  • Return INDETERMINATE status when timestamp/certificate has expired (instead of INVALID)
  • Fix RSA signature verification for non-standard DigestInfo (missing NULL in AlgorithmIdentifier) - affects old pre-Java 8 signed documents
  • Fix browser tests to handle new filelist.json format with expectedStatus

@greptile-apps
Copy link

greptile-apps bot commented Jan 4, 2026

Greptile Summary

This PR improves handling of legacy document signatures and expired timestamps by adding RSA DigestInfo workaround support and returning INDETERMINATE status for verification edge cases.

Major changes:

  • Added rsa-digestinfo-workaround.ts module that manually verifies RSA signatures with non-standard DigestInfo format (missing NULL in AlgorithmIdentifier), affecting pre-Java 8 signed documents
  • Modified verifySignedInfo to attempt fallback verification using the workaround when Web Crypto API's subtle.verify() fails for RSA signatures
  • Changed timestamp verification failure handling to return INDETERMINATE status instead of INVALID, allowing documents with expired timestamps to be distinguished from cryptographically invalid signatures
  • Updated test infrastructure to support expectedStatus field in filelist.json for testing INDETERMINATE cases
  • Added comprehensive test coverage for both browser and Node.js environments

Implementation quality:

  • The RSA workaround correctly implements modular exponentiation using BigInt in browsers and leverages Node.js native crypto for better performance
  • Constant-time hash comparison prevents timing attacks
  • The fallback only triggers after standard verification fails, maintaining backward compatibility
  • Browser tests properly use atob and Node tests use Buffer for base64 decoding

Confidence Score: 4/5

  • Safe to merge with minimal risk - addresses real compatibility issues with legacy signatures
  • The implementation is well-tested with comprehensive unit and browser tests. The RSA workaround logic is sound and uses established cryptographic patterns. However, the score is 4 rather than 5 due to the complexity of manual RSA verification and the security-critical nature of signature verification code.
  • Pay close attention to src/core/rsa-digestinfo-workaround.ts - while the implementation appears correct, any bugs in cryptographic verification code could have serious security implications

Important Files Changed

Filename Overview
src/core/rsa-digestinfo-workaround.ts New module for RSA signature verification with non-standard DigestInfo. Uses BigInt math in browser, Node crypto in Node.js. Well-documented with proper error handling.
src/core/verification.ts Added RSA DigestInfo fallback in verifySignedInfo and INDETERMINATE status for failed timestamp verification. Changes preserve backward compatibility.
tests/unit/core/rsa-digestinfo-workaround.test.ts New Node.js unit tests for RSA DigestInfo workaround. Good test coverage including edge cases (tampered data, wrong signature, invalid key).
tests-browser/rsa-digestinfo.spec.ts New browser tests for RSA DigestInfo workaround. Mirrors Node.js tests but uses browser APIs (atob, TextEncoder).

Sequence Diagram

sequenceDiagram
    participant Client
    participant verifySignature
    participant verifySignedInfo
    participant WebCrypto as Web Crypto API
    participant RSAWorkaround as verifyRsaWithNonStandardDigestInfo
    participant Fallback as BigInt/Node Crypto

    Client->>verifySignature: Verify document signature
    verifySignature->>verifySignature: Verify timestamp (if present)
    
    alt Timestamp verification fails
        verifySignature->>verifySignature: Set status = INDETERMINATE
        Note over verifySignature: Cannot establish proof of existence
    end
    
    verifySignature->>verifySignedInfo: Verify signature with public key
    verifySignedInfo->>WebCrypto: subtle.verify(algorithm, publicKey, signature, data)
    
    alt Standard verification succeeds
        WebCrypto-->>verifySignedInfo: true
        verifySignedInfo-->>verifySignature: isValid: true
    else Standard verification fails
        WebCrypto-->>verifySignedInfo: false
        
        alt RSA signature (RSASSA-PKCS1-v1_5)
            verifySignedInfo->>RSAWorkaround: Fallback verification
            
            alt Browser environment
                RSAWorkaround->>RSAWorkaround: Parse RSA key (modulus, exponent)
                RSAWorkaround->>Fallback: modPow(signature, e, n) using BigInt
                Fallback-->>RSAWorkaround: Decrypted signature block
            else Node.js environment
                RSAWorkaround->>Fallback: crypto.publicDecrypt()
                Fallback-->>RSAWorkaround: Decrypted signature block
            end
            
            RSAWorkaround->>RSAWorkaround: Extract DigestInfo (tolerates missing NULL)
            RSAWorkaround->>RSAWorkaround: Extract hash from DigestInfo
            RSAWorkaround->>RSAWorkaround: Compute expected hash
            RSAWorkaround->>RSAWorkaround: Constant-time comparison
            
            alt Non-standard DigestInfo valid
                RSAWorkaround-->>verifySignedInfo: true
                verifySignedInfo-->>verifySignature: isValid: true
            else Invalid signature
                RSAWorkaround-->>verifySignedInfo: false
                verifySignedInfo-->>verifySignature: isValid: false
            end
        else Non-RSA signature
            verifySignedInfo-->>verifySignature: isValid: false
        end
    end
    
    verifySignature-->>Client: VerificationResult with status
Loading

@edgarsj edgarsj merged commit ef4087b into main Jan 4, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants