Conversation
|
Build for testing: |
|
Build for testing: |
| if err != nil { | ||
| return err | ||
| } | ||
| _, err = openpgp.ReadEntity(packet.NewReader(bytes.NewBuffer(data))) |
There was a problem hiding this comment.
GopenPGP doesn't seem to have an equivalent to ReadEntity() here, so I just went with a basic encrypt/decrypt/verify validation instead
|
While this is probably being written with good intent, I am always extra cautious if anyone suggests any changes to security/crypto related aspects. So, who can vet this? |
There was a problem hiding this comment.
Pull request overview
This pull request migrates from the deprecated golang.org/x/crypto/openpgp package to ProtonMail's GopenPGP v3 library for PGP operations. The migration addresses the deprecation of x/crypto/openpgp since 2021 and adds support for modern cryptographic algorithms like ECDSA, which are important for compatibility with recent versions of GPG.
Key changes include:
- Replacing
openpgpandgpgeezdependencies withgopenpgp/v3 - Updating PGP key generation, validation, signing, and verification logic to use the new API
- Modifying return types to use
*crypto.Keyinstead of*openpgp.Entity
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| go.mod | Updated dependencies: removed gpgeez, added gopenpgp/v3 and its transitive dependencies |
| go.sum | Added checksums for new GopenPGP v3 dependencies and updated versions of golang.org/x packages |
| internal/helpers/opgp.go | Refactored all PGP operations to use GopenPGP v3 API, including key generation, validation, signing, and signature verification |
| src/appimagetool/cli.go | Updated signature validation to work with new *crypto.Key return type and added nil check for GetEntity() result |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Let's see if Copilot can address all the points from the review in #382. |
…eedback Apply changes from PR #377 based on Copilot review comments: - Migrate from deprecated x/crypto/openpgp to GopenPGP v3 - Fix typo "armoding" → "armoring" in public key error message - Fix typo "armoding" → "armoring" in private key error message - Update comment from "entity" to "key" - Remove unnecessary io.Reader() type conversion - Update error messages to simpler version - Fix typo "occured" → "occurred" Co-authored-by: probonopd <2480569+probonopd@users.noreply.github.com>
x/crypto/openpgp has been deprecated since 2021. GopenPGP is a well maintained (pseudo-)fork with an improved API and support for modern algorithms like ECDSA. This is especially important in the context of modern versions of GPG, where this is a default
|
Build for testing: |
The library (and it's fork of the base /x/ library) are already used by a good number of projects like ProtonMail - a company who has gained quite a good reputation over the years in the privacy/security FOSS space and undergone multiple audits - also use it in their applications As for the actual usage of it in this PR, I believe most of the changes here are quite obvious refactorings that keep the same behavior (this was intentional). The only actual behavior changes are in the validation, which was explained before |
|
Thanks @getchoo. I will have a deeper look at it. But since this is a security relevant change it may take some time. Thanks for your understanding! |
|
Any updates on this? |
x/crypto/openpgp has been deprecated since 2021. GopenPGP is a well
maintained (pseudo-)fork with an improved API and support for modern
algorithms like ECDSA. This is especially important in the context of
modern versions of GPG, where this is a default