What is the difference between fork and original golang/crypto?

[cristaloleg]

I think everyone will be happy to see a short summary in README what was changes.

Thank you for the lib 😉

[twiss]

Hi! 👋 Here's a current list of changes:

• Expand ECC support
• Bump default and minimum key-derivation (S2K) cost parameters
• Default to AES128 and SHA256 instead of CAST5 and RIPEMD160 for keys without preferred algorithms, as required by rfc4880bis-06 (rfc4880 required 3DES and SHA1)
• Disallow generating RSA keys of less than 1024 bits (recommended by the audit)
• Fix and move Signature.KeyExpired to PublicKey.KeyExpired; add Signature.SigExpired (x/crypto/openpgp: Incorrect comparison when checking if PGP key is expired golang/go#22312)
• Detect expired signatures; though leaving it up to the caller whether they want to reject them
• Use latest-created valid self-signature instead of the last-appearing one; don't drop other self-signatures when re-serializing the key
• Add higher-level function to verify clearsigned messages, in order to be able to verify that the algorithm mentioned in the header matches the algorithm used in the signature (one of the findings of the audit)

We're hoping to merge these changes back into the original, so I won't add them to the readme for now, but I hope that's useful anyway ^.^

[cristaloleg]

Oh, that's nice, thank you for the info!

(probably leaving this issue is a good idea, so everyone else may find it easily)

[syadav2015]

Hi @twiss,

I have a bunch of enhancements to the golang/crypto package for supporting adding subkeys and generating revocation signatures (review) as well as supporting GNU dummy S2K for missing private keys used to import keyrings generated by gpg --export-secret-subkey.

However the upstream package does not have an active maintainer/reviewer causing the reviews to be in a limbo for months. Since there are plans to push changes from this fork upstream, could we sync up on the "Widening golang.org/x/crypto/openpgp maintainership" forum thread on how to keep the upstream package alive or whether this fork would be the primary place to add new features? Any comments on the existing reviews would be greatly appreciated.

[twiss]

Hey @syadav2015, thanks for reaching out!

I'll start with reviewing the S2K patch (hopefully tomorrow), as it's smaller. I'll leave any comments there and then post on the mailinglist see if we can get it merged there.

If that doesn't succeed, you could make a PR here, yeah.

[syadav2015]

Thanks, @twiss!!

More than half of the patch for https://go-review.googlesource.com/c/crypto/+/161817 is the tests for the new API's so hopefully it's still a manageable review. Looking forward to your feedback.

[twiss]

Hey @syadav2015 👋 I left a review on https://go-review.googlesource.com/c/crypto/+/191658, did you see it? It'd be nice to try to get it merged there, and we'd like to merge it here, too (and the other one as well, eventually). Although it seems like there's gonna be a merge conflict here for this one so that's gonna be a bit of extra work ^.^

[syadav2015]

Thanks for the review!! Pushed an updated patchset with resolution for comments.

[syadav2015]

Hi @twiss, I rebased my changes to resolve the merge conflict in the review and ensure the review is up to date. Please let me know if anything needs to be addressed in the reviews. I would really like to get these reviews merged if possible since there is another pull request waiting for these APIs to merge in before the PR is accepted. I really appreciate you taking time to review the changes 😄 .

[syadav2015]

@twiss Sorry for the slight delay in response. Was on leave due to medical reasons. Have updated https://go-review.googlesource.com/c/crypto/+/191658. Hopefully the latest patchset should suffice.

[twiss]

@syadav2015 No worries, hope you're well! And thanks for all the patience with all the nitpicks :) I'll send another message to the mailing list trying to get it merged, and if not, I can merge it here instead.

[ddevault]

Do you have any central location where you're tracking the progress on upstreaming your changes?

[twiss]

Hey 👋 Our latest attempt at that was here: https://groups.google.com/d/msg/golang-openpgp/_P6AmeCmD9w/sDaiFm89EgAJ We haven't received a reply there.

At this point, if @syadav2015 is willing to rebase their changes, we'd be happy to merge them here directly. (If not, I might get to it myself eventually.) We will also continue to merge changes from upstream here.

[mssun]

We (passforios: https://github.com/mssun/passforios) are using github.com/ProtonMail/gopenpgp for PGP encryption/decryption. Recently, we want to migrate to V2 APIs (mssun/passforios#373). However, the "GNU dummy S2K for missing private keys" issue bothers me. Originally, we use V1 gopenpgp library with a patch (https://github.com/mssun/passforios/blob/master/go/crypto.patch). However, for V2, I found that the forked crypto changes a lot. I cannot get the original patch work. If @syadav2015's patch for GNU dummy S2K issue can be merged. That's will be great!

[zugzwang]

Hello @mssun, thanks for reaching out!

The issue mssun/passforios#373 is closed, did you manage to solve it? And the patch you provided is not accessible from this URL: https://github.com/mssun/passforios/blob/master/go/crypto.patch