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: crypto/tls: implement RFC7627 #43922

Open
brawer opened this issue Jan 26, 2021 · 16 comments
Open

proposal: crypto/tls: implement RFC7627 #43922

brawer opened this issue Jan 26, 2021 · 16 comments
Assignees
Labels
Proposal Proposal-Crypto Proposal related to crypto packages or other security issues Proposal-Hold
Milestone

Comments

@brawer
Copy link

brawer commented Jan 26, 2021

I’m not a crypto expert, but couldn’t Go’s TLS stack un-deprecate ConnectionState.TLSUnique by implementing RFC7627? In terms of API, when a TLS connection doesn’t use RFC7627, the TLSUnique field might be cleared. See also #18346.

@gopherbot gopherbot added this to the Proposal milestone Jan 26, 2021
@seankhliao seankhliao added the Proposal-Crypto Proposal related to crypto packages or other security issues label Jan 26, 2021
@ianlancetaylor ianlancetaylor added this to Incoming in Proposals (old) Feb 17, 2021
@ianlancetaylor
Copy link
Contributor

ianlancetaylor commented Feb 17, 2021

CC @FiloSottile

@bskidmore
Copy link

bskidmore commented Oct 1, 2021

Is there an update on this potential issue?

@bskidmore
Copy link

bskidmore commented Oct 21, 2021

Is there an update on this issue?

@rsc
Copy link
Contributor

rsc commented Oct 27, 2021

/cc @FiloSottile and also @agl (because he co-authored the RFC)

@rsc
Copy link
Contributor

rsc commented Oct 27, 2021

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc rsc moved this from Incoming to Active in Proposals (old) Oct 27, 2021
@FiloSottile
Copy link
Contributor

FiloSottile commented Oct 27, 2021

I am not sure what the deployment status of RFC 7627 is. I will look into it and try to figure out if it's worth the complexity.

ConnectionState.TLSUnique is unlikely to become useful though unless we can reject non-RFC 7627 connections, because clearing or setting it to nil would be a backwards compatibility breakage. I also don't want to expose something wonky and unsafe like a RFC7627InUse or TLSUniqueIsSafe bool.

What do you need TLSUnique for that is not served by ConnectionState.ExportKeyingMaterial by the way? The latter is available in TLS 1.3 and it returns an error, so if we need to we can make it degrade more gracefully.

@brawer
Copy link
Author

brawer commented Oct 28, 2021

The motivation for filling this proposal was to make it possible to support SCRAM with channel binding. SCRAM is an authentication method that avoids the transmission and storage of passwords, which is quite interesting from a security perspective. SCRAM is available (at least in the specification) as an authentication method for all SASL-based application protocols, such as SMTP, IMAP, POP, LDAP, and others. For XMPP, use of SCRAM is mandatory; and there's also some minor other, non-SASL protocols with SCRAM authentication. RFC7677 defines two authentication methods for SASL, SCRAM-SHA256-PLUS and SCRAM-SHA256 (with and without channel binding), and similar specs exist for other hash functions. Unfortunately, without a channel binding mechanism, SCRAM is vulnerable to meddler-in-the-middle attacks. Therefore, only the SCRAM-*-PLUS authentication schemes (ie. with channel binding) are actually secure.

On TLS 1.2, channel binding is done with the value of TLSUnique, whose security depends on RFC7627 being used —;hence this proposal here. As of October 2021, Channel bindings for TLS 1.3 are still going through the IETF standardization process, so TLS 1.3 is not yet an option for SCRAM-*-PLUS.

If implementing RFC7627 in TLS 1.2 turns out to be too much work (too risky, too painful to be bothered, whatever), Go could perhaps also just do nothing: Just wait until TLS 1.3 has a channel binding mechanism, and then expose the needed parts from the Go crypto stack. (Admittedly, when filling this proposal here, I didn't expect it would sit around for such a long time; meanwhile, requiring TLS 1.3 might have become more realistic. But of course we're all busy, no offense.) Looking at the TLS 1.3 channel binding draft, it appears that the existing implementation of the Go crypto stack might be sufficient to support SCRAM-*-PLUS on TLS 1.3 unless IETF makes substantial changes to the draft RFC, but perhaps you could have a look to confirm. Personally, I'd still find it nice if TLS 1.2 was supportable for a while (which would need RFC 7627 for secure channel binding), but I can certainly see the cost in complexity and pain.

@rsc
Copy link
Contributor

rsc commented Dec 1, 2021

/cc @FiloSottile

@rsc
Copy link
Contributor

rsc commented Jan 5, 2022

cc @golang/security

@ksridhar57
Copy link

ksridhar57 commented Jan 7, 2022

Hi,

I was trying to test the KDF functions for FIPS certification and realised that go's (version 1.12.1) crypto/tls does not implement RFC 7627 requirements. What is the recommended approach here? Is this specification now implemented in later versions of go releases?
I'm not too familiar with the specifications and I'm arriving at this understanding based on the crypto source code I happened to go through at a high level. Would appreciate some guidance here.

Thanks!

@Neustradamus
Copy link

Neustradamus commented Jan 11, 2022

About SCRAM, it will be nice to support it.
TLS 1.2 is always used (-PLUS variants: TLS Binding).

@ksridhar57
Copy link

ksridhar57 commented Jan 12, 2022

Do we have a solution where we could patch the TLS implementation in Go to support RFC 7627? As I understand this specification is now becoming mandatory for FIPS 140-3 certifications. Any advice is appreciated.

@FiloSottile FiloSottile self-assigned this Mar 2, 2022
@rsc
Copy link
Contributor

rsc commented Mar 16, 2022

@FiloSottile, you self-assigned this. What is the status of this? Do you think we should accept the proposal?

@FiloSottile
Copy link
Contributor

FiloSottile commented Mar 21, 2022

@FiloSottile, you self-assigned this. What is the status of this? Do you think we should accept the proposal?

I self-assigned it to look into it, which I haven't done properly yet.

@rsc
Copy link
Contributor

rsc commented May 4, 2022

Putting on hold until security team has more bandwidth to look at this.

@rsc
Copy link
Contributor

rsc commented May 4, 2022

Placed on hold.
— rsc for the proposal review group

@rsc rsc moved this from Active to Hold in Proposals (old) May 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Proposal Proposal-Crypto Proposal related to crypto packages or other security issues Proposal-Hold
Projects
Status: Hold
Development

No branches or pull requests

9 participants