From 070616b909792ca48fdfd2a8b985a0cd28884149 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Sun, 1 Sep 2019 13:42:33 -0400 Subject: [PATCH 1/7] Add *_certificate_type extensions --- common.go | 15 +++++++++++++ extensions.go | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+) diff --git a/common.go b/common.go index 4d6477a..eceee1a 100644 --- a/common.go +++ b/common.go @@ -125,6 +125,8 @@ const ( ExtensionTypeCookie ExtensionType = 44 ExtensionTypePSKKeyExchangeModes ExtensionType = 45 ExtensionTypeTicketEarlyDataInfo ExtensionType = 46 + ExtensionTypeClientCertType ExtensionType = 19 + ExtensionTypeServerCertType ExtensionType = 20 ) // enum {...} NamedGroup @@ -164,6 +166,19 @@ const ( KeyUpdateRequested KeyUpdateRequest = 1 ) +/* +0 X.509 Y [RFC6091] +1 OpenPGP_RESERVED N [RFC6091][RFC8446] Used in TLS versions prior to 1.3. +2 Raw Public Key Y [RFC7250] +3 1609Dot2 N +*/ +type CertificateType uint8 + +const ( + CertificateTypeX509 CertificateType = 0 + CertificateTypeRawPublicKey CertificateType = 2 +) + type State uint8 const ( diff --git a/extensions.go b/extensions.go index 07cb16c..8864d4c 100644 --- a/extensions.go +++ b/extensions.go @@ -624,3 +624,63 @@ func (c CookieExtension) Marshal() ([]byte, error) { func (c *CookieExtension) Unmarshal(data []byte) (int, error) { return syntax.Unmarshal(data, c) } + +// struct { +// select(ClientOrServerExtension) { +// case client: +// CertificateType client_certificate_types<1..2^8-1>; +// case server: +// CertificateType client_certificate_type; +// } +// } ClientCertTypeExtension; +// +// Same for ServerCertTypeExtension +type certTypeExtension struct { + HandshakeType HandshakeType `tls:"none"` + CertificateTypes []CertificateType `tls:"head=1,min=1"` +} + +func (ct certTypeExtension) Marshal() ([]byte, error) { + switch ct.HandshakeType { + case HandshakeTypeClientHello: + return syntax.Marshal(ct) + + case HandshakeTypeServerHello: + return syntax.Marshal(uint8(ct.CertificateTypes[0])) + } + + return nil, fmt.Errorf("tls.certificate_types: Handshake type not allowed") +} + +func (ct *certTypeExtension) Unmarshal(data []byte) (int, error) { + switch ct.HandshakeType { + case HandshakeTypeClientHello: + return syntax.Unmarshal(data, ct) + + case HandshakeTypeServerHello: + var val uint8 + n, err := syntax.Unmarshal(data, &val) + if err != nil { + return 0, err + } + + ct.CertificateTypes = []CertificateType{CertificateType(val)} + return n, err + } + + return 0, fmt.Errorf("tls.certificate_types: Handshake type not allowed") +} + +// struct {...} ClientCertTypeExtension; +type ClientCertTypeExtension certTypeExtension + +func (cct ClientCertTypeExtension) Type() ExtensionType { + return ExtensionTypeClientCertType +} + +// struct {...} ServerCertTypeExtension; +type ServerCertTypeExtension certTypeExtension + +func (sct ServerCertTypeExtension) Type() ExtensionType { + return ExtensionTypeServerCertType +} From ca8543c0e625310d6b957fa8265a9749d589d670 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Sun, 1 Sep 2019 15:06:14 -0400 Subject: [PATCH 2/7] Initial integration with handshake --- client-state-machine.go | 32 ++++++++++++++++- conn.go | 5 +++ extensions.go | 78 ++++++++++++++++++++++++++++++----------- server-state-machine.go | 52 +++++++++++++++++++++++---- 4 files changed, 139 insertions(+), 28 deletions(-) diff --git a/client-state-machine.go b/client-state-machine.go index d8464a1..efd8c45 100644 --- a/client-state-machine.go +++ b/client-state-machine.go @@ -102,6 +102,24 @@ func (state clientStateStart) Next(hr handshakeMessageReader) (HandshakeState, [ alpn = &ALPNExtension{Protocols: state.Opts.NextProtos} } + // Raw public keys + var cct *ClientCertTypeExtension + var sct *ClientCertTypeExtension + if state.Config.AllowRawPublicKeys || !state.Config.ForbidCertificates { + cct := &ClientCertTypeExtension{HandshakeType: HandshakeTypeClientHello} + sct := &ServerCertTypeExtension{HandshakeType: HandshakeTypeClientHello} + + if state.Config.AllowRawPublicKeys { + cct.CertificateTypes = append(cct.CertificateTypes, CertificateTypeRawPublicKey) + sct.CertificateTypes = append(sct.CertificateTypes, CertificateTypeRawPublicKey) + } + + if !state.Config.ForbidCertificates { + cct.CertificateTypes = append(cct.CertificateTypes, CertificateTypeX509) + sct.CertificateTypes = append(sct.CertificateTypes, CertificateTypeX509) + } + } + // Construct base ClientHello ch := &ClientHelloBody{ LegacyVersion: wireVersion(state.hsCtx.hIn), @@ -113,7 +131,7 @@ func (state clientStateStart) Next(hr handshakeMessageReader) (HandshakeState, [ return nil, nil, AlertInternalError } for _, ext := range []ExtensionBody{&sv, &sni, &ks, &sg, &sa} { - err := ch.Extensions.Add(ext) + err = ch.Extensions.Add(ext) if err != nil { logf(logTypeHandshake, "[ClientStateStart] Error adding extension type=[%v] [%v]", ext.Type(), err) return nil, nil, AlertInternalError @@ -128,6 +146,18 @@ func (state clientStateStart) Next(hr handshakeMessageReader) (HandshakeState, [ return nil, nil, AlertInternalError } } + if cct != nil { + err = ch.Extensions.Add(cct) + if err != nil { + logf(logTypeHandshake, "[ClientStateStart] Error adding ClientCertType extension [%v]", err) + return nil, nil, AlertInternalError + } + err = ch.Extensions.Add(sct) + if err != nil { + logf(logTypeHandshake, "[ClientStateStart] Error adding ServerCertType extension [%v]", err) + return nil, nil, AlertInternalError + } + } if state.cookie != nil { err := ch.Extensions.Add(&CookieExtension{Cookie: state.cookie}) if err != nil { diff --git a/conn.go b/conn.go index ccf8dc1..ffb31da 100644 --- a/conn.go +++ b/conn.go @@ -129,6 +129,11 @@ type Config struct { NonBlocking bool UseDTLS bool + // These bools are arranged in opposite directions so that their default + // values reflect the correct default semantics (certs yes, raw keys no) + AllowRawPublicKeys bool + ForbidCertificates bool + RecordLayer RecordLayerFactory // The same config object can be shared among different connections, so it diff --git a/extensions.go b/extensions.go index 8864d4c..629a4ba 100644 --- a/extensions.go +++ b/extensions.go @@ -633,54 +633,90 @@ func (c *CookieExtension) Unmarshal(data []byte) (int, error) { // CertificateType client_certificate_type; // } // } ClientCertTypeExtension; -// -// Same for ServerCertTypeExtension -type certTypeExtension struct { +type ClientCertTypeExtension struct { HandshakeType HandshakeType `tls:"none"` CertificateTypes []CertificateType `tls:"head=1,min=1"` } -func (ct certTypeExtension) Marshal() ([]byte, error) { - switch ct.HandshakeType { +func (cct ClientCertTypeExtension) Type() ExtensionType { + return ExtensionTypeClientCertType +} + +func (cct ClientCertTypeExtension) Marshal() ([]byte, error) { + switch cct.HandshakeType { case HandshakeTypeClientHello: - return syntax.Marshal(ct) + return syntax.Marshal(cct) - case HandshakeTypeServerHello: - return syntax.Marshal(uint8(ct.CertificateTypes[0])) + case HandshakeTypeEncryptedExtensions: + return syntax.Marshal(uint8(cct.CertificateTypes[0])) } return nil, fmt.Errorf("tls.certificate_types: Handshake type not allowed") } -func (ct *certTypeExtension) Unmarshal(data []byte) (int, error) { - switch ct.HandshakeType { +func (cct *ClientCertTypeExtension) Unmarshal(data []byte) (int, error) { + switch cct.HandshakeType { case HandshakeTypeClientHello: - return syntax.Unmarshal(data, ct) + return syntax.Unmarshal(data, cct) - case HandshakeTypeServerHello: + case HandshakeTypeEncryptedExtensions: var val uint8 n, err := syntax.Unmarshal(data, &val) if err != nil { return 0, err } - ct.CertificateTypes = []CertificateType{CertificateType(val)} + cct.CertificateTypes = []CertificateType{CertificateType(val)} return n, err } return 0, fmt.Errorf("tls.certificate_types: Handshake type not allowed") } -// struct {...} ClientCertTypeExtension; -type ClientCertTypeExtension certTypeExtension - -func (cct ClientCertTypeExtension) Type() ExtensionType { - return ExtensionTypeClientCertType +// struct { +// select(ClientOrServerExtension) { +// case client: +// CertificateType client_certificate_types<1..2^8-1>; +// case server: +// CertificateType client_certificate_type; +// } +// } ServerCertTypeExtension; +type ServerCertTypeExtension struct { + HandshakeType HandshakeType `tls:"none"` + CertificateTypes []CertificateType `tls:"head=1,min=1"` } -// struct {...} ServerCertTypeExtension; -type ServerCertTypeExtension certTypeExtension - func (sct ServerCertTypeExtension) Type() ExtensionType { return ExtensionTypeServerCertType } + +func (sct ServerCertTypeExtension) Marshal() ([]byte, error) { + switch sct.HandshakeType { + case HandshakeTypeClientHello: + return syntax.Marshal(sct) + + case HandshakeTypeEncryptedExtensions: + return syntax.Marshal(uint8(sct.CertificateTypes[0])) + } + + return nil, fmt.Errorf("tls.certificate_types: Handshake type not allowed") +} + +func (sct *ServerCertTypeExtension) Unmarshal(data []byte) (int, error) { + switch sct.HandshakeType { + case HandshakeTypeClientHello: + return syntax.Unmarshal(data, sct) + + case HandshakeTypeEncryptedExtensions: + var val uint8 + n, err := syntax.Unmarshal(data, &val) + if err != nil { + return 0, err + } + + sct.CertificateTypes = []CertificateType{CertificateType(val)} + return n, err + } + + return 0, fmt.Errorf("tls.certificate_types: Handshake type not allowed") +} diff --git a/server-state-machine.go b/server-state-machine.go index e392a5d..613173a 100644 --- a/server-state-machine.go +++ b/server-state-machine.go @@ -122,6 +122,8 @@ func (state serverStateStart) Next(hr handshakeMessageReader) (HandshakeState, [ clientALPN := new(ALPNExtension) clientPSKModes := new(PSKKeyExchangeModesExtension) clientCookie := new(CookieExtension) + clientClientCertTypes := new(ClientCertTypeExtension) + clientServerCertTypes := new(ServerCertTypeExtension) // Handle external extensions. if state.Config.ExtensionHandler != nil { @@ -144,6 +146,8 @@ func (state serverStateStart) Next(hr handshakeMessageReader) (HandshakeState, [ clientALPN, clientPSKModes, clientCookie, + clientClientCertTypes, + clientServerCertTypes, }) if err != nil { @@ -331,6 +335,7 @@ func (state serverStateStart) Next(hr handshakeMessageReader) (HandshakeState, [ var pskSecret []byte var cert *Certificate var certScheme SignatureScheme + rawPublicKey := false if connParams.UsingPSK { pskSecret = psk.Key } else { @@ -344,6 +349,11 @@ func (state serverStateStart) Next(hr handshakeMessageReader) (HandshakeState, [ return nil, nil, AlertMissingExtension } + // Figure out if we're going to use a certificate or a raw public key + if foundExts[ExtensionTypeServerCertType] && state.Config.AllowRawPublicKeys { + rawPublicKey = true + } + // Select a certificate name := string(*serverName) var err error @@ -394,6 +404,7 @@ func (state serverStateStart) Next(hr handshakeMessageReader) (HandshakeState, [ selectedPSK: selectedPSK, cert: cert, certScheme: certScheme, + rawPublicKey: rawPublicKey, legacySessionId: ch.LegacySessionID, clientEarlyTrafficSecret: clientEarlyTrafficSecret, @@ -456,6 +467,7 @@ type serverStateNegotiated struct { selectedPSK int cert *Certificate certScheme SignatureScheme + rawPublicKey bool legacySessionId []byte firstClientHello *HandshakeMessage helloRetryRequest *HandshakeMessage @@ -591,6 +603,27 @@ func (state serverStateNegotiated) Next(_ handshakeMessageReader) (HandshakeStat return nil, nil, AlertInternalError } } + if state.rawPublicKey { + logf(logTypeHandshake, "[server] sending ClientCertType extension") + err = eeList.Add(&ClientCertTypeExtension{ + HandshakeType: HandshakeTypeEncryptedExtensions, + CertificateTypes: []CertificateType{CertificateTypeRawPublicKey}, + }) + if err != nil { + logf(logTypeHandshake, "[ServerStateNegotiated] Error adding CCT to EncryptedExtensions [%v]", err) + return nil, nil, AlertInternalError + } + + logf(logTypeHandshake, "[server] sending ServerCertType extension") + err = eeList.Add(&ServerCertTypeExtension{ + HandshakeType: HandshakeTypeEncryptedExtensions, + CertificateTypes: []CertificateType{CertificateTypeRawPublicKey}, + }) + if err != nil { + logf(logTypeHandshake, "[ServerStateNegotiated] Error adding SCT to EncryptedExtensions [%v]", err) + return nil, nil, AlertInternalError + } + } ee := &EncryptedExtensionsBody{eeList} // Run the external extension handler. @@ -643,13 +676,19 @@ func (state serverStateNegotiated) Next(_ handshakeMessageReader) (HandshakeStat handshakeHash.Write(crm.Marshal()) } - // Create and send Certificate, CertificateVerify - certificate := &CertificateBody{ - CertificateList: make([]CertificateEntry, len(state.cert.Chain)), - } - for i, entry := range state.cert.Chain { - certificate.CertificateList[i] = CertificateEntry{CertData: entry} + // Create and send Certificate + var certList []CertificateEntry + if !state.rawPublicKey { + certList = make([]CertificateEntry, len(state.cert.Chain)) + for i, entry := range state.cert.Chain { + certList[i] = CertificateEntry{CertData: entry} + } + } else { + // TODO + panic("raw public keys not implemented yet") } + + certificate := &CertificateBody{CertificateList: certList} certm, err := state.hsCtx.hOut.HandshakeMessageFromBody(certificate) if err != nil { logf(logTypeHandshake, "[ServerStateNegotiated] Error marshaling Certificate [%v]", err) @@ -659,6 +698,7 @@ func (state serverStateNegotiated) Next(_ handshakeMessageReader) (HandshakeStat toSend = append(toSend, QueueHandshakeMessage{certm}) handshakeHash.Write(certm.Marshal()) + // Create and send CertificateVerify certificateVerify := &CertificateVerifyBody{Algorithm: state.certScheme} logf(logTypeHandshake, "Creating CertVerify: %04x %v", state.certScheme, params.Hash) From 7ee5f773c3f767edf0999add00dc9a37d1fe0a67 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Sun, 1 Sep 2019 15:52:18 -0400 Subject: [PATCH 3/7] Complete handshake integration --- client-state-machine.go | 111 ++++++++++++++++++++++++++----------- conn.go | 3 +- conn_test.go | 4 +- crypto.go | 25 +++++++++ handshake-messages.go | 51 ++--------------- handshake-messages_test.go | 18 ++---- server-state-machine.go | 78 +++++++++++++++++++------- state-machine.go | 3 +- 8 files changed, 178 insertions(+), 115 deletions(-) diff --git a/client-state-machine.go b/client-state-machine.go index efd8c45..ead5406 100644 --- a/client-state-machine.go +++ b/client-state-machine.go @@ -620,11 +620,15 @@ func (state clientStateWaitEE) Next(hr handshakeMessageReader) (HandshakeState, serverALPN := &ALPNExtension{} serverEarlyData := &EarlyDataExtension{} + serverClientCertType := &ClientCertTypeExtension{HandshakeType: HandshakeTypeEncryptedExtensions} + serverServerCertType := &ServerCertTypeExtension{HandshakeType: HandshakeTypeEncryptedExtensions} foundExts, err := ee.Extensions.Parse( []ExtensionBody{ serverALPN, serverEarlyData, + serverClientCertType, + serverServerCertType, }) if err != nil { logf(logTypeHandshake, "[ClientStateWaitEE] Error decoding extensions: %v", err) @@ -637,6 +641,14 @@ func (state clientStateWaitEE) Next(hr handshakeMessageReader) (HandshakeState, state.Params.NextProto = serverALPN.Protocols[0] } + if foundExts[ExtensionTypeClientCertType] && + foundExts[ExtensionTypeServerCertType] && + state.Config.AllowRawPublicKeys { + foundClient := serverClientCertType.CertificateTypes[0] == CertificateTypeRawPublicKey + foundServer := serverServerCertType.CertificateTypes[0] == CertificateTypeRawPublicKey + state.Params.UsingRawPublicKeys = foundClient && foundServer + } + state.handshakeHash.Write(hm.Marshal()) toSend := []HandshakeAction{} @@ -848,44 +860,67 @@ func (state clientStateWaitCV) Next(hr handshakeMessageReader) (HandshakeState, hcv := state.handshakeHash.Sum(nil) logf(logTypeHandshake, "Handshake Hash to be verified: [%d] %x", len(hcv), hcv) - serverPublicKey := state.serverCertificate.CertificateList[0].CertData.PublicKey + certs := make([][]byte, len(state.serverCertificate.CertificateList)) + for i, certEntry := range state.serverCertificate.CertificateList { + certs[i] = certEntry.CertData + } + + var err error + var serverPublicKey crypto.PublicKey + var eeCert *x509.Certificate + if !state.Params.UsingRawPublicKeys { + eeCert, err = x509.ParseCertificate(certs[0]) + if err != nil { + logf(logTypeHandshake, "[ClientStateWaitCV] Unable to parse client cert: %v", err) + } + + serverPublicKey = eeCert.PublicKey + } else { + serverPublicKey, err = unmarshalSigningKey(certs[0]) + if err != nil { + logf(logTypeHandshake, "[ClientStateWaitCV] Unable to parse raw public key: %v", err) + } + } + if err := certVerify.Verify(serverPublicKey, hcv); err != nil { logf(logTypeHandshake, "[ClientStateWaitCV] Server signature failed to verify") return nil, nil, AlertHandshakeFailure } - certs := make([]*x509.Certificate, len(state.serverCertificate.CertificateList)) - rawCerts := make([][]byte, len(state.serverCertificate.CertificateList)) - for i, certEntry := range state.serverCertificate.CertificateList { - certs[i] = certEntry.CertData - rawCerts[i] = certEntry.CertData.Raw - } - var verifiedChains [][]*x509.Certificate - if !state.Config.InsecureSkipVerify { - opts := x509.VerifyOptions{ - Roots: state.Config.RootCAs, - CurrentTime: state.Config.time(), - DNSName: state.Config.ServerName, - Intermediates: x509.NewCertPool(), - } + if !state.Params.UsingRawPublicKeys { + if !state.Config.InsecureSkipVerify { + opts := x509.VerifyOptions{ + Roots: state.Config.RootCAs, + CurrentTime: state.Config.time(), + DNSName: state.Config.ServerName, + Intermediates: x509.NewCertPool(), + } + + for i, cert := range certs { + if i == 0 { + continue + } + + caCert, err := x509.ParseCertificate(cert) + if err != nil { + logf(logTypeHandshake, "[ClientStateWaitCV] Error parsing server chain: %v", err) + return nil, nil, AlertDecodeError + } - for i, cert := range certs { - if i == 0 { - continue + opts.Intermediates.AddCert(caCert) + } + var err error + verifiedChains, err = eeCert.Verify(opts) + if err != nil { + logf(logTypeHandshake, "[ClientStateWaitCV] Certificate verification failed: %s", err) + return nil, nil, AlertBadCertificate } - opts.Intermediates.AddCert(cert) - } - var err error - verifiedChains, err = certs[0].Verify(opts) - if err != nil { - logf(logTypeHandshake, "[ClientStateWaitCV] Certificate verification failed: %s", err) - return nil, nil, AlertBadCertificate } } if state.Config.VerifyPeerCertificate != nil { - if err := state.Config.VerifyPeerCertificate(rawCerts, verifiedChains); err != nil { + if err := state.Config.VerifyPeerCertificate(certs, verifiedChains); err != nil { logf(logTypeHandshake, "[ClientStateWaitCV] Application rejected server certificate: %s", err) return nil, nil, AlertBadCertificate } @@ -918,7 +953,7 @@ type clientStateWaitFinished struct { certificates []*Certificate serverCertificateRequest *CertificateRequestBody - peerCertificates []*x509.Certificate + peerCertificates [][]byte verifiedChains [][]*x509.Certificate masterSecret []byte @@ -1030,12 +1065,23 @@ func (state clientStateWaitFinished) Next(hr handshakeMessageReader) (HandshakeS state.handshakeHash.Write(certm.Marshal()) } else { // Create and send Certificate, CertificateVerify - certificate := &CertificateBody{ - CertificateList: make([]CertificateEntry, len(cert.Chain)), - } - for i, entry := range cert.Chain { - certificate.CertificateList[i] = CertificateEntry{CertData: entry} + var certList []CertificateEntry + if !state.Params.UsingRawPublicKeys { + certList = make([]CertificateEntry, len(cert.Chain)) + for i, entry := range cert.Chain { + certList[i] = CertificateEntry{CertData: entry.Raw} + } + } else { + certData, err := marshalSigningKey(cert.PublicKey) + if err != nil { + logf(logTypeHandshake, "[ClientStateWaitFinished] Unable to marshal raw public key [%v]", err) + return nil, nil, AlertInternalError + } + + certList = []CertificateEntry{{CertData: certData}} } + + certificate := &CertificateBody{CertificateList: certList} certm, err := state.hsCtx.hOut.HandshakeMessageFromBody(certificate) if err != nil { logf(logTypeHandshake, "[ClientStateWaitFinished] Error marshaling Certificate [%v]", err) @@ -1045,6 +1091,7 @@ func (state clientStateWaitFinished) Next(hr handshakeMessageReader) (HandshakeS toSend = append(toSend, QueueHandshakeMessage{certm}) state.handshakeHash.Write(certm.Marshal()) + // Create and send CertificateVerify hcv := state.handshakeHash.Sum(nil) logf(logTypeHandshake, "Handshake Hash to be verified: [%d] %x", len(hcv), hcv) diff --git a/conn.go b/conn.go index ffb31da..e464993 100644 --- a/conn.go +++ b/conn.go @@ -16,6 +16,7 @@ import ( type Certificate struct { Chain []*x509.Certificate PrivateKey crypto.Signer + PublicKey crypto.PublicKey } type PreSharedKey struct { @@ -255,7 +256,7 @@ var ( type ConnectionState struct { HandshakeState State CipherSuite CipherSuiteParams // cipher suite in use (TLS_RSA_WITH_RC4_128_SHA, ...) - PeerCertificates []*x509.Certificate // certificate chain presented by remote peer + PeerCertificates [][]byte // certificate chain presented by remote peer VerifiedChains [][]*x509.Certificate // verified chains built from PeerCertificates NextProto string // Selected ALPN proto UsingPSK bool // Are we using PSK. diff --git a/conn_test.go b/conn_test.go index 5c8314a..f56e3b2 100644 --- a/conn_test.go +++ b/conn_test.go @@ -1232,9 +1232,9 @@ func TestConnectionState(t *testing.T) { serverCS := server.ConnectionState() assertEquals(t, clientCS.CipherSuite.Suite, configClient.CipherSuites[0]) assertDeepEquals(t, clientCS.VerifiedChains, [][]*x509.Certificate{{serverCert}}) - assertDeepEquals(t, clientCS.PeerCertificates, []*x509.Certificate{serverCert}) + assertDeepEquals(t, clientCS.PeerCertificates, [][]byte{serverCert.Raw}) assertEquals(t, serverCS.CipherSuite.Suite, serverConfig.CipherSuites[0]) - assertDeepEquals(t, serverCS.PeerCertificates, []*x509.Certificate{clientCert}) + assertDeepEquals(t, serverCS.PeerCertificates, [][]byte{clientCert.Raw}) } func TestDTLS(t *testing.T) { diff --git a/crypto.go b/crypto.go index 4fa59a2..4e9140a 100644 --- a/crypto.go +++ b/crypto.go @@ -328,6 +328,31 @@ func newSigningKey(sig SignatureScheme) (crypto.Signer, error) { } } +func marshalSigningKey(pub crypto.PublicKey) ([]byte, error) { + switch pub.(type) { + case *rsa.PublicKey: + return x509.MarshalPKIXPublicKey(pub) + + case *ecdsa.PublicKey: + return x509.MarshalPKIXPublicKey(pub) + + // TODO support Ed25519 + + default: + return nil, fmt.Errorf("tls.marshalsigningkey: Unsupported public key type") + } +} + +func unmarshalSigningKey(data []byte) (interface{}, error) { + pub, err := x509.ParsePKIXPublicKey(data) + if err == nil { + return pub, err + } + + // TODO attempt to parse Ed25519 + return nil, err +} + // XXX(rlb): Copied from crypto/x509 type ecdsaSignature struct { R, S *big.Int diff --git a/handshake-messages.go b/handshake-messages.go index e8e3aef..a96c6d9 100644 --- a/handshake-messages.go +++ b/handshake-messages.go @@ -3,7 +3,6 @@ package mint import ( "bytes" "crypto" - "crypto/x509" "encoding/binary" "fmt" @@ -269,23 +268,13 @@ func (ee *EncryptedExtensionsBody) Unmarshal(data []byte) (int, error) { // CertificateEntry certificate_list<0..2^24-1>; // } Certificate; type CertificateEntry struct { - CertData *x509.Certificate - Extensions ExtensionList -} - -type CertificateBody struct { - CertificateRequestContext []byte - CertificateList []CertificateEntry -} - -type certificateEntryInner struct { CertData []byte `tls:"head=3,min=1"` Extensions ExtensionList `tls:"head=2"` } -type certificateBodyInner struct { - CertificateRequestContext []byte `tls:"head=1"` - CertificateList []certificateEntryInner `tls:"head=3"` +type CertificateBody struct { + CertificateRequestContext []byte `tls:"head=1"` + CertificateList []CertificateEntry `tls:"head=3"` } func (c CertificateBody) Type() HandshakeType { @@ -293,41 +282,11 @@ func (c CertificateBody) Type() HandshakeType { } func (c CertificateBody) Marshal() ([]byte, error) { - inner := certificateBodyInner{ - CertificateRequestContext: c.CertificateRequestContext, - CertificateList: make([]certificateEntryInner, len(c.CertificateList)), - } - - for i, entry := range c.CertificateList { - inner.CertificateList[i] = certificateEntryInner{ - CertData: entry.CertData.Raw, - Extensions: entry.Extensions, - } - } - - return syntax.Marshal(inner) + return syntax.Marshal(c) } func (c *CertificateBody) Unmarshal(data []byte) (int, error) { - inner := certificateBodyInner{} - read, err := syntax.Unmarshal(data, &inner) - if err != nil { - return read, err - } - - c.CertificateRequestContext = inner.CertificateRequestContext - c.CertificateList = make([]CertificateEntry, len(inner.CertificateList)) - - for i, entry := range inner.CertificateList { - c.CertificateList[i].CertData, err = x509.ParseCertificate(entry.CertData) - if err != nil { - return 0, fmt.Errorf("tls:certificate: Certificate failed to parse: %v", err) - } - - c.CertificateList[i].Extensions = entry.Extensions - } - - return read, nil + return syntax.Unmarshal(data, c) } // struct { diff --git a/handshake-messages_test.go b/handshake-messages_test.go index 0f387d6..22cd98c 100644 --- a/handshake-messages_test.go +++ b/handshake-messages_test.go @@ -153,11 +153,11 @@ var ( CertificateRequestContext: []byte{0, 0, 0, 0}, CertificateList: []CertificateEntry{ { - CertData: cert1, + CertData: cert1.Raw, Extensions: extListValidIn, }, { - CertData: cert2, + CertData: cert2.Raw, Extensions: extListValidIn, }, }, @@ -166,7 +166,7 @@ var ( CertificateRequestContext: []byte{0, 0, 0, 0}, CertificateList: []CertificateEntry{ { - CertData: cert1, + CertData: cert1.Raw, Extensions: extListSingleTooLongIn, }, }, @@ -462,11 +462,11 @@ func TestCertificateMarshalUnmarshal(t *testing.T) { certValidIn.CertificateRequestContext = originalContext // Test marshal failure on no raw certa - originalRaw := cert1.Raw - cert1.Raw = []byte{} + originalCert := certValidIn.CertificateList[0].CertData + certValidIn.CertificateList[0].CertData = nil out, err = certValidIn.Marshal() assertError(t, err, "Marshaled a Certificate with an empty cert") - cert1.Raw = originalRaw + certValidIn.CertificateList[0].CertData = originalCert // Test marshal failure on extension list marshal failure out, err = certOverflowIn.Marshal() @@ -500,12 +500,6 @@ func TestCertificateMarshalUnmarshal(t *testing.T) { _, err = cert.Unmarshal(certValid) assertError(t, err, "Unmarshaled a Certificate with truncated certificates") certValid[8] ^= 0xFF - - // Test unmarshal failure on malformed certificate - certValid[11] ^= 0xFF // Clobber first octet of first cert - _, err = cert.Unmarshal(certValid) - assertError(t, err, "Unmarshaled a Certificate with truncated certificates") - certValid[11] ^= 0xFF } func TestCertificateVerifyMarshalUnmarshal(t *testing.T) { diff --git a/server-state-machine.go b/server-state-machine.go index 613173a..b9ac48f 100644 --- a/server-state-machine.go +++ b/server-state-machine.go @@ -2,6 +2,7 @@ package mint import ( "bytes" + "crypto" "crypto/x509" "fmt" "hash" @@ -122,8 +123,8 @@ func (state serverStateStart) Next(hr handshakeMessageReader) (HandshakeState, [ clientALPN := new(ALPNExtension) clientPSKModes := new(PSKKeyExchangeModesExtension) clientCookie := new(CookieExtension) - clientClientCertTypes := new(ClientCertTypeExtension) - clientServerCertTypes := new(ServerCertTypeExtension) + clientClientCertType := new(ClientCertTypeExtension) + clientServerCertType := new(ServerCertTypeExtension) // Handle external extensions. if state.Config.ExtensionHandler != nil { @@ -146,8 +147,8 @@ func (state serverStateStart) Next(hr handshakeMessageReader) (HandshakeState, [ clientALPN, clientPSKModes, clientCookie, - clientClientCertTypes, - clientServerCertTypes, + clientClientCertType, + clientServerCertType, }) if err != nil { @@ -335,7 +336,6 @@ func (state serverStateStart) Next(hr handshakeMessageReader) (HandshakeState, [ var pskSecret []byte var cert *Certificate var certScheme SignatureScheme - rawPublicKey := false if connParams.UsingPSK { pskSecret = psk.Key } else { @@ -350,8 +350,28 @@ func (state serverStateStart) Next(hr handshakeMessageReader) (HandshakeState, [ } // Figure out if we're going to use a certificate or a raw public key - if foundExts[ExtensionTypeServerCertType] && state.Config.AllowRawPublicKeys { - rawPublicKey = true + // TODO(rlb): Move this to a negotiation function? + if foundExts[ExtensionTypeClientCertType] && + foundExts[ExtensionTypeServerCertType] && + state.Config.AllowRawPublicKeys { + foundClient := false + for _, t := range clientClientCertType.CertificateTypes { + if t == CertificateTypeRawPublicKey { + foundClient = true + break + } + } + + foundServer := false + for _, t := range clientServerCertType.CertificateTypes { + if t == CertificateTypeRawPublicKey { + foundServer = true + break + } + } + + // Only support symmetric usage of raw public keys for now + connParams.UsingRawPublicKeys = foundClient && foundServer } // Select a certificate @@ -404,7 +424,6 @@ func (state serverStateStart) Next(hr handshakeMessageReader) (HandshakeState, [ selectedPSK: selectedPSK, cert: cert, certScheme: certScheme, - rawPublicKey: rawPublicKey, legacySessionId: ch.LegacySessionID, clientEarlyTrafficSecret: clientEarlyTrafficSecret, @@ -467,7 +486,6 @@ type serverStateNegotiated struct { selectedPSK int cert *Certificate certScheme SignatureScheme - rawPublicKey bool legacySessionId []byte firstClientHello *HandshakeMessage helloRetryRequest *HandshakeMessage @@ -603,7 +621,7 @@ func (state serverStateNegotiated) Next(_ handshakeMessageReader) (HandshakeStat return nil, nil, AlertInternalError } } - if state.rawPublicKey { + if state.Params.UsingRawPublicKeys { logf(logTypeHandshake, "[server] sending ClientCertType extension") err = eeList.Add(&ClientCertTypeExtension{ HandshakeType: HandshakeTypeEncryptedExtensions, @@ -678,14 +696,19 @@ func (state serverStateNegotiated) Next(_ handshakeMessageReader) (HandshakeStat // Create and send Certificate var certList []CertificateEntry - if !state.rawPublicKey { + if !state.Params.UsingRawPublicKeys { certList = make([]CertificateEntry, len(state.cert.Chain)) for i, entry := range state.cert.Chain { - certList[i] = CertificateEntry{CertData: entry} + certList[i] = CertificateEntry{CertData: entry.Raw} } } else { - // TODO - panic("raw public keys not implemented yet") + certData, err := marshalSigningKey(state.cert.PublicKey) + if err != nil { + logf(logTypeHandshake, "[ServerStateNegotiated] Unable to marshal raw public key [%v]", err) + return nil, nil, AlertInternalError + } + + certList = []CertificateEntry{{CertData: certData}} } certificate := &CertificateBody{CertificateList: certList} @@ -1085,22 +1108,35 @@ func (state serverStateWaitCV) Next(hr handshakeMessageReader) (HandshakeState, certVerify := &CertificateVerifyBody{} if err := safeUnmarshal(certVerify, hm.body); err != nil { - logf(logTypeHandshake, "[ServerStateWaitCert] Error decoding message %v", err) + logf(logTypeHandshake, "[ServerStateWaitCV] Error decoding message %v", err) return nil, nil, AlertDecodeError } - rawCerts := make([][]byte, len(state.clientCertificate.CertificateList)) - certs := make([]*x509.Certificate, len(state.clientCertificate.CertificateList)) + certs := make([][]byte, len(state.clientCertificate.CertificateList)) for i, certEntry := range state.clientCertificate.CertificateList { certs[i] = certEntry.CertData - rawCerts[i] = certEntry.CertData.Raw } // Verify client signature over handshake hash + var err error + var clientPublicKey crypto.PublicKey + if !state.Params.UsingRawPublicKeys { + cert, err := x509.ParseCertificate(certs[0]) + if err != nil { + logf(logTypeHandshake, "[ServerStateWaitCV] Unable to parse client cert: %v", err) + } + + clientPublicKey = cert.PublicKey + } else { + clientPublicKey, err = unmarshalSigningKey(certs[0]) + if err != nil { + logf(logTypeHandshake, "[ServerStateWaitCV] Unable to parse raw public key: %v", err) + } + } + hcv := state.handshakeHash.Sum(nil) logf(logTypeHandshake, "Handshake Hash to be verified: [%d] %x", len(hcv), hcv) - clientPublicKey := state.clientCertificate.CertificateList[0].CertData.PublicKey if err := certVerify.Verify(clientPublicKey, hcv); err != nil { logf(logTypeHandshake, "[ServerStateWaitCV] Failure in client auth verification [%v]", err) return nil, nil, AlertHandshakeFailure @@ -1108,7 +1144,7 @@ func (state serverStateWaitCV) Next(hr handshakeMessageReader) (HandshakeState, if state.Config.VerifyPeerCertificate != nil { // TODO(#171): pass in the verified chains, once we support different client auth types - if err := state.Config.VerifyPeerCertificate(rawCerts, nil); err != nil { + if err := state.Config.VerifyPeerCertificate(certs, nil); err != nil { logf(logTypeHandshake, "[ServerStateWaitCV] Application rejected client certificate: %s", err) return nil, nil, AlertBadCertificate } @@ -1141,7 +1177,7 @@ type serverStateWaitFinished struct { masterSecret []byte clientHandshakeTrafficSecret []byte - peerCertificates []*x509.Certificate + peerCertificates [][]byte verifiedChains [][]*x509.Certificate handshakeHash hash.Hash diff --git a/state-machine.go b/state-machine.go index f0635c8..5ad0207 100644 --- a/state-machine.go +++ b/state-machine.go @@ -57,6 +57,7 @@ type ConnectionOptions struct { type ConnectionParameters struct { UsingPSK bool UsingDH bool + UsingRawPublicKeys bool ClientSendingEarlyData bool UsingEarlyData bool RejectedEarlyData bool @@ -97,7 +98,7 @@ type stateConnected struct { clientTrafficSecret []byte serverTrafficSecret []byte exporterSecret []byte - peerCertificates []*x509.Certificate + peerCertificates [][]byte verifiedChains [][]*x509.Certificate } From 6491c0caf8dafebe5472d89fc4ddf03b4deee733 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Sun, 1 Sep 2019 16:09:33 -0400 Subject: [PATCH 4/7] Add test for raw public key mode --- client-state-machine.go | 2 +- conn_test.go | 22 ++++++++++++++++------ server-state-machine.go | 2 +- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/client-state-machine.go b/client-state-machine.go index ead5406..a795a3a 100644 --- a/client-state-machine.go +++ b/client-state-machine.go @@ -1072,7 +1072,7 @@ func (state clientStateWaitFinished) Next(hr handshakeMessageReader) (HandshakeS certList[i] = CertificateEntry{CertData: entry.Raw} } } else { - certData, err := marshalSigningKey(cert.PublicKey) + certData, err := marshalSigningKey(cert.PrivateKey.Public()) if err != nil { logf(logTypeHandshake, "[ClientStateWaitFinished] Unable to marshal raw public key [%v]", err) return nil, nil, AlertInternalError diff --git a/conn_test.go b/conn_test.go index f56e3b2..cfdd42e 100644 --- a/conn_test.go +++ b/conn_test.go @@ -179,7 +179,7 @@ var ( psk PreSharedKey psks *PSKMapCache - basicConfig, dtlsConfig, nbConfig, nbDTLSConfig, hrrConfig, alpnConfig, pskConfig, pskDTLSConfig, pskECDHEConfig, pskDHEConfig, resumptionConfig, ffdhConfig, x25519Config *Config + basicConfig, dtlsConfig, nbConfig, nbDTLSConfig, hrrConfig, alpnConfig, rawConfig, pskConfig, pskDTLSConfig, pskECDHEConfig, pskDHEConfig, resumptionConfig, ffdhConfig, x25519Config *Config ) func init() { @@ -262,6 +262,13 @@ func init() { InsecureSkipVerify: true, } + rawConfig = &Config{ + ServerName: serverName, + Certificates: certificates, + AllowRawPublicKeys: true, + InsecureSkipVerify: true, + } + pskConfig = &Config{ ServerName: serverName, CipherSuites: []CipherSuite{TLS_AES_128_GCM_SHA256}, @@ -357,11 +364,13 @@ func checkConsistency(t *testing.T, client *Conn, server *Conn) { func testConnInner(t *testing.T, name string, p testInstanceState) { // Configs array: - configs := map[string]*Config{"basic config": basicConfig, - "HRR": hrrConfig, - "ALPN": alpnConfig, - "FFDH": ffdhConfig, - "x25519": x25519Config, + configs := map[string]*Config{ + "basic config": basicConfig, + "HRR": hrrConfig, + "ALPN": alpnConfig, + "RawPK": rawConfig, + "FFDH": ffdhConfig, + "x25519": x25519Config, } c := configs[p["config"]] @@ -400,6 +409,7 @@ func TestBasicFlows(t *testing.T) { "basic config", "HRR", "ALPN", + "RawPK", "FFDH", "x25519", }, diff --git a/server-state-machine.go b/server-state-machine.go index b9ac48f..69219d7 100644 --- a/server-state-machine.go +++ b/server-state-machine.go @@ -702,7 +702,7 @@ func (state serverStateNegotiated) Next(_ handshakeMessageReader) (HandshakeStat certList[i] = CertificateEntry{CertData: entry.Raw} } } else { - certData, err := marshalSigningKey(state.cert.PublicKey) + certData, err := marshalSigningKey(state.cert.PrivateKey.Public()) if err != nil { logf(logTypeHandshake, "[ServerStateNegotiated] Unable to marshal raw public key [%v]", err) return nil, nil, AlertInternalError From 2f2aa5403b2f170d5728198eaed8b828fc4d31d9 Mon Sep 17 00:00:00 2001 From: "Richard L. Barnes" Date: Wed, 16 Jun 2021 11:13:02 -0400 Subject: [PATCH 5/7] Comments from @upros --- client-state-machine.go | 6 +++--- server-state-machine.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/client-state-machine.go b/client-state-machine.go index a795a3a..4d201ed 100644 --- a/client-state-machine.go +++ b/client-state-machine.go @@ -104,10 +104,10 @@ func (state clientStateStart) Next(hr handshakeMessageReader) (HandshakeState, [ // Raw public keys var cct *ClientCertTypeExtension - var sct *ClientCertTypeExtension + var sct *CServerCertTypeExtension if state.Config.AllowRawPublicKeys || !state.Config.ForbidCertificates { - cct := &ClientCertTypeExtension{HandshakeType: HandshakeTypeClientHello} - sct := &ServerCertTypeExtension{HandshakeType: HandshakeTypeClientHello} + cct = &ClientCertTypeExtension{HandshakeType: HandshakeTypeClientHello} + sct = &ServerCertTypeExtension{HandshakeType: HandshakeTypeClientHello} if state.Config.AllowRawPublicKeys { cct.CertificateTypes = append(cct.CertificateTypes, CertificateTypeRawPublicKey) diff --git a/server-state-machine.go b/server-state-machine.go index 69219d7..cc33fa2 100644 --- a/server-state-machine.go +++ b/server-state-machine.go @@ -123,8 +123,8 @@ func (state serverStateStart) Next(hr handshakeMessageReader) (HandshakeState, [ clientALPN := new(ALPNExtension) clientPSKModes := new(PSKKeyExchangeModesExtension) clientCookie := new(CookieExtension) - clientClientCertType := new(ClientCertTypeExtension) - clientServerCertType := new(ServerCertTypeExtension) + clientClientCertType := &ClientCertTypeExtension{HandshakeType: HandshakeTypeClientHello} + clientServerCertType := &ServerCertTypeExtension{HandshakeType: HandshakeTypeClientHello} // Handle external extensions. if state.Config.ExtensionHandler != nil { From 5ea91c6fc6586b0ead216d50854d8b4c42445899 Mon Sep 17 00:00:00 2001 From: "Richard L. Barnes" Date: Wed, 16 Jun 2021 15:23:04 -0400 Subject: [PATCH 6/7] Typo --- client-state-machine.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client-state-machine.go b/client-state-machine.go index 4d201ed..12f93e9 100644 --- a/client-state-machine.go +++ b/client-state-machine.go @@ -104,7 +104,7 @@ func (state clientStateStart) Next(hr handshakeMessageReader) (HandshakeState, [ // Raw public keys var cct *ClientCertTypeExtension - var sct *CServerCertTypeExtension + var sct *ServerCertTypeExtension if state.Config.AllowRawPublicKeys || !state.Config.ForbidCertificates { cct = &ClientCertTypeExtension{HandshakeType: HandshakeTypeClientHello} sct = &ServerCertTypeExtension{HandshakeType: HandshakeTypeClientHello} From b7f0873a2e9d47004b48979d8dcc358dcbda1a4b Mon Sep 17 00:00:00 2001 From: "Richard L. Barnes" Date: Wed, 16 Jun 2021 17:55:24 -0400 Subject: [PATCH 7/7] Responses to @ekr comments --- client-state-machine.go | 88 ++++++++++++++++++++++------------------- conn.go | 25 ++++++++---- negotiation.go | 22 +++++++++++ server-state-machine.go | 83 +++++++++++++++++++++++--------------- state-machine.go | 4 +- 5 files changed, 142 insertions(+), 80 deletions(-) diff --git a/client-state-machine.go b/client-state-machine.go index 12f93e9..1fbc512 100644 --- a/client-state-machine.go +++ b/client-state-machine.go @@ -96,30 +96,6 @@ func (state clientStateStart) Next(hr handshakeMessageReader) (HandshakeState, [ state.Params.ServerName = state.Opts.ServerName - // Application Layer Protocol Negotiation - var alpn *ALPNExtension - if (state.Opts.NextProtos != nil) && (len(state.Opts.NextProtos) > 0) { - alpn = &ALPNExtension{Protocols: state.Opts.NextProtos} - } - - // Raw public keys - var cct *ClientCertTypeExtension - var sct *ServerCertTypeExtension - if state.Config.AllowRawPublicKeys || !state.Config.ForbidCertificates { - cct = &ClientCertTypeExtension{HandshakeType: HandshakeTypeClientHello} - sct = &ServerCertTypeExtension{HandshakeType: HandshakeTypeClientHello} - - if state.Config.AllowRawPublicKeys { - cct.CertificateTypes = append(cct.CertificateTypes, CertificateTypeRawPublicKey) - sct.CertificateTypes = append(sct.CertificateTypes, CertificateTypeRawPublicKey) - } - - if !state.Config.ForbidCertificates { - cct.CertificateTypes = append(cct.CertificateTypes, CertificateTypeX509) - sct.CertificateTypes = append(sct.CertificateTypes, CertificateTypeX509) - } - } - // Construct base ClientHello ch := &ClientHelloBody{ LegacyVersion: wireVersion(state.hsCtx.hIn), @@ -131,22 +107,36 @@ func (state clientStateStart) Next(hr handshakeMessageReader) (HandshakeState, [ return nil, nil, AlertInternalError } for _, ext := range []ExtensionBody{&sv, &sni, &ks, &sg, &sa} { - err = ch.Extensions.Add(ext) + err := ch.Extensions.Add(ext) if err != nil { logf(logTypeHandshake, "[ClientStateStart] Error adding extension type=[%v] [%v]", ext.Type(), err) return nil, nil, AlertInternalError } } - // XXX: These optional extensions can't be folded into the above because Go - // interface-typed values are never reported as nil - if alpn != nil { + + // Application Layer Protocol Negotiation + if (state.Opts.NextProtos != nil) && (len(state.Opts.NextProtos) > 0) { + alpn := &ALPNExtension{Protocols: state.Opts.NextProtos} err := ch.Extensions.Add(alpn) if err != nil { logf(logTypeHandshake, "[ClientStateStart] Error adding ALPN extension [%v]", err) return nil, nil, AlertInternalError } } - if cct != nil { + + // Certificate type negotiation (to enable raw public keys) + if state.Config.AllowRawPublicKeys { + cct := &ClientCertTypeExtension{HandshakeType: HandshakeTypeClientHello} + sct := &ServerCertTypeExtension{HandshakeType: HandshakeTypeClientHello} + + cct.CertificateTypes = append(cct.CertificateTypes, CertificateTypeRawPublicKey) + sct.CertificateTypes = append(sct.CertificateTypes, CertificateTypeRawPublicKey) + + if !state.Config.ForbidX509 { + cct.CertificateTypes = append(cct.CertificateTypes, CertificateTypeX509) + sct.CertificateTypes = append(sct.CertificateTypes, CertificateTypeX509) + } + err = ch.Extensions.Add(cct) if err != nil { logf(logTypeHandshake, "[ClientStateStart] Error adding ClientCertType extension [%v]", err) @@ -158,6 +148,7 @@ func (state clientStateStart) Next(hr handshakeMessageReader) (HandshakeState, [ return nil, nil, AlertInternalError } } + if state.cookie != nil { err := ch.Extensions.Add(&CookieExtension{Cookie: state.cookie}) if err != nil { @@ -641,12 +632,24 @@ func (state clientStateWaitEE) Next(hr handshakeMessageReader) (HandshakeState, state.Params.NextProto = serverALPN.Protocols[0] } - if foundExts[ExtensionTypeClientCertType] && - foundExts[ExtensionTypeServerCertType] && - state.Config.AllowRawPublicKeys { - foundClient := serverClientCertType.CertificateTypes[0] == CertificateTypeRawPublicKey - foundServer := serverServerCertType.CertificateTypes[0] == CertificateTypeRawPublicKey - state.Params.UsingRawPublicKeys = foundClient && foundServer + if foundExts[ExtensionTypeServerCertType] { + certType := serverServerCertType.CertificateTypes[0] + if !CertificateTypeValid(certType, state.Config.AllowRawPublicKeys, state.Config.ForbidX509) { + logf(logTypeHandshake, "[ClientStateWaitEE] Server sent illegal certificate type: %v", certType) + return nil, nil, AlertHandshakeFailure + } + + state.Params.ServerCertType = certType + } + + if foundExts[ExtensionTypeClientCertType] { + certType := serverClientCertType.CertificateTypes[0] + if !CertificateTypeValid(certType, state.Config.AllowRawPublicKeys, state.Config.ForbidX509) { + logf(logTypeHandshake, "[ClientStateWaitEE] Client sent illegal certificate type: %v", certType) + return nil, nil, AlertHandshakeFailure + } + + state.Params.ClientCertType = certType } state.handshakeHash.Write(hm.Marshal()) @@ -868,17 +871,21 @@ func (state clientStateWaitCV) Next(hr handshakeMessageReader) (HandshakeState, var err error var serverPublicKey crypto.PublicKey var eeCert *x509.Certificate - if !state.Params.UsingRawPublicKeys { + switch state.Params.ServerCertType { + case CertificateTypeX509: eeCert, err = x509.ParseCertificate(certs[0]) if err != nil { logf(logTypeHandshake, "[ClientStateWaitCV] Unable to parse client cert: %v", err) + return nil, nil, AlertDecodeError } serverPublicKey = eeCert.PublicKey - } else { + + case CertificateTypeRawPublicKey: serverPublicKey, err = unmarshalSigningKey(certs[0]) if err != nil { logf(logTypeHandshake, "[ClientStateWaitCV] Unable to parse raw public key: %v", err) + return nil, nil, AlertDecodeError } } @@ -888,7 +895,7 @@ func (state clientStateWaitCV) Next(hr handshakeMessageReader) (HandshakeState, } var verifiedChains [][]*x509.Certificate - if !state.Params.UsingRawPublicKeys { + if state.Params.ServerCertType == CertificateTypeX509 { if !state.Config.InsecureSkipVerify { opts := x509.VerifyOptions{ Roots: state.Config.RootCAs, @@ -1066,12 +1073,13 @@ func (state clientStateWaitFinished) Next(hr handshakeMessageReader) (HandshakeS } else { // Create and send Certificate, CertificateVerify var certList []CertificateEntry - if !state.Params.UsingRawPublicKeys { + switch state.Params.ClientCertType { + case CertificateTypeX509: certList = make([]CertificateEntry, len(cert.Chain)) for i, entry := range cert.Chain { certList[i] = CertificateEntry{CertData: entry.Raw} } - } else { + case CertificateTypeRawPublicKey: certData, err := marshalSigningKey(cert.PrivateKey.Public()) if err != nil { logf(logTypeHandshake, "[ClientStateWaitFinished] Unable to marshal raw public key [%v]", err) diff --git a/conn.go b/conn.go index e464993..c8fa1bb 100644 --- a/conn.go +++ b/conn.go @@ -131,9 +131,10 @@ type Config struct { UseDTLS bool // These bools are arranged in opposite directions so that their default - // values reflect the correct default semantics (certs yes, raw keys no) + // values reflect the correct default semantics (certs yes, raw keys no). + // ForbidX509 is only meaningful if AllowRawPublicKeys is true. AllowRawPublicKeys bool - ForbidCertificates bool + ForbidX509 bool RecordLayer RecordLayerFactory @@ -204,15 +205,25 @@ func (c *Config) Init(isClient bool) error { return nil } +func (c *Config) certTypeValid() bool { + // ForbidX509 can only be set when AllowRawPublicKeys is also set + // Note that this is equivalent to: + // ForbidX509 => AllowRawPublicKeys + return !c.ForbidX509 || c.AllowRawPublicKeys +} + func (c *Config) ValidForServer() bool { - return (reflect.ValueOf(c.PSKs).IsValid() && c.PSKs.Size() > 0) || - (len(c.Certificates) > 0 && - len(c.Certificates[0].Chain) > 0 && - c.Certificates[0].PrivateKey != nil) + // The server must have either PSKs or certificates + havePSK := reflect.ValueOf(c.PSKs).IsValid() && c.PSKs.Size() > 0 + haveCert := len(c.Certificates) > 0 && + len(c.Certificates[0].Chain) > 0 && + c.Certificates[0].PrivateKey != nil + + return (havePSK || haveCert) && c.certTypeValid() } func (c *Config) ValidForClient() bool { - return len(c.ServerName) > 0 + return len(c.ServerName) > 0 && c.certTypeValid() } func (c *Config) time() time.Time { diff --git a/negotiation.go b/negotiation.go index 7463953..3a131f5 100644 --- a/negotiation.go +++ b/negotiation.go @@ -216,3 +216,25 @@ func ALPNNegotiation(psk *PreSharedKey, offered, supported []string) (string, er } return "", err } + +func CertificateTypeValid(certType CertificateType, allowRawPublicKey, forbidX509 bool) bool { + switch certType { + case CertificateTypeRawPublicKey: + return allowRawPublicKey + case CertificateTypeX509: + return !forbidX509 + default: + return false + } +} + +func CertificateTypeNegotiation(offered []CertificateType, allowRawPublicKey, forbidX509 bool) *CertificateType { + for _, t := range offered { + if CertificateTypeValid(t, allowRawPublicKey, forbidX509) { + out := new(CertificateType) + *out = t + return out + } + } + return nil +} diff --git a/server-state-machine.go b/server-state-machine.go index cc33fa2..4e17493 100644 --- a/server-state-machine.go +++ b/server-state-machine.go @@ -336,6 +336,8 @@ func (state serverStateStart) Next(hr handshakeMessageReader) (HandshakeState, [ var pskSecret []byte var cert *Certificate var certScheme SignatureScheme + var clientCertType *CertificateType + var serverCertType *CertificateType if connParams.UsingPSK { pskSecret = psk.Key } else { @@ -349,31 +351,35 @@ func (state serverStateStart) Next(hr handshakeMessageReader) (HandshakeState, [ return nil, nil, AlertMissingExtension } - // Figure out if we're going to use a certificate or a raw public key - // TODO(rlb): Move this to a negotiation function? - if foundExts[ExtensionTypeClientCertType] && - foundExts[ExtensionTypeServerCertType] && - state.Config.AllowRawPublicKeys { - foundClient := false - for _, t := range clientClientCertType.CertificateTypes { - if t == CertificateTypeRawPublicKey { - foundClient = true - break - } + // Negotiate certificate types for the client and server + if foundExts[ExtensionTypeClientCertType] && state.Config.RequireClientAuth { + clientCertType = CertificateTypeNegotiation(clientClientCertType.CertificateTypes, + state.Config.AllowRawPublicKeys, state.Config.ForbidX509) + + if clientCertType == nil { + logf(logTypeHandshake, "[ServerStateStart] Client certificate type negotiation failure") + return nil, nil, AlertHandshakeFailure } - foundServer := false - for _, t := range clientServerCertType.CertificateTypes { - if t == CertificateTypeRawPublicKey { - foundServer = true - break - } + connParams.ClientCertType = *clientCertType + } + + if foundExts[ExtensionTypeServerCertType] { + serverCertType = CertificateTypeNegotiation(clientServerCertType.CertificateTypes, + state.Config.AllowRawPublicKeys, state.Config.ForbidX509) + + if serverCertType == nil { + logf(logTypeHandshake, "[ServerStateStart] Client certificate type negotiation failure") + return nil, nil, AlertHandshakeFailure } - // Only support symmetric usage of raw public keys for now - connParams.UsingRawPublicKeys = foundClient && foundServer + connParams.ServerCertType = *serverCertType } + // Since we have already failed in cases where `nil` is invalid, from here + // on out, clientCertType == nil or serverCertType == nil indicate whether + // those extensions are to be sent. + // Select a certificate name := string(*serverName) var err error @@ -426,6 +432,8 @@ func (state serverStateStart) Next(hr handshakeMessageReader) (HandshakeState, [ certScheme: certScheme, legacySessionId: ch.LegacySessionID, clientEarlyTrafficSecret: clientEarlyTrafficSecret, + clientCertType: clientCertType, + serverCertType: serverCertType, firstClientHello: firstClientHello, helloRetryRequest: helloRetryRequest, @@ -490,6 +498,8 @@ type serverStateNegotiated struct { firstClientHello *HandshakeMessage helloRetryRequest *HandshakeMessage clientHello *HandshakeMessage + clientCertType *CertificateType + serverCertType *CertificateType } var _ HandshakeState = &serverStateNegotiated{} @@ -613,6 +623,7 @@ func (state serverStateNegotiated) Next(_ handshakeMessageReader) (HandshakeStat return nil, nil, AlertInternalError } } + if state.Params.UsingEarlyData { logf(logTypeHandshake, "[server] sending EDI extension") err = eeList.Add(&EarlyDataExtension{}) @@ -621,27 +632,31 @@ func (state serverStateNegotiated) Next(_ handshakeMessageReader) (HandshakeStat return nil, nil, AlertInternalError } } - if state.Params.UsingRawPublicKeys { - logf(logTypeHandshake, "[server] sending ClientCertType extension") - err = eeList.Add(&ClientCertTypeExtension{ + + if state.serverCertType != nil { + logf(logTypeHandshake, "[server] sending ServerCertType extension") + err = eeList.Add(&ServerCertTypeExtension{ HandshakeType: HandshakeTypeEncryptedExtensions, - CertificateTypes: []CertificateType{CertificateTypeRawPublicKey}, + CertificateTypes: []CertificateType{*state.serverCertType}, }) if err != nil { - logf(logTypeHandshake, "[ServerStateNegotiated] Error adding CCT to EncryptedExtensions [%v]", err) + logf(logTypeHandshake, "[ServerStateNegotiated] Error adding SCT to EncryptedExtensions [%v]", err) return nil, nil, AlertInternalError } + } - logf(logTypeHandshake, "[server] sending ServerCertType extension") - err = eeList.Add(&ServerCertTypeExtension{ + if state.Config.RequireClientAuth && state.clientCertType != nil { + logf(logTypeHandshake, "[server] sending ClientCertType extension") + err = eeList.Add(&ClientCertTypeExtension{ HandshakeType: HandshakeTypeEncryptedExtensions, - CertificateTypes: []CertificateType{CertificateTypeRawPublicKey}, + CertificateTypes: []CertificateType{*state.clientCertType}, }) if err != nil { - logf(logTypeHandshake, "[ServerStateNegotiated] Error adding SCT to EncryptedExtensions [%v]", err) + logf(logTypeHandshake, "[ServerStateNegotiated] Error adding CCT to EncryptedExtensions [%v]", err) return nil, nil, AlertInternalError } } + ee := &EncryptedExtensionsBody{eeList} // Run the external extension handler. @@ -696,12 +711,14 @@ func (state serverStateNegotiated) Next(_ handshakeMessageReader) (HandshakeStat // Create and send Certificate var certList []CertificateEntry - if !state.Params.UsingRawPublicKeys { + switch state.Params.ServerCertType { + case CertificateTypeX509: certList = make([]CertificateEntry, len(state.cert.Chain)) for i, entry := range state.cert.Chain { certList[i] = CertificateEntry{CertData: entry.Raw} } - } else { + + case CertificateTypeRawPublicKey: certData, err := marshalSigningKey(state.cert.PrivateKey.Public()) if err != nil { logf(logTypeHandshake, "[ServerStateNegotiated] Unable to marshal raw public key [%v]", err) @@ -1120,14 +1137,16 @@ func (state serverStateWaitCV) Next(hr handshakeMessageReader) (HandshakeState, // Verify client signature over handshake hash var err error var clientPublicKey crypto.PublicKey - if !state.Params.UsingRawPublicKeys { + switch state.Params.ClientCertType { + case CertificateTypeX509: cert, err := x509.ParseCertificate(certs[0]) if err != nil { logf(logTypeHandshake, "[ServerStateWaitCV] Unable to parse client cert: %v", err) } clientPublicKey = cert.PublicKey - } else { + + case CertificateTypeRawPublicKey: clientPublicKey, err = unmarshalSigningKey(certs[0]) if err != nil { logf(logTypeHandshake, "[ServerStateWaitCV] Unable to parse raw public key: %v", err) diff --git a/state-machine.go b/state-machine.go index 5ad0207..b5001be 100644 --- a/state-machine.go +++ b/state-machine.go @@ -57,12 +57,14 @@ type ConnectionOptions struct { type ConnectionParameters struct { UsingPSK bool UsingDH bool - UsingRawPublicKeys bool ClientSendingEarlyData bool UsingEarlyData bool RejectedEarlyData bool UsingClientAuth bool + ClientCertType CertificateType + ServerCertType CertificateType + CipherSuite CipherSuite ServerName string NextProto string