golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.64k stars 17.61k forks source link

crypto/tls: implement RFC7627 #43922

Closed brawer closed 11 months ago

brawer commented 3 years ago

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 https://github.com/golang/go/issues/18346.

ianlancetaylor commented 3 years ago

CC @FiloSottile

bskidmore commented 3 years ago

Is there an update on this potential issue?

bskidmore commented 3 years ago

Is there an update on this issue?

rsc commented 2 years ago

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

rsc commented 2 years ago

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

FiloSottile commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

/cc @FiloSottile

rsc commented 2 years ago

cc @golang/security

ksridhar57 commented 2 years ago

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 commented 2 years ago

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

ksridhar57 commented 2 years ago

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.

rsc commented 2 years ago

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

FiloSottile commented 2 years ago

@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 commented 2 years ago

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

rsc commented 2 years ago

Placed on hold. — rsc for the proposal review group

krissridhar commented 1 year ago

Hi,

Resurrecting the earlier query on RFC 7627 does Go library 1.18 or later support the use of Extended Master Secret extensions in TLS? Any guidance here is much appreciated!

Thanks

krissridhar commented 1 year ago

Hi @https://github.com/rsc, is the proposal to implement RFC 7627 still on hold? Any advise on how this requirement is going to be addressed in Go?

neverpanic commented 1 year ago

Just to re-iterate on this, and why this might be important: Submissions to NIST for FIPS-certification after May 16th, 2023 must enforce the use of the extended master secret in TLS 1.2. This means that systems in FIPS mode will no longer be able to talk to Go TLS servers unless they offer TLS 1.3 (which admittedly they can since it's supported in anything >= Go 1.13).

See the FIPS 140-3 Implementation Guidance, section D.Q "Transition of the TLS 1.2 KDF to Support the Extended Master Secret".

FiloSottile commented 1 year ago

Ok, to recap, there are two channel binding mechanisms we care about here.

  1. tls-unique is defined in RFC 5929 and is basically just the Finished hash. tls-unique is exposed as ConnectionState.TLSUnique and "will be nil for TLS 1.3 connections and for all resumed connections".
  2. Exported Key Material is defined in RFC 5705 and is basically a hash of master secret and the Randoms. EKM is exposed as ConnectionState.ExportKeyingMaterial and returns an error if renegotiation (not resumption) is enabled.

RFC 9266 introduced tls-exporter which is based on EKM, and actually recommended replacing tls-unique with it.

Without RFC 7627, tls-unique can be forced not to be unique only for resumed connections, while EKM can be forced not to be unique for any connections. In that sense, it looks like we deprecated the wrong thing: TLSUnique is nil when resuming, and is always unique otherwise; ExportKeyingMaterial (and hence the new tls-exporter) instead might not be unique even in initial handshakes.

(An aside about renegotiation. Renegotiation is bad. We refuse to do it as a server. We disable it by default as a client. When we do it, we require certificates not to change, so I think our renegotiation implementation is actually unaffected by this. Still I guess we can refuse to renegotiate resumed connections if extended_master_secret is not in use. But also I don't care about renegotiation.)

Concretely, I propose for Go 1.21 we:

  1. Implement the extended_master_secret extension both on the client and on the server, and always enable it.
  2. Add a bool to sessionState to carry over whether e_m_s was used in the original session.
    • Note that this will make tickets not interoperate across Go 1.20 and Go 1.21 (falling back cleanly to full handshakes). This resolves a concern I had with the MUST in RFC 7627 to break if the original connection supported e_m_s and the resumed one doesn't. That felt too harsh if the cause was progressive rollout, but with this change resumption will just be skipped in that scenario.
  3. Implement the recommendation in Section 5.4 of RFC 7627 to disable tls-unique for resumed connections without e_m_s. This actually means enabling TLSUnique for resumed connections with e_m_s, since it's currently disabled for all resumed connections. We can also undeprecate TLSUnique.
  4. Implement the recommendation in Section 5.4 of RFC 7627 to disable EKM for all connections without e_m_s by returning an error from ExportKeyingMaterial. This is indirectly also recommended in Section 2 of RFC 9266 (which says tls-exporter is not defined for connections without e_m_s, and the only way to reflect that is to refuse EKM).

Note that (4) is a breaking change. In particular, EKM will not be available on the Go 1.21 side of a connection between Go 1.20 and Go 1.21. We should either provide a GODEBUG for this, or wait until Go 1.22 to enforce it, since by then all supported versions of Go will support e_m_s. Opinions?

FiloSottile commented 1 year ago

Oh, and about FIPS compliance, thank you @neverpanic for referencing the IG. The requirement is only for new validations, so it doesn't affect what we need select for now (even in crypto/tls/fipsonly mode), but you're correct that modules tested after May 16, 2023 won't be able to communicate with Go 1.20. I think that's going to be ok: modules tested in May 2023 are unlikely to get deployed before August 2023, when Go 1.21 will be released.

neverpanic commented 1 year ago

I think that's going to be ok: modules tested in May 2023 are unlikely to get deployed before August 2023, when Go 1.21 will be released.

While that is correct, keep in mind that modules can be released before they are certified. This will be the case for RHEL 9.2 and its derivatives, which will get an update to require the extended master secret in FIPS configurations shortly after its GA release. Of course those modules won't have a certification for a while, and users that care about FIPS should not run them before that. On the other hand, your time plan sounds solid and there isn't anything else that can be done on a shorter time frame anyway, plus users on such systems could always enable TLS 1.3.

rsc commented 1 year ago

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 commented 1 year ago

We should definitely add a GODEBUG if this is for Go 1.21. Even for Go 1.22 it should probably be a GODEBUG.

Is the goal to get this into Go 1.21?

Have all remaining concerns about this proposal been addressed?

FiloSottile commented 1 year ago

Let's leave (4) for Go 1.22, we don't need to do that part to be compatible with new FIPS modules.

I'll mail a CL for (1)-(3) today for Go 1.21.

gopherbot commented 1 year ago

Change https://go.dev/cl/497376 mentions this issue: crypto/tls: implement Extended Master Secret

rsc commented 1 year ago

Based on the discussion above, this proposal seems like a likely accept. — rsc for the proposal review group

Neustradamus commented 1 year ago

@FiloSottile: Thanks for your proposal, I hope the support of tls-exporter (in more tls-unique) soon...

rsc commented 1 year ago

Update: (1), (2), (3) were landed for Go 1.21 because they are non-breaking changes. (4) is planned for Go 1.22 with a GODEBUG.

rsc commented 1 year ago

No change in consensus, so accepted. 🎉 This issue now tracks the work of implementing the proposal. — rsc for the proposal review group

ianlancetaylor commented 1 year ago

@FiloSottile Would you be able to add a release note for the changes here? There is a TODO in doc/go1.21.html. Thanks.

Also I see that this issue is still open. What is left to do?

Also I see a comment about a GODEBUG setting, which as far as I can tell is not implemented. Is that still useful or did the CL go in a different direction?

(My apologies for not understanding what this issue is actually about.)

Neustradamus commented 1 year ago

@ianlancetaylor: It is not fully supported: @FiloSottile has specified "Let's leave (4) for Go 1.22".

ianlancetaylor commented 1 year ago

@Neustradamus Thanks. So we just need release notes for the other parts.

ianlancetaylor commented 1 year ago

Ah, this has actually been done in https://go.dev/cl/501303. Sorry for the noise.

Neustradamus commented 1 year ago

Now Go 1.21.0 has been released, what is the status of this ticket?

jgallucci32 commented 1 year ago

Looks like change was merged several months ago https://go-review.googlesource.com/c/go/+/497376

Neustradamus commented 1 year ago

@jgallucci32: Not the 4* point...

@FiloSottile details here:

Neustradamus commented 11 months ago

I think that you have seen the jabber.ru MITM and Channel Binding is the solution:

Little details, to know easily:

gopherbot commented 11 months ago

Change https://go.dev/cl/544155 mentions this issue: crypto/tls: disable ExportKeyingMaterial without EMS

Neustradamus commented 11 months ago

Hello @FiloSottile, now it is perfect for you about TLS 1.3?

Can you look "tls-server-end-point" too?

It is in RFC and XEPs too:

Thanks in advance.

Neustradamus commented 10 months ago

@FiloSottile: You have forgotten to answer me, can you look?

Neustradamus commented 9 months ago

@FiloSottile: I wish you a Happy New Year 2024!

Have you looked for "tls-server-end-point" part?

Thanks in advance.