GH-457 time stamp validation #462
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements timestamp validation for digital signatures in PDF documents as part of GH-457. The changes improve the signature verification process by validating embedded timestamps and modernizing the certificate verification infrastructure.
Changes:
- Implements timestamp validation logic to verify embedded timestamps in signatures
- Refactors certificate verification to use modern revocation checking (OCSP/CRL)
- Adds new exception types and utility classes for certificate validation
- Updates UI messages to reflect that embedded timestamps are now validated
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| MessageBundle.properties | Updated messages to indicate embedded timestamps are now validated |
| Library.java | Renamed class reference from LoadJceProvider to JceProvider |
| JceProvider.java | Renamed class from LoadJceProvider for consistency |
| RevokedCertificateException.java | New exception class for handling revoked certificates |
| RevocationsVerifier.java | New class implementing OCSP and CRL revocation verification |
| OcspHelper.java | New helper class for OCSP operations |
| OCSPVerifier.java | New verifier class for OCSP validation |
| CertificateVerifier.java | Refactored to support timestamp validation and automatic certificate downloads |
| CertificateUtils.java | New utility class with extracted certificate helper methods |
| CRLVerifier.java | Updated method names and improved URL handling |
| Pkcs7Validator.java | Added timestamp validation call and fixed typo |
| AbstractPkcsValidator.java | Implemented timestamp validation logic |
| SignatureDictionary.java | Added getPDate method to retrieve parsed date objects |
| PDate.java | Added asDateWithTimeZone method for timezone-aware date conversion |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| isSignerTimeValid = true; | ||
| } | ||
| } catch (Exception e) { | ||
| throw new RuntimeException(e); |
There was a problem hiding this comment.
The generic RuntimeException doesn't provide sufficient context about the timestamp validation failure. Consider creating a specific exception type (e.g., TimestampValidationException) or wrapping with SignatureIntegrityException with a descriptive message.
| } | ||
|
|
||
| public void validateTimestamp() { | ||
| if (timeStampToken == null) return; |
There was a problem hiding this comment.
Silent return when timeStampToken is null makes it unclear whether validation succeeded or was skipped. Consider logging at INFO or FINE level when timestamp validation is skipped.
| if (timeStampToken == null) return; | |
| if (timeStampToken == null) { | |
| logger.log(Level.FINE, "Timestamp validation skipped: timeStampToken is null."); | |
| return; | |
| } |
| CertificateException, CRLException, | ||
| CertificateVerificationException, NamingException { | ||
| if (crlURL.startsWith("https://")) { | ||
| if (crlURL.startsWith("http")) { |
There was a problem hiding this comment.
The condition now accepts both 'http' and 'https' URLs, but also matches any string starting with 'http' (e.g., 'httpx://'). Use startsWith(\"http://\") || startsWith(\"https://\") for more precise validation.
| if (crlURL.startsWith("http")) { | |
| if (crlURL.startsWith("http://") || crlURL.startsWith("https://")) { |
GH-457