Fix CertificateVerify for ed25519

We were incorrectly hashing the handshakeBodies and passing that into
Sign. However, for ed25519 you're not supposed to do that, it's part of
the signature scheme (aside from the fact that it also uses SHA-512, not
SHA-256).

This should now result in ED25519 client certificates working correctly.

Fixes: #469

Author: daenney

crypto.go

@@ -108,16 +108,20 @@ func verifyKeySignature(message, remoteKeySignature []byte, hashAlgorithm hash.A
 // the private key in the certificate.
 // https://tools.ietf.org/html/rfc5246#section-7.3
 func generateCertificateVerify(handshakeBodies []byte, privateKey crypto.PrivateKey, hashAlgorithm hash.Algorithm) ([]byte, error) {
+	if p, ok := privateKey.(ed25519.PrivateKey); ok {
+		// https://pkg.go.dev/crypto/ed25519#PrivateKey.Sign
+		// Sign signs the given message with priv. Ed25519 performs two passes over
+		// messages to be signed and therefore cannot handle pre-hashed messages.
+		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)
 
 	switch p := privateKey.(type) {
-	case ed25519.PrivateKey:
-		// https://crypto.stackexchange.com/a/55483
-		return p.Sign(rand.Reader, hashed, crypto.Hash(0))
 	case *ecdsa.PrivateKey:
 		return p.Sign(rand.Reader, hashed, hashAlgorithm.CryptoHash())
 	case *rsa.PrivateKey:

e2e/e2e_test.go

@@ -7,6 +7,7 @@ import (
 	"context"
 	"crypto/ed25519"
 	"crypto/rand"
+	"crypto/rsa"
 	"crypto/tls"
 	"errors"
 	"fmt"
@@ -362,6 +363,127 @@ func testPionE2ESimpleED25519(t *testing.T, server, client func(*comm)) {
 	}
 }
 
+func testPionE2ESimpleED25519ClientCert(t *testing.T, server, client func(*comm)) {
+	lim := test.TimeOut(time.Second * 30)
+	defer lim.Stop()
+
+	report := test.CheckRoutines(t)
+	defer report()
+
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+
+	_, skey, err := ed25519.GenerateKey(rand.Reader)
+	if err != nil {
+		t.Fatal(err)
+	}
+	scert, err := selfsign.SelfSign(skey)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_, ckey, err := ed25519.GenerateKey(rand.Reader)
+	if err != nil {
+		t.Fatal(err)
+	}
+	ccert, err := selfsign.SelfSign(ckey)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	scfg := &dtls.Config{
+		Certificates: []tls.Certificate{scert},
+		CipherSuites: []dtls.CipherSuiteID{dtls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256},
+		ClientAuth:   dtls.RequireAnyClientCert,
+	}
+	ccfg := &dtls.Config{
+		Certificates:       []tls.Certificate{ccert},
+		CipherSuites:       []dtls.CipherSuiteID{dtls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256},
+		InsecureSkipVerify: true,
+	}
+	serverPort := randomPort(t)
+	comm := newComm(ctx, ccfg, scfg, serverPort, server, client)
+	comm.assert(t)
+}
+
+func testPionE2ESimpleECDSAClientCert(t *testing.T, server, client func(*comm)) {
+	lim := test.TimeOut(time.Second * 30)
+	defer lim.Stop()
+
+	report := test.CheckRoutines(t)
+	defer report()
+
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+
+	scert, err := selfsign.GenerateSelfSigned()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	ccert, err := selfsign.GenerateSelfSigned()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	scfg := &dtls.Config{
+		Certificates: []tls.Certificate{scert},
+		CipherSuites: []dtls.CipherSuiteID{dtls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256},
+		ClientAuth:   dtls.RequireAnyClientCert,
+	}
+	ccfg := &dtls.Config{
+		Certificates:       []tls.Certificate{ccert},
+		CipherSuites:       []dtls.CipherSuiteID{dtls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256},
+		InsecureSkipVerify: true,
+	}
+	serverPort := randomPort(t)
+	comm := newComm(ctx, ccfg, scfg, serverPort, server, client)
+	comm.assert(t)
+}
+
+func testPionE2ESimpleRSAClientCert(t *testing.T, server, client func(*comm)) {
+	lim := test.TimeOut(time.Second * 30)
+	defer lim.Stop()
+
+	report := test.CheckRoutines(t)
+	defer report()
+
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+
+	spriv, err := rsa.GenerateKey(rand.Reader, 2048)
+	if err != nil {
+		t.Fatal(err)
+	}
+	scert, err := selfsign.SelfSign(spriv)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	cpriv, err := rsa.GenerateKey(rand.Reader, 2048)
+	if err != nil {
+		t.Fatal(err)
+	}
+	ccert, err := selfsign.SelfSign(cpriv)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	scfg := &dtls.Config{
+		Certificates: []tls.Certificate{scert},
+		CipherSuites: []dtls.CipherSuiteID{dtls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256},
+		ClientAuth:   dtls.RequireAnyClientCert,
+	}
+	ccfg := &dtls.Config{
+		Certificates:       []tls.Certificate{ccert},
+		CipherSuites:       []dtls.CipherSuiteID{dtls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256},
+		InsecureSkipVerify: true,
+	}
+	serverPort := randomPort(t)
+	comm := newComm(ctx, ccfg, scfg, serverPort, server, client)
+	comm.assert(t)
+}
+
 func TestPionE2ESimple(t *testing.T) {
 	testPionE2ESimple(t, serverPion, clientPion)
 }
@@ -377,3 +499,15 @@ func TestPionE2EMTUs(t *testing.T) {
 func TestPionE2ESimpleED25519(t *testing.T) {
 	testPionE2ESimpleED25519(t, serverPion, clientPion)
 }
+
+func TestPionE2ESimpleED25519ClientCert(t *testing.T) {
+	testPionE2ESimpleED25519ClientCert(t, serverPion, clientPion)
+}
+
+func TestPionE2ESimpleECDSAClientCert(t *testing.T) {
+	testPionE2ESimpleECDSAClientCert(t, serverPion, clientPion)
+}
+
+func TestPionE2ESimpleRSAClientCert(t *testing.T) {
+	testPionE2ESimpleRSAClientCert(t, serverPion, clientPion)
+}