bifurcation / mint

A Minimal TLS 1.3 Implementation in Go
MIT License
225 stars 36 forks source link

expose the peer certificates in the connection state #162

Closed marten-seemann closed 6 years ago

marten-seemann commented 6 years ago

The ConnectionState.PeerCertificates is now filled with the certificate chain that the peer sent. This works for both the server as well as the client.

Note that this PR is unrelated to #161. The certificate chain exposed via ConnectionState.PeerCertificates is just what the peer sent.

@ekr: This was marked as a TODO for you in the code, so I guess you'll have the most context to review this PR, right?

marten-seemann commented 6 years ago

@ekr, @bifurcation: Any thoughts on this PR?

ekr commented 6 years ago

On Fri, Jan 26, 2018 at 3:08 PM, Marten Seemann notifications@github.com wrote:

@marten-seemann commented on this pull request.

In client-state-machine.go https://github.com/bifurcation/mint/pull/162#discussion_r164243708:

@@ -821,6 +822,10 @@ func (state ClientStateWaitCV) Next(hr handshakeMessageReader) (HandshakeState, } else { logf(logTypeHandshake, "[ClientStateWaitCV] WARNING: No verification of server certificate") }

  • serverCertChain := make([]*x509.Certificate, len(state.serverCertificate.CertificateList))
  • for i, certEntry := range state.serverCertificate.CertificateList {
  • serverCertChain[i] = certEntry.CertData
  • }

I'm not sure I understand your point. Certificate verification is broken anyway (#161 https://github.com/bifurcation/mint/issues/161).

Yes, but eventually it will be fixed, and so it would be good to have the semantics be correct when that happens.

-Ekr

You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/bifurcation/mint/pull/162#discussion_r164243708, or mute the thread https://github.com/notifications/unsubscribe-auth/ABD1oQTSNT8nCUT4Ph3fi5zJz54_FLmrks5tOlrVgaJpZM4RaflB .

marten-seemann commented 6 years ago

I don't think we can do this change without fixing #161. I'll submit a PR for #161 very soon, and I hope we can move forward with this quickly. This is a blocking issue for me currently, since I need a way to authenticate the peer.

bifurcation commented 6 years ago

Closing in favor of #173