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

x/crypto/openpgp: remove DSA support #27905

Closed
FiloSottile opened this issue Sep 27, 2018 · 10 comments
Closed

x/crypto/openpgp: remove DSA support #27905

FiloSottile opened this issue Sep 27, 2018 · 10 comments
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@FiloSottile
Copy link
Contributor

DSA is obsolete and brings unnecessary complexity to the package, which should aim for a useful, safe, limited subset of the spec. Remove support for it, so we don't have to carry its complexity into future features.

If the whole world breaks at once we can consider reverting this, so avoid merging things that would make the revert impossible for a couple weeks.

See #27883 and #27889 (comment)

@FiloSottile FiloSottile added help wanted NeedsFix The path to resolution is known, but the work has not been done. labels Sep 27, 2018
@gopherbot gopherbot added this to the Unreleased milestone Sep 27, 2018
@Merovius
Copy link
Contributor

Continuing from #27889

I disagree about complexity, as this issue proves it would have required changes all the way into the standard library to keep carrying support for DSA.

I disagree. As I mentioned #27889 itself is largely cosmetic. It has zero effect on the openpgp implementation.

if enough users would find themselves needing a fork I am open to backing out the change.

Can you quantify this? Specifically, if after you remove DSA, if I wanted to demonstrate that adding it back adds negligible complexity - what would be acceptable (say, in SLOC or %SLOC)?

I am really sorry you feel like your effort was wasted

FTR, it's not about the effort being wasted. I accept wasted effort whenever I contribute to open source. It's the fact that I demonstrably cared about this enough to put my money where my mouth is - and that leading to the thing I cared about being yanked away instead. Wasted effort would mean it had no positive outcome. Instead it's having a negative outcome, because I now have to either fork this package or ship worse code (and yes. I'll probably opt for the latter).

@FiloSottile
Copy link
Contributor Author

Complexity is not just about SLOC. OpenPGP has terrible downgrade protections (see how supporting messages without MDC at all makes it possible to attack messages with MDC), so anything we don't support is something we can't be bitten by. I believe a OpenPGP package without DSA is better than one with DSA, by our goals. I understand we disagree on this.

But if you have a practical use for DSA keys, I misunderstood, and that's a relevant data point. And if it turns out there are users out there who do (while not doing edge case work, like intentionally scanning all the keyservers) that also matters in the decision. It might be possible we can't afford to drop DSA without disrupting everyday users, and that's the case in which I'd opt to back this out.

@Merovius
Copy link
Contributor

Okay. As I said, all of this is extremely discouraging. When I started my project I was operating under the assumption that there was a solid foundation to build on and an opportunity to meaningfully contribute to the Go project by upstreaming needed fixes. TBQH, if every time I come here saying "using $feature, I discovered $bug, here is a patch" I have to fear being told "actually, let's just remove $feature" then that's not a solid foundation to build on - and it ultimately means contributing is risky.

But yeah, obviously your mind is made up :(

@ianlancetaylor
Copy link
Contributor

I don't find this issue convincing, at least not yet. We can remove features because they have unfixable security holes, but I don't see how we can remove features because they are obsolete and complex. That is why we don't add features in the first place: because once they are there we don't remove them.

@jvehent
Copy link

jvehent commented Oct 22, 2018

Adding my vote against removal because DSA is, unfortunately, still very much used in mission critical operations.

As an example, I am adding DSA support to a PKCS7 library that signs Android APKs (it's a rare occurence but some older APKs are pinned to DSA keys that can't be rotated or renewed), and not only do I need a trustworthy DSA implementation to do this, I also need the same level of functionality other algorithms have, specifically crypto.Signer.

@Merovius
Copy link
Contributor

I just opened up my gpgagent code again, hoping to finally finish that off. For that, it would be good to actually get a decision on all of this; I have to decide whether I have to code around x/crypto/openpgp's lack of DSA support (by checking for DSA keys and returning an appropriate error) or not. Personally, I would consider that check an anti-feature and would prefer not to add it, but currently the code is panicing on a type-assertion and that clearly can't be published. I updated my CL, just in case.

And yes, I am aware of the discussion to deprecate x/crypto/openpgp altogether (my opinion on that is, I assume, clear) and if it comes to that, I'll have to do something about that then. But I'd still like to know what to do now.

@FiloSottile
Copy link
Contributor Author

This is an example of part of what I hope to address with #30141: there can be a community maintained fork of x/crypto/openpgp which does not suffer from the tension between a minimalist philosophy and a legacy-packed multi-dimensional protocol.

I'll do my best to get a decision on #30141 as soon as possible, I understand limbo is worse than both sides.

@Merovius
Copy link
Contributor

I honestly don't understand why I can't just get my CL merged. ISTM that the past four months wouldn't have been different for anyone but me, if it would've been merged when I first sent it. (Assuming, of course, the code isn't bad, but ISTM that's something that would've been said in a review)

@bradfitz
Copy link
Contributor

bradfitz commented May 7, 2019

@FiloSottile, what's the latest here? The delays in closure are putting off contributors.

@FiloSottile
Copy link
Contributor Author

I mostly stepped back from making decisions on x/crypto/openpgp. Hopefully the discussion on golang-openpgp@ will develop into picking a maintainer who will shepherd proposals through the usual process.

Since this one was mostly propelled by my last attempts to clean up the package, it's unlikely it will go forward. Closing.

@golang golang locked and limited conversation to collaborators May 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

6 participants