Repository: pion/dtls
Pull request #194 (2020-02-18 20:46:59)

Add Extension Server Name Indication
#### Description
Add Server Name Indication extension following the [RFC 6066 section 3](https://tools.ietf.org/html/rfc6066#section-3).

I change the way with choose the Certificate to use by comparing the server name with the CN and SANs.

This PR is based on the implementation in TLS in the standard lib.

daenney: For some weird reason the code coverage bot seems to not want to show up on this PR. Here's the current status of this PR: https://codecov.io/gh/pion/dtls/pull/194/changes

Patch:
diff --git a/README.md b/README.md
index 3dd890f..715b8a1 100644
--- a/README.md
+++ b/README.md
@@ -133,6 +133,7 @@ Check out the **[contributing wiki](https://github.com/pion/webrtc/wiki/Contribu
 * [Jozef Kralik](https://github.com/jkralik)
 * [Robert Eperjesi](https://github.com/epes)
 * [Atsushi Watanabe](https://github.com/at-wat)
+* [Julien Salleyron](https://github.com/juliens) - *Server Name Indication*
 
 ### License
 MIT License - see [LICENSE](LICENSE) for full text
diff --git a/cipher_suite.go b/cipher_suite.go
index 63ca1f5..b2017d5 100644
--- a/cipher_suite.go
+++ b/cipher_suite.go
@@ -119,8 +119,6 @@ func defaultCipherSuites() []cipherSuite {
 		&cipherSuiteTLSEcdheRsaWithAes128GcmSha256{},
 		&cipherSuiteTLSEcdheEcdsaWithAes256CbcSha{},
 		&cipherSuiteTLSEcdheRsaWithAes256CbcSha{},
-		newCipherSuiteTLSEcdheEcdsaWithAes128Ccm(),
-		newCipherSuiteTLSEcdheEcdsaWithAes128Ccm8(),
 	}
 }
 
diff --git a/client_handlers.go b/client_handlers.go
index 9930cca..2007f4f 100644
--- a/client_handlers.go
+++ b/client_handlers.go
@@ -2,6 +2,7 @@ package dtls
 
 import (
 	"bytes"
+	"crypto"
 	"crypto/x509"
 	"fmt"
 	"sync/atomic"
@@ -291,6 +292,10 @@ func clientFlightHandler(c *Conn) (bool, *alert, error) {
 			})
 		}
 
+		if len(c.serverName) > 0 {
+			extensions = append(extensions, &extensionServerName{serverName: c.serverName})
+		}
+
 		if err := c.bufferPacket(&packet{
 			record: &recordLayer{
 				recordLayerHeader: recordLayerHeader{
@@ -323,12 +328,19 @@ func clientFlightHandler(c *Conn) (bool, *alert, error) {
 			return true, nil, nil
 		}
 
+		var certBytes [][]byte
+		var privateKey crypto.PrivateKey
+		if len(c.localCertificates) > 0 {
+			certificate, err := c.getCertificate(c.serverName)
+			if err != nil {
+				return false, &alert{alertLevelFatal, alertHandshakeFailure}, err
+			}
+			certBytes = certificate.Certificate
+			privateKey = certificate.PrivateKey
+		}
+
 		messageSequence := c.handshakeMessageSequence
 		if c.remoteRequestedCertificate {
-			var certificate [][]byte
-			if len(c.localCertificates) > 0 {
-				certificate = c.localCertificates[0].Certificate
-			}
 			if err := c.bufferPacket(&packet{
 				record: &recordLayer{
 					recordLayerHeader: recordLayerHeader{
@@ -339,7 +351,7 @@ func clientFlightHandler(c *Conn) (bool, *alert, error) {
 							messageSequence: uint16(messageSequence),
 						},
 						handshakeMessage: &handshakeMessageCertificate{
-							certificate: certificate,
+							certificate: certBytes,
 						}},
 				},
 			}); err != nil {
@@ -420,8 +432,8 @@ func clientFlightHandler(c *Conn) (bool, *alert, error) {
 					handshakeCachePullRule{handshakeTypeCertificate, true},
 					handshakeCachePullRule{handshakeTypeClientKeyExchange, true},
 				)
-				//TODO: choose right certficate by CAs provided in Certificate Request.
-				certVerify, err := generateCertificateVerify(plainText, c.localCertificates[0].PrivateKey)
+
+				certVerify, err := generateCertificateVerify(plainText, privateKey)
 				if err != nil {
 					return false, &alert{alertLevelFatal, alertInternalError}, err
 				}
diff --git a/config.go b/config.go
index bfcca5e..06b5dfd 100644
--- a/config.go
+++ b/config.go
@@ -18,7 +18,6 @@ type Config struct {
 	// Certificates contains certificate chain to present to the other side of the connection.
 	// Server MUST set this if PSK is non-nil
 	// client SHOULD sets this so CertificateRequests can be handled if PSK is non-nil
-	// TODO: add support to use more certificates then one.
 	Certificates []tls.Certificate
 
 	// CipherSuites is a list of supported cipher suites.
diff --git a/conn.go b/conn.go
index 70bca8e..d6a452a 100644
--- a/conn.go
+++ b/conn.go
@@ -8,6 +8,7 @@ import (
 	"fmt"
 	"io"
 	"net"
+	"strings"
 	"sync"
 	"sync/atomic"
 	"time"
@@ -91,6 +92,8 @@ type Conn struct {
 	writeDeadline *deadline.Deadline
 
 	log logging.LeveledLogger
+
+	nameToCertificate map[string]*tls.Certificate
 }
 
 func createConn(ctx context.Context, nextConn net.Conn, flightHandler flightHandler, handshakeMessageHandler handshakeMessageHandler, config *Config, isClient bool) (*Conn, error) {
@@ -128,6 +131,25 @@ func createConn(ctx context.Context, nextConn net.Conn, flightHandler flightHand
 	handshakeDoneSignal := closer.NewCloser()
 	connectionClosed := closer.NewCloser()
 
+	nameToCertificate := make(map[string]*tls.Certificate)
+	for i := range config.Certificates {
+		cert := &config.Certificates[i]
+		x509Cert := cert.Leaf
+		if x509Cert == nil {
+			var parseErr error
+			x509Cert, parseErr = x509.ParseCertificate(cert.Certificate[0])
+			if parseErr != nil {
+				continue
+			}
+		}
+		if len(x509Cert.Subject.CommonName) > 0 {
+			nameToCertificate[strings.ToLower(x509Cert.Subject.CommonName)] = cert
+		}
+		for _, san := range x509Cert.DNSNames {
+			nameToCertificate[strings.ToLower(san)] = cert
+		}
+	}
+
 	c := &Conn{
 		nextConn:                    nextConn,
 		currFlight:                  newFlight(isClient, logger),
@@ -137,6 +159,7 @@ func createConn(ctx context.Context, nextConn net.Conn, flightHandler flightHand
 		flightHandler:               flightHandler,
 		maximumTransmissionUnit:     mtu,
 		localCertificates:           config.Certificates,
+		nameToCertificate:           nameToCertificate,
 		clientAuth:                  config.ClientAuth,
 		extendedMasterSecret:        config.ExtendedMasterSecret,
 		insecureSkipVerify:          config.InsecureSkipVerify,
@@ -205,7 +228,7 @@ func createConn(ctx context.Context, nextConn net.Conn, flightHandler flightHand
 	}
 
 	if err != nil {
-		c.close() //nolint
+		c.close() // nolint
 	} else {
 		c.setHandshakeCompletedSuccessfully()
 	}
@@ -278,6 +301,41 @@ func ServerWithContext(ctx context.Context, conn net.Conn, config *Config) (*Con
 	return createConn(ctx, conn, serverFlightHandler, serverHandshakeHandler, config, false)
 }
 
+func (c *Conn) getCertificate(serverName string) (*tls.Certificate, error) {
+	if len(c.localCertificates) == 0 {
+		return nil, errNoCertificates
+	}
+
+	if len(c.localCertificates) == 1 {
+		// There's only one choice, so no point doing any work.
+		return &c.localCertificates[0], nil
+	}
+
+	if len(serverName) == 0 {
+		return &c.localCertificates[0], nil
+	}
+
+	name := strings.TrimRight(strings.ToLower(serverName), ".")
+
+	if cert, ok := c.nameToCertificate[name]; ok {
+		return cert, nil
+	}
+
+	// try replacing labels in the name with wildcards until we get a
+	// match.
+	labels := strings.Split(name, ".")
+	for i := range labels {
+		labels[i] = "*"
+		candidate := strings.Join(labels, ".")
+		if cert, ok := c.nameToCertificate[candidate]; ok {
+			return cert, nil
+		}
+	}
+
+	// If nothing matches, return the first certificate.
+	return &c.localCertificates[0], nil
+}
+
 // Read reads data from the connection.
 func (c *Conn) Read(p []byte) (n int, err error) {
 	var out []byte
diff --git a/errors.go b/errors.go
index e975e0d..078e8dd 100644
--- a/errors.go
+++ b/errors.go
@@ -15,6 +15,7 @@ var (
 	errClientCertificateRequired         = errors.New("dtls: server required client verification, but got none")
 	errClientCertificateNotVerified      = errors.New("dtls: client sent certificate but did not verify it")
 	errCertificateVerifyNoCertificate    = errors.New("dtls: client sent certificate verify but we have no certificate to verify")
+	errNoCertificates                    = errors.New("dtls: no certificates configured")
 	errCipherSuiteNoIntersection         = errors.New("dtls: Client+Server do not support any shared cipher suites")
 	errCipherSuiteUnset                  = errors.New("dtls: server hello can not be created without a cipher suite")
 	errCompressionMethodUnset            = errors.New("dtls: server hello can not be created without a compression method")
@@ -31,6 +32,7 @@ var (
 	errInvalidECDSASignature             = errors.New("dtls: ECDSA signature contained zero or negative values")
 	errInvalidEllipticCurveType          = errors.New("dtls: invalid or unknown elliptic curve type")
 	errInvalidExtensionType              = errors.New("dtls: invalid extension type")
+	errInvalidSNIFormat                  = errors.New("dtls: invalid server name format")
 	errInvalidHashAlgorithm              = errors.New("dtls: invalid hash algorithm")
 	errInvalidMAC                        = errors.New("dtls: invalid mac")
 	errInvalidNamedCurve                 = errors.New("dtls: invalid named curve")
diff --git a/extension.go b/extension.go
index a1ff553..9e4bdb4 100644
--- a/extension.go
+++ b/extension.go
@@ -8,6 +8,7 @@ import (
 type extensionValue uint16
 
 const (
+	extensionServerNameValue                   extensionValue = 0
 	extensionSupportedEllipticCurvesValue      extensionValue = 10
 	extensionSupportedPointFormatsValue        extensionValue = 11
 	extensionSupportedSignatureAlgorithmsValue extensionValue = 13
@@ -47,6 +48,8 @@ func decodeExtensions(buf []byte) ([]extension, error) {
 		}
 		var err error
 		switch extensionValue(binary.BigEndian.Uint16(buf[offset:])) {
+		case extensionServerNameValue:
+			err = unmarshalAndAppend(buf[offset:], &extensionServerName{})
 		case extensionSupportedEllipticCurvesValue:
 			err = unmarshalAndAppend(buf[offset:], &extensionSupportedEllipticCurves{})
 		case extensionUseSRTPValue:
diff --git a/extension_server_name.go b/extension_server_name.go
new file mode 100644
index 0000000..07eddc2
--- /dev/null
+++ b/extension_server_name.go
@@ -0,0 +1,70 @@
+package dtls
+
+import (
+	"strings"
+
+	"golang.org/x/crypto/cryptobyte"
+)
+
+const extensionServerNameTypeDNSHostName = 0
+
+type extensionServerName struct {
+	serverName string
+}
+
+func (e extensionServerName) extensionValue() extensionValue {
+	return extensionServerNameValue
+}
+
+func (e *extensionServerName) Marshal() ([]byte, error) {
+	var b cryptobyte.Builder
+	b.AddUint16(uint16(e.extensionValue()))
+	b.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) {
+		b.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) {
+			b.AddUint8(extensionServerNameTypeDNSHostName)
+			b.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) {
+				b.AddBytes([]byte(e.serverName))
+			})
+		})
+	})
+	return b.Bytes()
+}
+
+func (e *extensionServerName) Unmarshal(data []byte) error {
+	s := cryptobyte.String(data)
+	var extension uint16
+	s.ReadUint16(&extension)
+	if extensionValue(extension) != e.extensionValue() {
+		return errInvalidExtensionType
+	}
+
+	var extData cryptobyte.String
+	s.ReadUint16LengthPrefixed(&extData)
+
+	var nameList cryptobyte.String
+	if !extData.ReadUint16LengthPrefixed(&nameList) || nameList.Empty() {
+		return errInvalidSNIFormat
+	}
+	for !nameList.Empty() {
+		var nameType uint8
+		var serverName cryptobyte.String
+		if !nameList.ReadUint8(&nameType) ||
+			!nameList.ReadUint16LengthPrefixed(&serverName) ||
+			serverName.Empty() {
+			return errInvalidSNIFormat
+		}
+		if nameType != extensionServerNameTypeDNSHostName {
+			continue
+		}
+		if len(e.serverName) != 0 {
+			// Multiple names of the same name_type are prohibited.
+			return errInvalidSNIFormat
+		}
+		e.serverName = string(serverName)
+		// An SNI value may not include a trailing dot.
+		if strings.HasSuffix(e.serverName, ".") {
+			return errInvalidSNIFormat
+		}
+	}
+	return nil
+}
diff --git a/handshake_message_client_hello.go b/handshake_message_client_hello.go
index d68ba9d..5287c31 100644
--- a/handshake_message_client_hello.go
+++ b/handshake_message_client_hello.go
@@ -114,6 +114,5 @@ func (h *handshakeMessageClientHello) Unmarshal(data []byte) error {
 		return err
 	}
 	h.extensions = extensions
-
 	return nil
 }
diff --git a/pkg/crypto/selfsign/selfsign.go b/pkg/crypto/selfsign/selfsign.go
index 0ccbe43..a61d3cb 100644
--- a/pkg/crypto/selfsign/selfsign.go
+++ b/pkg/crypto/selfsign/selfsign.go
@@ -29,11 +29,25 @@ func GenerateSelfSigned() (tls.Certificate, error) {
 	return SelfSign(priv)
 }
 
+// GenerateSelfSignedWithDNS creates a self-signed certificate
+func GenerateSelfSignedWithDNS(cn string, sans ...string) (tls.Certificate, error) {
+	priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	if err != nil {
+		return tls.Certificate{}, err
+	}
+
+	return WithDNS(priv, cn, sans...)
+}
+
 // SelfSign creates a self-signed certificate from a elliptic curve key
 func SelfSign(key crypto.PrivateKey) (tls.Certificate, error) {
+	return WithDNS(key, hex.EncodeToString(make([]byte, 16)))
+}
+
+// WithDNS creates a self-signed certificate from a elliptic curve key
+func WithDNS(key crypto.PrivateKey, cn string, sans ...string) (tls.Certificate, error) {
 	var (
 		pubKey    crypto.PublicKey
-		origin    = make([]byte, 16)
 		maxBigInt = new(big.Int) // Max random value, a 130-bits integer, i.e 2^130 - 1
 	)
 
@@ -65,8 +79,9 @@ func SelfSign(key crypto.PrivateKey) (tls.Certificate, error) {
 		NotAfter:              time.Now().AddDate(0, 1, 0),
 		SerialNumber:          serialNumber,
 		Version:               2,
-		Subject:               pkix.Name{CommonName: hex.EncodeToString(origin)},
+		Subject:               pkix.Name{CommonName: cn},
 		IsCA:                  true,
+		DNSNames:              sans,
 	}
 
 	raw, err := x509.CreateCertificate(rand.Reader, &template, &template, pubKey, key)
@@ -77,5 +92,6 @@ func SelfSign(key crypto.PrivateKey) (tls.Certificate, error) {
 	return tls.Certificate{
 		Certificate: [][]byte{raw},
 		PrivateKey:  key,
+		Leaf:        &template,
 	}, nil
 }
diff --git a/server_handlers.go b/server_handlers.go
index 30312c7..22adc9e 100644
--- a/server_handlers.go
+++ b/server_handlers.go
@@ -48,6 +48,8 @@ func serverHandshakeHandler(c *Conn) (*alert, error) {
 					if c.extendedMasterSecret != DisableExtendedMasterSecret {
 						c.state.extendedMasterSecret = true
 					}
+				case *extensionServerName:
+					c.serverName = e.serverName
 				}
 			}
 
@@ -348,10 +350,11 @@ func serverFlightHandler(c *Conn) (bool, *alert, error) {
 		messageSequence++
 
 		if c.localPSKCallback == nil {
-			var certificate [][]byte
-			if len(c.localCertificates) > 0 {
-				certificate = c.localCertificates[0].Certificate
+			certificate, err := c.getCertificate(c.serverName)
+			if err != nil {
+				return false, &alert{alertLevelFatal, alertHandshakeFailure}, err
 			}
+
 			if err := c.bufferPacket(&packet{
 				record: &recordLayer{
 					recordLayerHeader: recordLayerHeader{
@@ -362,7 +365,7 @@ func serverFlightHandler(c *Conn) (bool, *alert, error) {
 							messageSequence: uint16(messageSequence),
 						},
 						handshakeMessage: &handshakeMessageCertificate{
-							certificate: certificate,
+							certificate: certificate.Certificate,
 						}},
 				},
 			}); err != nil {
@@ -380,7 +383,7 @@ func serverFlightHandler(c *Conn) (bool, *alert, error) {
 					return false, &alert{alertLevelFatal, alertInternalError}, err
 				}
 
-				signature, err := generateKeySignature(clientRandom, serverRandom, c.localKeypair.publicKey, c.namedCurve, c.localCertificates[0].PrivateKey, hashAlgorithmSHA256)
+				signature, err := generateKeySignature(clientRandom, serverRandom, c.localKeypair.publicKey, c.namedCurve, certificate.PrivateKey, hashAlgorithmSHA256)
 				if err != nil {
 					return false, &alert{alertLevelFatal, alertInternalError}, err
 				}


Test patch:
diff --git a/conn_test.go b/conn_test.go
index 66e3110..53961c0 100644
--- a/conn_test.go
+++ b/conn_test.go
@@ -9,6 +9,7 @@ import (
 	"errors"
 	"fmt"
 	"net"
+	"reflect"
 	"testing"
 	"time"
 
@@ -1125,3 +1126,89 @@ func TestServerTimeout(t *testing.T) {
 	default:
 	}
 }
+
+func TestGetCertificate(t *testing.T) {
+	ca, cb := dpipe.Pipe()
+	defer func() {
+		err := ca.Close()
+		if err != nil {
+			t.Fatal(err)
+		}
+	}()
+
+	ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond)
+	defer cancel()
+
+	certificateWildcard, err := selfsign.GenerateSelfSignedWithDNS("*.test.test")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	certificateTest, err := selfsign.GenerateSelfSignedWithDNS("test.test", "www.test.test", "pop.test.test")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	certificateRandom, err := selfsign.GenerateSelfSigned()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	configServer := &Config{
+		Certificates: []tls.Certificate{
+			certificateRandom,
+			certificateTest,
+			certificateWildcard,
+		},
+	}
+
+	conn, err := createConn(ctx, cb, serverFlightHandler, serverHandshakeHandler, configServer, false)
+	if err != errConnectTimeout {
+		t.Fatal(err)
+	}
+
+	testCases := []struct {
+		desc                string
+		serverName          string
+		expectedCertificate tls.Certificate
+	}{
+		{
+			desc:                "Simple match in CN",
+			serverName:          "test.test",
+			expectedCertificate: certificateTest,
+		},
+		{
+			desc:                "Simple match in SANs",
+			serverName:          "www.test.test",
+			expectedCertificate: certificateTest,
+		},
+
+		{
+			desc:                "Wildcard match",
+			serverName:          "foo.test.test",
+			expectedCertificate: certificateWildcard,
+		},
+		{
+			desc:                "No match return first",
+			serverName:          "foo.bar",
+			expectedCertificate: certificateRandom,
+		},
+	}
+
+	for _, test := range testCases {
+		test := test
+
+		t.Run(test.desc, func(t *testing.T) {
+			t.Parallel()
+
+			cert, err := conn.getCertificate(test.serverName)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			if !reflect.DeepEqual(cert.Leaf, test.expectedCertificate.Leaf) {
+				t.Fatalf("Certificate does not match: expected(%v) actual(%v)", test.expectedCertificate.Leaf, cert.Leaf)
+			}
+		})
+	}
+}
diff --git a/extension_server_name_test.go b/extension_server_name_test.go
new file mode 100644
index 0000000..236de32
--- /dev/null
+++ b/extension_server_name_test.go
@@ -0,0 +1,22 @@
+package dtls
+
+import "testing"
+
+func TestServerName(t *testing.T) {
+	extension := extensionServerName{serverName: "test.domain"}
+
+	raw, err := extension.Marshal()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	newExtension := extensionServerName{}
+	err = newExtension.Unmarshal(raw)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if newExtension.serverName != extension.serverName {
+		t.Errorf("extensionServerName marshal: got %s expected %s", newExtension.serverName, extension.serverName)
+	}
+}