Add support for recently published ISO extensions to digital signing #106
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JcaPEMKeyConverter, when initialised using default params, uses the current default security provider. It makes sense to make that default the actual BouncyCastle provider that's currently relevant. (It's also a necessity to deal with EdDSA keys on pre-15 JDKs)
Many fields and methods in PdfPKCS7 have names that are misleading, wrong or no longer make sense in a setting where RSA is not the only game in town. This commit renames a number of internal fields to be less confusing and updates the documentation accordingly. For compatibility reasons, the public API has not been modified.
- SHA-3 digests & signing algorithm combinations - RSASSA with PKCS #1 v1.5 padding + SHA3 validation test - Validation tests with BrainpoolP384r1, NIST-P256 for both SHA-3 and SHA-2 digests. - Ensure the digest function is pinned for Ed448 and Ed25519 regardless of what the user specifies.
ECDSA and DSA don't have a generic signature algorithm OID, it always depends on the choice of hash function. The one serialised by iText was the one for keys, not signatures. DEVSIX-4842
- EdDSA signing integration - Tests for SHA-3 and ISO/TS 32002 ECC signing
MatthiasValvekens
force-pushed
the
feature/iso-signing-extensions
branch
from
c7e9500
to
82b55e4
Compare
|
Rebased after the signature migration landed in |
This is a breaking change that renames several pieces of the signing api
with the goal to make it more consistent and easier to navigate.
Methods and classes were renamed in accordance with the following
general principles:
- To ensure consistency within the codebase, adopt the following
terminological conventions as much as possible:
- "Digest" refers to hash functions only. Examples: SHA-256,
SHA3-512.
- "Signature algorithm" refers to the signature algorithm only,
not to any associated digest function (if any). Examples: ECDSA, Ed25519.
- "Signature mechanism" refers to the combination of a signature
algorithm with a digest function (if the algorithm requires
one). Examples: SHA-256 with ECDSA, Ed25519 (since Ed25519
doesn't require any digest).
- Avoid using the term "encryption", since that is typically
understood to refer to a _reversible_ procedure meant to
ensure data _confidentiality_. Neither applies in the
signing context.
- Method name transparency:
- Getters that return an OID are suffixed with "Oid"
- Getters that return a JCA-style name are suffixed with "Name"
These are the most impactful changes:
- The algorithm getters in IExternalSignature were renamed.
- Various getters in PdfPKCS7 were renamed.
- The EncryptionAlgorithms utility class was renamed to
SignatureMechanisms.
|
Since breaking changes are currently allowed on I did this as a separate commit to make it easier to remove in case you'd prefer to go ahead with a "neutral" changeset after all, but if need be I can move it further back into the history so it goes together with the name changes I did in the private API of Side note: the choice of terminology that I'm proposing is only one of several possible alternatives. My main concern is making sure that the API uses the same convention everywhere. |
There's an issue causing certificate loading to fail when BC-FIPS is used with a CertificateFactory from the default security provider on JDK 8, when the subject key is an EdDSA key. This commit changes that by ensuring that the currently relevant BC provider is always used when constructing CertificateFactory objects in the 'sign' module. The change was applied to both BC and BCFIPS for consistency.
|
Thanks @vitali-pr ! I'll close this one then. As you may have noticed, I made a follow-up PR for the RSASSA-PSS stuff; see #108. :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
(Warning: this is a relatively big contribution, but the vast majority of the changes are local to the
signmodule)ISO published ISO/TS 32001 and ISO/TS 32002 in September and October, respectively. These technical specifications extend PDF's digital signing capabilities in the following ways:
This PR adds support for these two extensions to iText (both signing & validation) without breaking API compatibility. Here's a rough overview of the changes:
PdfPKCS7andPrivateKeySignatureand renamed a bunch of variables and private fields so it's easier to tell what they actually represent (especially now that the number of supported algorithms is rising). I didn't touch the public API other than rewriting some Javadoc snippets, but I strongly recommend doing away with the old naming scheme at the next opportunity (i.e. major version upgrade).kernelyet, so I added it.Caveats:
PdfSignerto auto-register the relevant developer extensions when they were "obviously" required, there's a minor issue for ECDSA: from withinsignDetached, there's no convenient way to tell which curve is being used to sign, and in particular whether it appears on the list in ISO/TS 32002. In those cases the caller needs to declare the relevant extension manually.PdfPKCS7and friends won't autoport... That's part of the reason why I tried to clean them up as much as humanly possible in one PR, since it's probably less effort to manually port those changes than it would be to get 3 separate PRs with manual porting through the merge pipeline.Questions, comments, requests, etc. are welcome.