From 2684fa6bcbba19f480286241f8b6391b92b12983 Mon Sep 17 00:00:00 2001 From: mschexnaydre Date: Tue, 17 Oct 2023 22:13:40 +0000 Subject: [PATCH] Respect Algorithm value in CertificateRequest Before it was hardcoded to always use SHA256. This change stores the HASH algorithm from the CertificateRequest message in the State object so that we can reference these later when generating the CertificateVerify message. Removed hard-coded usage of SHA-256 in generateCertificateVerify, now uses the Digest method of the passed in algorithm. Resolves #418 --- AUTHORS.txt | 2 ++ conn_test.go | 8 ++++++++ crypto.go | 7 +------ flight3handler.go | 3 ++- flight5handler.go | 3 ++- state.go | 2 ++ 6 files changed, 17 insertions(+), 8 deletions(-) diff --git a/AUTHORS.txt b/AUTHORS.txt index fbaf97711..d80ca9973 100644 --- a/AUTHORS.txt +++ b/AUTHORS.txt @@ -38,6 +38,7 @@ Lukas Lihotzki ManuelBk <26275612+ManuelBk@users.noreply.github.com> Michael Zabka Michiel De Backker +mschexnaydre Rachel Chen Robert Eperjesi Ryan Gordon @@ -53,6 +54,7 @@ Steffen Vogel Vadim Vadim Filimonov wmiao +Xinjun Ma ZHENK 吕海涛 diff --git a/conn_test.go b/conn_test.go index 755897bd9..d3226d876 100644 --- a/conn_test.go +++ b/conn_test.go @@ -864,6 +864,14 @@ func TestClientCertificate(t *testing.T) { ClientAuth: RequireAnyClientCert, }, }, + "RequestClientCert_cert_sigscheme": { // specify signature algorithm + clientCfg: &Config{RootCAs: srvCAPool, Certificates: []tls.Certificate{cert}}, + serverCfg: &Config{ + SignatureSchemes: []tls.SignatureScheme{tls.ECDSAWithP521AndSHA512}, + Certificates: []tls.Certificate{srvCert}, + ClientAuth: RequestClientCert, + }, + }, "RequestClientCert_cert": { clientCfg: &Config{RootCAs: srvCAPool, Certificates: []tls.Certificate{cert}}, serverCfg: &Config{ diff --git a/crypto.go b/crypto.go index 968910c7e..7b01002f3 100644 --- a/crypto.go +++ b/crypto.go @@ -9,7 +9,6 @@ import ( "crypto/ed25519" "crypto/rand" "crypto/rsa" - "crypto/sha256" "crypto/x509" "encoding/asn1" "encoding/binary" @@ -118,11 +117,7 @@ func generateCertificateVerify(handshakeBodies []byte, privateKey crypto.Private return p.Sign(rand.Reader, handshakeBodies, crypto.Hash(0)) } - h := sha256.New() - if _, err := h.Write(handshakeBodies); err != nil { - return nil, err - } - hashed := h.Sum(nil) + hashed := hashAlgorithm.Digest(handshakeBodies) switch p := privateKey.(type) { case *ecdsa.PrivateKey: diff --git a/flight3handler.go b/flight3handler.go index b17a7986c..920ee73bd 100644 --- a/flight3handler.go +++ b/flight3handler.go @@ -153,7 +153,8 @@ func flight3Parse(ctx context.Context, c flightConn, state *State, cache *handsh } } - if _, ok := msgs[handshake.TypeCertificateRequest].(*handshake.MessageCertificateRequest); ok { + if creq, ok := msgs[handshake.TypeCertificateRequest].(*handshake.MessageCertificateRequest); ok { + state.remoteCertRequestAlgs = creq.SignatureHashAlgorithms state.remoteRequestedCertificate = true } diff --git a/flight5handler.go b/flight5handler.go index e1cca6238..14ad66e53 100644 --- a/flight5handler.go +++ b/flight5handler.go @@ -193,7 +193,8 @@ func flight5Generate(c flightConn, state *State, cache *handshakeCache, cfg *han ), merged...) // Find compatible signature scheme - signatureHashAlgo, err := signaturehash.SelectSignatureScheme(cfg.localSignatureSchemes, privateKey) + + signatureHashAlgo, err := signaturehash.SelectSignatureScheme(state.remoteCertRequestAlgs, privateKey) if err != nil { return nil, &alert.Alert{Level: alert.Fatal, Description: alert.InsufficientSecurity}, err } diff --git a/state.go b/state.go index cbc287b42..a65d426bc 100644 --- a/state.go +++ b/state.go @@ -10,6 +10,7 @@ import ( "github.com/pion/dtls/v2/pkg/crypto/elliptic" "github.com/pion/dtls/v2/pkg/crypto/prf" + "github.com/pion/dtls/v2/pkg/crypto/signaturehash" "github.com/pion/dtls/v2/pkg/protocol/handshake" "github.com/pion/transport/v3/replaydetector" ) @@ -53,6 +54,7 @@ type State struct { handshakeSendSequence int handshakeRecvSequence int serverName string + remoteCertRequestAlgs []signaturehash.Algorithm remoteRequestedCertificate bool // Did we get a CertificateRequest localCertificatesVerify []byte // cache CertificateVerify localVerifyData []byte // cached VerifyData