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

proposal: adopt golang.org/design/cryptography-principles #32466

Closed
FiloSottile opened this issue Jun 6, 2019 · 12 comments
Closed

proposal: adopt golang.org/design/cryptography-principles #32466

FiloSottile opened this issue Jun 6, 2019 · 12 comments

Comments

@FiloSottile
Copy link
Contributor

@FiloSottile FiloSottile commented Jun 6, 2019

The Go crypto libraries are opinionated, but those opinions are not explicitly presented anywhere. I wrote https://golang.org/design/cryptography-principles as a first step towards documenting them.

I propose we adopt the principles listed in that document as the criteria for choosing what's accepted in crypto/... and golang.org/x/crypto/....

Note that it's a one-page document, so it can't be an extensive policy explaining all decisions, but aims to align and make clear the goals that we are trying to achieve.

@gopherbot gopherbot added this to the Proposal milestone Jun 6, 2019
@mdempsky
Copy link
Member

@mdempsky mdempsky commented Jun 7, 2019

Document seems reasonable to me. Some comments:

We will only accept changes when there are enough maintainer resources to ensure their (ongoing) security.

I think "changes" here probably really means "new features"? Or am I being unimaginative, and there are non-new-feature changes that demand maintainer resources too?

Also, "ensuring their (ongoing) security" is probably an impossible goal for the underlying cryptographic primitives (short of preventing cryptographers from conducting further research), since attacks only ever get stronger. Maybe this could be worded more clearly?

@FiloSottile
Copy link
Contributor Author

@FiloSottile FiloSottile commented Jun 13, 2019

I think "changes" here probably really means "new features"? Or am I being unimaginative, and there are non-new-feature changes that demand maintainer resources too?

Changes include things like a new optimized assembly implementation. Whether you call that a feature or not I guess is subjective. It can also be a rewrite of existing code.

Also, "ensuring their (ongoing) security" is probably an impossible goal for the underlying cryptographic primitives (short of preventing cryptographers from conducting further research), since attacks only ever get stronger. Maybe this could be worded more clearly?

It definitely doesn't mean ensuring there are never security issues, but that we have the resources to audit the change, to confidently make changes in the future to the code it introduces, and to handle bug and vulnerability reports about it.

I welcome wording suggestions on how to make that clearer.

@rasky
Copy link
Member

@rasky rasky commented Jun 26, 2019

I suggest to explicitly state that unsafe or potentially-unsafe or deemed-unsafe algorithms will not be accepted (if not present) or will be eventually deprecated and removed (if present). This is the single most straightforward and important knowledge point for anybody looking into Go crypto policy, and shouldn't be desumed by semantically merging statements here and there, but simply explicitly stated.

@FiloSottile
Copy link
Contributor Author

@FiloSottile FiloSottile commented Jun 27, 2019

@rasky I feel like this sentence captures the practical implication of what you are saying.

unsafe functionality, if at all available, should require explicit acknowledgement in the API

We are not going to remove InsecureSkipVerify, for example.

@eginez
Copy link

@eginez eginez commented Jun 27, 2019

@FiloSottile I like to commend you in the effort to articulate and document golang crypto principles. It gives everybody in the community a north start to work towards.

At the same time I am wondering if you have considered the option to make certain parts of the crypto packages implementations pluggable. As go's footprint grows, we are going to see it used in places that have a strict requirement on security, and even though go's crypto implementation are correct and performant, many projects are not able to use them given that it does not meet certain security goals( for example FIPS)

Go and its libraries shine because it gives a complete set of tools to build modern applications. However when more specialized needs are required, we are left with very little options that lead to curious decisions(for example maintaining a separate branch of the whole language to in order to use a different crypto implementation).

Is there space to maintain secure, safe, practical and modern cryptography libraries and at the same time give go users an easy way to opt-out(presumably at the user's risk) of the default implementation without hindering the rest of the packages?

@FiloSottile
Copy link
Contributor Author

@FiloSottile FiloSottile commented Jun 27, 2019

@eginez That is a good question and one that I periodically try to revisit. At the moment, my conclusion is still that doing so for the whole library, in such a way that it would be useful for swapping a FIPS backend, would introduce more complexity than it's worth. Complexity that would be paid by the users who do not have the niche requirements that it would help address.

On the other hand, it might make sense to make things like the whole TLS stack pluggable, since most of the forks I see are due to crypto/tls. This is a big decision though, and I wanted to wait for modules to act on it, because it might be the most appropriate way to swap implementations.

@yaronf
Copy link

@yaronf yaronf commented Jul 1, 2019

Thank you for this obviously well thought out proposal!

I am missing an explicit statement that security trumps performance for these packages. I suggest to add a sentence to Secure, "In many cases a secure implementation of cryptographic code is by necessity less performant, e.g. to avoid side-channel attacks. In these cases we prefer the secure implementation."

@securityPirate
Copy link

@securityPirate securityPirate commented Jul 3, 2019

Maybe something like that :
import "crypto"
...
crypto.secure() // return a list of secure algorthims like sha256,sha512,...
crypto.cracked() // return a list of insecure algorithems like md4,md5,...

@rsc
Copy link
Contributor

@rsc rsc commented Jul 16, 2019

To @yaronf's comment, it would be good to end above the horizontal line with a reminder that this is an ordered list and giving one real example of a case where the order matters for making a decision.

Other than that, does anyone object to accepting this proposal?

@gopherbot
Copy link

@gopherbot gopherbot commented Aug 1, 2019

Change https://golang.org/cl/188518 mentions this issue: design/cryptography-principles: underscore that the list is sorted

gopherbot pushed a commit to golang/proposal that referenced this issue Sep 30, 2019
Updates golang/go#32466

Change-Id: I5ad26960d48387b97331f050b86e2d4249c099ba
Reviewed-on: https://go-review.googlesource.com/c/proposal/+/188518
Reviewed-by: Andrew Bonventre <andybons@golang.org>
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Nov 5, 2019

What is the status of this issue? The CL seems to be merged. Is there something else to do here?

@FiloSottile
Copy link
Contributor Author

@FiloSottile FiloSottile commented Nov 6, 2019

No, I think we were waiting to hear final comments, but I think we can mark this accepted and close it.

@FiloSottile FiloSottile closed this Nov 6, 2019
@golang golang locked and limited conversation to collaborators Nov 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
9 participants