Alternative approach at signature creation using PGPainless#56
Alternative approach at signature creation using PGPainless#56vanitasvitae wants to merge 16 commits intoeclipse-packager:masterfrom
Conversation
This factory can be used to create signing streams
Use it in RepostoryCreator
…rocessors and SignerCreators
vanitasvitae
left a comment
vanitasvitae
left a comment
There was a problem hiding this comment.
I'm happy for feedback :)
Please let me know, how you'd inject the PgpSignerCreatorFactory and PgpSignatureProcessorFactory instances via the optional dependency :)
...rc/main/java/org/eclipse/packager/rpm/signature/pgpainless/PGPainlessSignatureProcessor.java
Show resolved
Hide resolved
| case PublicKeyAlgorithmTags.DSA: // 17 | ||
| case PublicKeyAlgorithmTags.EDDSA_LEGACY: // 22 | ||
| getLogger().info("DSA: {}", value); | ||
| signature.putBlob(RpmSignatureTag.GPG, value); |
There was a problem hiding this comment.
RpmSignatureTag.GPG was missing, so I added it.
Do I understand the protocol correctly in that RsaSignatureProcessor (and analogue PGPainlessSignatureProcessor) do implement the "RPM v3 signature with the same key on the header+payload" the manual is talking about?
Again, the documentation only mentions ECDSA, not EdDSA.
There was a problem hiding this comment.
We support RPMv4, not RPMv3, so I don't think we want the older tags mentioned there.
There was a problem hiding this comment.
I mean, you can keep the constants defined, but I am not sure we want to write them in the header.
There was a problem hiding this comment.
RsaSignatureProcessor is using RpmSignatureTag.PGP (which as far as I understand is RPM v3) here.
There was a problem hiding this comment.
RsaSignatureProcessor is using RpmSignatureTag.PGP (which as far as I understand is RPM v3) here.
It depends what version of RPM we are emulating. I can find "RPMSIGTAG_PGP, RPMSIGTAG_GPG (and RPMSIGTAG_PGP5) are no longer created in >= 4.16". So, we can have both sets, but if we are going to only have one set, it should not be these.
I think the new ones only process the header, whereas the old ones processed the header together with the payload.
There was a problem hiding this comment.
I'm still confused.
To slim down the PR I'd remove the new constant again.
However, what RpmSignatureTag should I use in the PGPainlessSignatureProcessor for EdDSA/DSA signatures?
If RpmSignatureTag.PGP (used for RSA) is also no longer created in newer Rpm versions, shall I just remove the PGPainlessSignatureProcessor altogether? What about its BC counterpart (RsaSignatureProcessor)? Remove that as well? It appears it is still in use in RpmFileSignatureProcessor. Is this class also obsolete then?
There was a problem hiding this comment.
There are two types of signatures. One is header-only and one is header and payload. That makes two possible for RSA and two possible for DSA/EdDSA for a total of four signature tags.
I think it is like this:
- DSA tag: 267 type: header (RPMv4)
- RSA tag: 268 type: header (RPMv4)
- PGP tag: 1002 type: header+payload (RPMv3; deprecated)
- GPG tag: 1005 type: header+payload (RPMv3; deprecated)
There was a problem hiding this comment.
So RpmFileSignatureProcessor currently uses RPMv3 signing then (RsaSignatureProcessor uses PGP tag).
I guess I'll leave RsaSignatureProcessor untouched and also won't change RpmFileSignatureProcessor in this PR.
I'll remove PGPainlessSignatureProcessor (which would do RPMv3 signing) and instead only implement PGPainlessHeaderSignatureProcessor.
...n/java/org/eclipse/packager/rpm/signature/pgpainless/PGPainlessHeaderSignatureProcessor.java
Outdated
Show resolved
Hide resolved
|
I'm not super fond of the name of the module, so feel free to provide suggestions :) |
I would just call the directory/module pgpainless, and the artifact packager-pgpainless. |
..._signer/src/test/java/org/eclipse/packager/rpm/signature/pgpainless/AbstractSigningTest.java
Outdated
Show resolved
Hide resolved
2 participants
This PR allows to use PGPainless for OpenPGP signature creation.
It is an alternative approach to #48 which incorporates some of the feedback given.
Contrary to #48, this PR does not straight up replace the existing BC-based signature implementation, but instead introduces a factory pattern, which allows to swap out the implementation backend.
The code is not yet tested, so I'll mark it as WIP.
Still, I'd like to get some feedback to get an idea of how to proceed :)