Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for OpenPGP V3 signatures #164

Open
haydentherapper opened this issue Apr 17, 2023 · 15 comments
Open

Support for OpenPGP V3 signatures #164

haydentherapper opened this issue Apr 17, 2023 · 15 comments

Comments

@haydentherapper
Copy link

I work on a project that verifies signed RPM packages, which still use OpenPGP V3 signatures. Both the Golang RPM library and our service's verifier have to use x/crypto/openpgp, but we'd like to migrate to using this library to pick up support for other key types, bug fixes, etc. Is there any interest in bringing back support for V3 signatures? We don't need generation of them, only verification.

@twiss
Copy link
Member

twiss commented Apr 17, 2023

Hey 👋 V3 signatures are really old, and even V4 signatures have been in the OpenPGP standard for almost two decades now, and a new version of the standard specifying V6 signatures is around the corner. Is there any possibility to suggest to the RPM folks to switch to V4 (or soon V6)? I think that should be tried first, otherwise we'll be stuck in the past forever.

@haydentherapper
Copy link
Author

Thanks for the context! I'm going to try to find someone at RedHat who works on RPM, though I suspect some pushback to format changes since it's an old file type. Unfortunately we might still be stuck needing to verify older RPM packages, though if the argument can be made that V3 signatures are insecure, it might be sufficient to drop support for them. Do you happen to have any documentation that discusses any security concerns with V3 vs the newer formats?

@andrewgdotcom
Copy link
Contributor

Lack of V3 support is a blocker for Hockeypuck. We need #165 but can't upgrade our dependency because of this.

@andrewgdotcom
Copy link
Contributor

I think that should be tried first, otherwise we'll be stuck in the past forever.

Until the WG actively declares V3 unsound, we are obliged to handle v3 sigs in the wild. Even after everyone stops creating new ones, there is still value in verifying older ones.

@haydentherapper
Copy link
Author

We have the same concern with RPM packages. Even if we can convince RPM to move off V3, we'll need to handle verification of existing packages.

@andrewgdotcom
Copy link
Contributor

Support was removed in 18633fd, which was nearly three years ago.

@twiss
Copy link
Member

twiss commented Apr 26, 2023

Do you happen to have any documentation that discusses any security concerns with V3 vs the newer formats?

So - one issue is that V3 signatures only allow using RSA and DSA (and not ECDSA and EdDSA). Both of those algorithms are actually deprecated in draft-ietf-openpgp-crypto-refresh-08, which makes V3 signatures essentially indirectly deprecated as well (though perhaps that should be made more explicit).

To make matters worse, in my experience, V3 signatures are usually signed by 1024-bit keys. Though it's a slightly orthogonal point, the crypto refresh says:

An implementation MUST NOT encrypt, sign or verify using RSA keys of size less than 2048 bits.

We don't comply with this yet, but will have to do so in the future. At that point, I assume you'll again have the same issue of not being able to verify old signatures, and so will need some kind of migration path anyway, I think? So I'm not sure it's worth adding support for V3 signatures just for 2048+ bit RSA V3 signatures, of which I don't think there are that many?


We have the same concern with RPM packages. Even if we can convince RPM to move off V3, we'll need to handle verification of existing packages.

Someone told me last week that RPM at least as used by Fedora did move from generating V3 signatures to V4 signatures, and RPM now shows a warning when it's been asked to generate a V3 signature, which is something at least. Hopefully the number of new V3 signatures will thus decrease.


In general, we'd prefer to nudge signers to update their software in this way, rather than add back support for V3 signatures. But, I understand of course that if you have lots of V3 signatures today, that's an issue. I think, if you temporarily create a fork of go-crypto, adding back support during the time generating software is transitioning to V4, and simultaneously try to plan for no longer validating old insecure signatures, that would be ideal, IMHO ☺️

@andrewgdotcom
Copy link
Contributor

It appears Sequoia added v3 support specifically because of this issue: https://sequoia-pgp.org/blog/2023/04/27/rpm-sequoia/

@andrewgdotcom
Copy link
Contributor

RFC4880 specifically states:

   Implementations SHOULD accept V3 signatures.  Implementations SHOULD
   generate V4 signatures.

Since RFC4880 is (still!) the latest published standard, it is totally premature to remove support for V3 IMO. We should not remove support for things that are merely deprecated (not forbidden) in a WIP draft.

I tried to re-add support for V3 in the pgpkeys-eu fork, but I ran into a dependency issue that I can't track down the root cause of:

andrewg@barney s2k % go test
# github.com/ProtonMail/go-crypto/openpgp/s2k.test
golang.org/x/crypto/blake2b.supportsAVX2: relocation target runtime.support_avx2 not defined
golang.org/x/crypto/blake2b.supportsAVX: relocation target runtime.support_avx not defined
FAIL	github.com/ProtonMail/go-crypto/openpgp/s2k [build failed]
andrewg@barney s2k %

The internet suggests this can be fixed by updating a vendor dependency, but since there is no mention of "AVX" anywhere in the code I can't track down which indirect dependency is causing it.

@twiss
Copy link
Member

twiss commented Apr 30, 2023

Since RFC4880 is (still!) the latest published standard, it is totally premature to remove support for V3 IMO.

Yeah, I understand your point, but if we were to follow RFC4880 to the letter, there are many other quite absurd things we'd have to do, such as using TripleDES when a key doesn't indicate any algorithm preferences. It also says "Implementations MUST implement SHA-1", whereas we have been trying to move away from that as much as possible. In general, we have followed rfc4880bis (and now the crypto refresh) and deviated from RFC4880 already since the inception of this package, as the aim was to modernize the cryptography being used. We could probably make it more explicit in the README and project description that this is the goal, but I don't think we should change that goal back to following RFC4880 now.


Looking at pgpkeys-eu@3286d90, I think the issue is here (for example) - back when this change was made, this package was a fork of golang.org/x/crypto/openpgp, whereas now it's a standalone package; so references to golang.org/x/crypto/openpgp should be replaced by github.com/ProtonMail/go-crypto/openpgp everywhere.

@andrewgdotcom
Copy link
Contributor

3286d90 is two commits behind HEAD, I discovered the current issue while attempting to fix those stale dependencies. Current HEAD is pgpkeys-eu@f24b301

@twiss
Copy link
Member

twiss commented May 2, 2023

Ah, sorry, I looked at that as it was linked from #55. Running go test ./... in that branch, I don't get the same error, so I'm not sure what could be the cause, but I do get a bunch of other errors, related to checkSignatureDetails, which has been added recently to fix some missing security checks. You may need to add handling for (or a special version of the function for) v3 signatures there.

@rolandshoemaker
Copy link
Contributor

@andrewgdotcom you're running into golang/go#25098 which was fixed by golang/crypto@ae8bce0, likely combined with usage of an older version of Go. What is your go version?

@andrewgdotcom
Copy link
Contributor

andrewgdotcom commented May 3, 2023

@rolandshoemaker I'm using go1.18.3 darwin/amd64. I've opened a PR on my side to see what happens when the standard tests run on github, and they don't display the dependency issue (they have other issues of course!). I've cleaned my module cache locally but it doesn't help...

[EDIT] it seems there's something misconfigured in my go workspace, using a clean workspace got rid of the issue. Sorry for the noise.

@andrewgdotcom
Copy link
Contributor

andrewgdotcom commented May 6, 2023

https://github.com/pgpkeys-eu/go-crypto now passes all the tests (including the restored V3 tests)

Run the following commands in your project to use the soft fork:

go mod edit -replace github.com/ProtonMail/go-crypto=github.com/pgpkeys-eu/go-crypto@40edd8c9dfc3
go mod tidy

(You can replace the 40edd8c9dfc3 with any more recent git commit ID)

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

No branches or pull requests

4 participants