Skip to content

proposal: x/crypto/openpgp: add support for more features #29301

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

Closed
syadav2015 opened this issue Dec 17, 2018 · 8 comments
Closed

proposal: x/crypto/openpgp: add support for more features #29301

syadav2015 opened this issue Dec 17, 2018 · 8 comments
Labels
FrozenDueToAge Proposal Proposal-Crypto Proposal related to crypto packages or other security issues
Milestone

Comments

@syadav2015
Copy link

The current implementation of openpgp/crypto package does not allow the following operations on an openpgp.Entity object:

  • Supporting embedded signature (0x19) for subkeys (x/crypto/openpgp: Creating a signing subkey with an EmbeddedSignature doesn't seem possible #23231 is logged for supporting this in openpgp.ReadEntity)
    • Proposal: Entity.AddSubKey will generate this signature if the subkey will be used for signing.
  • Support for RevocationSignatures against Primary key (0x20) and subkeys (0x28)
    • Proposal: Add new methods Entity.RevokeKey() as well as SubKey.Revoke() to generate and store revocation signatures against the corresponding key.
  • Support for certification revocation signature as defined in RFC4880 against an identity
  • Adding a new subkey to the an openpgp.Entity object
    • Proposal: Add a new API Entity.AddSubKey(canSign bool) which takes in a parameter to configure if the subkey can sign as well. The subkey generated in NewEntity() currently is an encrpt only key and new subkeys cannot be generated via the library for encryption.

P.S: I've noticed we have a lot of these functionalities are present in ReadEntity e.g supporting subkeys and revocation certificates, but there seems no way to do these operations in the library on an entity directly using the library. After going over #27889 and considering @FiloSottile's opinion to keep things simple here, it would be great if we could filter out the aforementioned proposals that we can keep in the openpgp package while the rest can be implemented in a separate package.

@gopherbot gopherbot added this to the Proposal milestone Dec 17, 2018
@rsc rsc changed the title proposal: Enhancements to crypto/openpgp package proposal: x/crypto/openpgp: add support for more features Dec 19, 2018
@rsc rsc added the Proposal-Crypto Proposal related to crypto packages or other security issues label Dec 19, 2018
@rsc
Copy link
Contributor

rsc commented Dec 19, 2018

@bradfitz says if these are small, non-breaking API changes then go ahead and send them. Otherwise please explain the changes in more detail. Thanks.

@syadav2015
Copy link
Author

@rsc, @bradfitz These are fairly small changes and do not break the existing API (rather adds new API Entity.AddSubKey(), Entity.RevokeKey(), Subkey.Revoke() and Identity.RevokeSignature()). A short explanation for the aforementioned API's is as follows:

  1. Entity.AddSubKey(config *packet.Config, canSign bool): Generates a RSA/RSA keypair based on the config paramater. Generates a 0x18 signature (RFC 4880 Section 5.2.1) and a 0x19 signature (RFC 4880 Section 5.2.1) if the subkey supports signing. Essentially factoring out L564-L584 into a new method and adding 0x19 signature support.
  2. Entity.RevokeKey(): Generates a 0x20 signature (RFC 4880 Section 5.2.1) and adds it to Entity.Revocations.
  3. Subkey.Revoke(): Generates a 0x28 signature (RFC 4880 Section 5.2.1) and stores it in Subkey.Sig.

Please let me know if any further details/ design document is required.

@syadav2015
Copy link
Author

@FiloSottile @bradfitz The code changes are pending review in https://go-review.googlesource.com/c/crypto/+/161817. The code changes are not API breaking and have breadth tests added for the newly introduced APIs. Please let me know if any more information is needed about the changes and I'll be happy to address any issues.

Thanks

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/161817 mentions this issue: openpgp: add RevokeKey, RevokeSubkey and AddSubkey methods to Entity

@syadav2015
Copy link
Author

Hi everyone,

Changes in https://go-review.googlesource.com/c/crypto/+/161817 are pending reviews since April.

Since @bradfitz is on leave right now, who else would be the right person to review and potentially merge these changes?

@ALTree
Copy link
Member

ALTree commented Aug 6, 2019

@syadav2015 The crypto/openpgp situation is... unfortunate. The bottom line is that the library is not-deprecated-but-also-somewhat-frozen and it doesn't have a maintainer in the Go team.

There's a lenghty discussion at #30141 (search for openpgp in that page). You should also read Filippo's message at https://groups.google.com/forum/#!topic/golang-openpgp/_P6AmeCmD9w

Maybe write/ping that mailing list for your proposal.

@syadav2015
Copy link
Author

@ALTree Thanks for the input. I have added the proposal to the thread for now. Let's see how it goes :)

twiss pushed a commit to ProtonMail/go-crypto that referenced this issue May 27, 2020
The existing implementation does not support operations on subkeys using
the library, so the following changes have been made to support subkey
interactions as per RFC 4880 (https://tools.ietf.org/html/rfc4880).

1. AddSubkey adds support for generating new subkeys associated with an
Entity. It also adds support for adding signing subkeys.
2. RevokeKey generates a 0x20 key revocation signature against the entity.
3. RevokeSubkey generates a 0x28 subkey revocation signature for a subkey.
4. Add Revocation reason subpacket and EmbeddedSignature subpacket to
output subpackets with corresponding tests.
5. Re-sign the embedded signatures for subkeys in entity.SerializePrivate().

Fixes golang/go#29301

Change-Id: If8ee111e825c17ccaa19e4afbac4a756671d9bf5
twiss added a commit to ProtonMail/go-crypto that referenced this issue Jun 5, 2020
…ionSubkey methods to Entity (#53)

The existing implementation does not support operations on subkeys using
the library, so the following changes have been made to support subkey
interactions as per RFC 4880 (https://tools.ietf.org/html/rfc4880).

1. AddSigningSubkey and AddEncryptionSubkey add support for generating
new subkeys associated with an entity.
2. RevokeKey generates a key revocation signature for an entity.
3. RevokeSubkey generates a subkey revocation signature for a subkey.
4. Add Revocation reason subpacket and EmbeddedSignature subpacket to
output subpackets with corresponding tests.
5. Re-sign the embedded signatures for subkeys in entity.SerializePrivate().

Fixes golang/go#29301

Change-Id: If8ee111e825c17ccaa19e4afbac4a756671d9bf5
@FiloSottile
Copy link
Contributor

Per the accepted #44226 proposal and due to lack of maintenance, the golang.org/x/crypto/openpgp package is now frozen and deprecated. No new changes will be accepted except for security fixes. The package will not be removed.

If this is a security issue, please email [email protected] and we will assess it and provide a fix.

If you're looking for alternatives, consider the crypto/ed25519 package for simple signatures, golang.org/x/mod/sumdb/note for inline signatures, or filippo.io/age for encryption. You can read a summary of OpenPGP issues and alternatives here.

If you are required to interoperate with OpenPGP systems and need a maintained package, we suggest considering one of multiple community forks of golang.org/x/crypto/openpgp. We don't endorse any specific one.

@golang golang locked and limited conversation to collaborators Mar 29, 2022
@rsc rsc moved this to Declined in Proposals Aug 10, 2022
@rsc rsc added this to Proposals Aug 10, 2022
@rsc rsc removed this from Proposals Oct 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge Proposal Proposal-Crypto Proposal related to crypto packages or other security issues
Projects
None yet
Development

No branches or pull requests

5 participants