Repository: pion/dtls
Pull request #81 (2019-07-23 13:05:12)

Verify server and client certificates
Description:
- Verify certificates using CAs `Config.RootCAs`, current time and an optional callback `Config.VerifyPeerCertificate`
- Verification for server certificates is enabled by default and for client certificates based on `Config.ClientAuth (VerifyClientCertIfGiven, RequireAndVerifyClientCert)`
- Verification can be disabled by `Config.InsecureSkipVerify`
- Config parameters are inspired on `crypto/tls`

TODO: Verify server name


Sean-Der: woohoo awesome work @igolaizola 

I will look at this right after work, but feature wise it is a merge from me :) 
Sean-Der: @igolaizola ping me again when server name validation is good. happy to help in any way I can!

Thanks for grabbing this!
codecov-io: # [Codecov](https://codecov.io/gh/pion/dtls/pull/81?src=pr&el=h1) Report
> Merging [#81](https://codecov.io/gh/pion/dtls/pull/81?src=pr&el=desc) into [master](https://codecov.io/gh/pion/dtls/commit/bcc3927a53a8f925a2d30ee83cb319270a7bd667?src=pr&el=desc) will **increase** coverage by `0.27%`.
> The diff coverage is `73.68%`.

[![Impacted file tree graph](https://codecov.io/gh/pion/dtls/pull/81/graphs/tree.svg?width=650&token=bcCB2jB88I&height=150&src=pr)](https://codecov.io/gh/pion/dtls/pull/81?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master      #81      +/-   ##
==========================================
+ Coverage   70.56%   70.83%   +0.27%     
==========================================
  Files          51       51              
  Lines        2857     2894      +37     
==========================================
+ Hits         2016     2050      +34     
- Misses        600      602       +2     
- Partials      241      242       +1
```


| [Impacted Files](https://codecov.io/gh/pion/dtls/pull/81?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [conn.go](https://codecov.io/gh/pion/dtls/pull/81/diff?src=pr&el=tree#diff-Y29ubi5nbw==) | `77.59% <100%> (+0.22%)` | :arrow_up: |
| [server\_handlers.go](https://codecov.io/gh/pion/dtls/pull/81/diff?src=pr&el=tree#diff-c2VydmVyX2hhbmRsZXJzLmdv) | `84.39% <62.06%> (+0.12%)` | :arrow_up: |
| [handshake\_message\_certificate.go](https://codecov.io/gh/pion/dtls/pull/81/diff?src=pr&el=tree#diff-aGFuZHNoYWtlX21lc3NhZ2VfY2VydGlmaWNhdGUuZ28=) | `68% <81.81%> (+11.47%)` | :arrow_up: |
| [client\_handlers.go](https://codecov.io/gh/pion/dtls/pull/81/diff?src=pr&el=tree#diff-Y2xpZW50X2hhbmRsZXJzLmdv) | `81.84% <85.71%> (+0.59%)` | :arrow_up: |
| [internal/udp/conn.go](https://codecov.io/gh/pion/dtls/pull/81/diff?src=pr&el=tree#diff-aW50ZXJuYWwvdWRwL2Nvbm4uZ28=) | `75.45% <0%> (-1.82%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pion/dtls/pull/81?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pion/dtls/pull/81?src=pr&el=footer). Last update [bcc3927...cff63fd](https://codecov.io/gh/pion/dtls/pull/81?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

igolaizola: > @igolaizola ping me again when server name validation is good. happy to help in any way I can!

I've added ServerName validation.

In [crypto/tls](https://golang.org/pkg/crypto/tls/#Config) `Config.ServerName` is also used for SNI extension in the ClientHello, but the implementation of that feature should go in a different PR.
igolaizola: Important note about this PR: configurations that were working previously (selfsigned certs) may no longer work unless `InsecureSkipVerify` param is enabled. From a security point of view I think this breaking change is necessary.
daenney: I have a PR in mind to add a proper CHANGELOG, both to the repo and the release we tag, where we should mention these things.
Sean-Der: ACK! As soon as this lands we can make a new tag and I can update pion/webrtc that is my big worry :)

Might be nice to look at godoc and reach out to users!
daenney: Good point. Took a look, it's pion/webrtc and go-ocf/coap. I know the OCF folks, they recently integrated pion/dtls at my request to support coaps over UDP. I don't think anyone is really using it yet or would be surprised by having to set `InsecureSkipVerify` but I'll raise the issue with them. Though even when using go-ocf/coap you're expected to pass in your own pion/dtls.Config.
igolaizola: Should we merge this, or wait until receive some feedback?
Sean-Der: @igolaizola I am going to open a PR using your branch against `pion/webrtc` if that is green (will know in 10 mins) 👍 on merge from me!
Sean-Der: @igolaizola created PR! Mind approving https://github.com/pion/webrtc/pull/757 ?

When that is green I will merge both PRs so we don't break master at all (and will bump version numbers!)

I am not going to do a new major version. If we do `github.com/pion/dtls/v2` it will break a lot of people who don't use modules. I sort of regret doing it with `pion/webrtc` since tooling is still bad. They work great for us, but outside users really get hung up on it.

thanks!

Patch:
diff --git a/README.md b/README.md
index 72d62cc..9073f77 100644
--- a/README.md
+++ b/README.md
@@ -117,7 +117,7 @@ Check out the **[contributing wiki](https://github.com/pion/webrtc/wiki/Contribu
 * [Sean DuBois](https://github.com/Sean-Der) - *Original Author*
 * [Michiel De Backker](https://github.com/backkem) - *Public API*
 * [Chris Hiszpanski](https://github.com/thinkski) - *Support Signature Algorithms Extension*
-* [Iñigo Garcia Olaizola](https://github.com/igolaizola) - *Support serialization and resumption"*
+* [Iñigo Garcia Olaizola](https://github.com/igolaizola) - *Serialization & resumption, cert verification*
 * [Daniele Sluijters](https://github.com/daenney) - *AES-CCM support*
 * [Jin Lei](https://github.com/jinleileiking) - *Logging*
 * [Hugo Arregui](https://github.com/hugoArregui)
diff --git a/client_handlers.go b/client_handlers.go
index ca0ec7e..e734f89 100644
--- a/client_handlers.go
+++ b/client_handlers.go
@@ -45,9 +45,19 @@ func clientHandshakeHandler(c *Conn) error {
 		}
 
 		if c.localPSKCallback == nil {
-			expectedHash := valueKeySignature(clientRandom, serverRandom, h.publicKey, h.namedCurve, h.hashAlgorithm)
-			if err := verifyKeySignature(expectedHash, h.signature, h.hashAlgorithm, c.state.remoteCertificate); err != nil {
-				return err
+			if !c.insecureSkipVerify {
+				expectedHash := valueKeySignature(clientRandom, serverRandom, h.publicKey, h.namedCurve, h.hashAlgorithm)
+				if err := verifyKeySignature(expectedHash, h.signature, h.hashAlgorithm, c.state.remoteCertificate); err != nil {
+					return err
+				}
+				if err := verifyServerCert(c.state.remoteCertificate, c.rootCAs, c.serverName); err != nil {
+					return err
+				}
+			}
+			if c.verifyPeerCertificate != nil {
+				if err := c.verifyPeerCertificate(c.state.remoteCertificate, !c.insecureSkipVerify); err != nil {
+					return err
+				}
 			}
 		}
 		return nil
@@ -326,7 +336,10 @@ func clientFlightHandler(c *Conn) (bool, error) {
 
 		sequenceNumber++
 
-		if c.remoteRequestedCertificate {
+		// If the client has sent a certificate with signing ability, a digitally-signed
+		// CertificateVerify message is sent to explicitly verify possession of the
+		// private key in the certificate.
+		if c.remoteRequestedCertificate && c.localCertificate != nil {
 			if len(c.localCertificateVerify) == 0 {
 				plainText := c.handshakeCache.pullAndMerge(
 					handshakeCachePullRule{handshakeTypeClientHello, true},
diff --git a/config.go b/config.go
index 7716221..3aa5ce6 100644
--- a/config.go
+++ b/config.go
@@ -42,6 +42,36 @@ type Config struct {
 	PSK             PSKCallback
 	PSKIdentityHint []byte
 
+	// InsecureSkipVerify controls whether a client verifies the
+	// server's certificate chain and host name.
+	// If InsecureSkipVerify is true, TLS accepts any certificate
+	// presented by the server and any host name in that certificate.
+	// In this mode, TLS is susceptible to man-in-the-middle attacks.
+	// This should be used only for testing.
+	InsecureSkipVerify bool
+
+	// VerifyPeerCertificate, if not nil, is called after normal
+	// certificate verification by either a client or server. It
+	// receives the certificate provided by the peer and also a flag
+	// that tells if normal verification has succeedded. If it returns a
+	// non-nil error, the handshake is aborted and that error results.
+	//
+	// If normal verification fails then the handshake will abort before
+	// considering this callback. If normal verification is disabled by
+	// setting InsecureSkipVerify, or (for a server) when ClientAuth is
+	// RequestClientCert or RequireAnyClientCert, then this callback will
+	// be considered but the verified flag will always be false.
+	VerifyPeerCertificate func(cer *x509.Certificate, verified bool) error
+
+	// RootCAs defines the set of root certificate authorities
+	// that one peer uses when verifying the other peer's certificates.
+	// If RootCAs is nil, TLS uses the host's root CA set.
+	RootCAs *x509.CertPool
+
+	// ServerName is used to verify the hostname on the returned
+	// certificates unless InsecureSkipVerify is given.
+	ServerName string
+
 	LoggerFactory logging.LoggerFactory
 }
 
@@ -58,4 +88,6 @@ const (
 	NoClientCert ClientAuthType = iota
 	RequestClientCert
 	RequireAnyClientCert
+	VerifyClientCertIfGiven
+	RequireAndVerifyClientCert
 )
diff --git a/conn.go b/conn.go
index dfd196c..ce812ea 100644
--- a/conn.go
+++ b/conn.go
@@ -64,6 +64,11 @@ type Conn struct {
 	localKeySignature         []byte // cached keySignature
 	remoteCertificateVerified bool
 
+	insecureSkipVerify    bool
+	verifyPeerCertificate func(cer *x509.Certificate, verified bool) error
+	rootCAs               *x509.CertPool
+	serverName            string
+
 	handshakeMessageHandler handshakeMessageHandler
 	flightHandler           flightHandler
 	handshakeCompleted      chan bool
@@ -115,6 +120,10 @@ func createConn(nextConn net.Conn, flightHandler flightHandler, handshakeMessage
 		localCertificate:            config.Certificate,
 		localPrivateKey:             config.PrivateKey,
 		clientAuth:                  config.ClientAuth,
+		insecureSkipVerify:          config.InsecureSkipVerify,
+		verifyPeerCertificate:       config.VerifyPeerCertificate,
+		rootCAs:                     config.RootCAs,
+		serverName:                  config.ServerName,
 		localSRTPProtectionProfiles: config.SRTPProtectionProfiles,
 		localCipherSuites:           cipherSuites,
 		namedCurve:                  defaultNamedCurve,
@@ -128,6 +137,17 @@ func createConn(nextConn net.Conn, flightHandler flightHandler, handshakeMessage
 		log:                loggerFactory.NewLogger("dtls"),
 	}
 
+	// Use host from conn address when serverName is not provided
+	if isClient && c.serverName == "" && nextConn.RemoteAddr() != nil {
+		remoteAddr := nextConn.RemoteAddr().String()
+		var host string
+		host, _, err = net.SplitHostPort(remoteAddr)
+		if err != nil {
+			c.serverName = remoteAddr
+		}
+		c.serverName = host
+	}
+
 	var zeroEpoch uint16
 	c.state.localEpoch.Store(zeroEpoch)
 	c.state.remoteEpoch.Store(zeroEpoch)
diff --git a/crypto.go b/crypto.go
index ccad8b7..01f9148 100644
--- a/crypto.go
+++ b/crypto.go
@@ -10,6 +10,7 @@ import (
 	"encoding/asn1"
 	"encoding/binary"
 	"math/big"
+	"time"
 )
 
 type ecdsaSignature struct {
@@ -121,6 +122,32 @@ func verifyCertificateVerify(handshakeBodies []byte, hashAlgorithm HashAlgorithm
 	return errKeySignatureVerifyUnimplemented
 }
 
+func verifyClientCert(cert *x509.Certificate, roots *x509.CertPool) error {
+	opts := x509.VerifyOptions{
+		Roots:         roots,
+		CurrentTime:   time.Now(),
+		Intermediates: x509.NewCertPool(),
+		KeyUsages:     []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth},
+	}
+	if _, err := cert.Verify(opts); err != nil {
+		return err
+	}
+	return nil
+}
+
+func verifyServerCert(cert *x509.Certificate, roots *x509.CertPool, serverName string) error {
+	opts := x509.VerifyOptions{
+		Roots:         roots,
+		CurrentTime:   time.Now(),
+		DNSName:       serverName,
+		Intermediates: x509.NewCertPool(),
+	}
+	if _, err := cert.Verify(opts); err != nil {
+		return err
+	}
+	return nil
+}
+
 func generateAEADAdditionalData(h *recordLayerHeader, payloadLen int) []byte {
 	var additionalData [13]byte
 	// SequenceNumber MUST be set first
diff --git a/errors.go b/errors.go
index d1c434d..f8fdecd 100644
--- a/errors.go
+++ b/errors.go
@@ -7,7 +7,6 @@ var (
 	ErrConnClosed = errors.New("dtls: conn is closed")
 
 	errBufferTooSmall                    = errors.New("dtls: buffer is too small")
-	errCertificateUnset                  = errors.New("dtls: handshakeMessageCertificate can not be marshalled without a certificate")
 	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")
diff --git a/examples/dial/main.go b/examples/dial/main.go
index 542989b..2b33602 100644
--- a/examples/dial/main.go
+++ b/examples/dial/main.go
@@ -21,7 +21,7 @@ func main() {
 	//
 
 	// Prepare the configuration of the DTLS connection
-	config := &dtls.Config{Certificate: certificate, PrivateKey: privateKey}
+	config := &dtls.Config{Certificate: certificate, PrivateKey: privateKey, InsecureSkipVerify: true}
 
 	// Connect to a DTLS server
 	dtlsConn, err := dtls.Dial("udp", addr, config)
diff --git a/handshake_message_certificate.go b/handshake_message_certificate.go
index 3627013..485b399 100644
--- a/handshake_message_certificate.go
+++ b/handshake_message_certificate.go
@@ -13,15 +13,16 @@ func (h handshakeMessageCertificate) handshakeType() handshakeType {
 }
 
 func (h *handshakeMessageCertificate) Marshal() ([]byte, error) {
-	if h.certificate == nil {
-		return nil, errCertificateUnset
+	var raw []byte
+	if h.certificate != nil {
+		raw = h.certificate.Raw
 	}
 
 	out := make([]byte, 6)
-	putBigEndianUint24(out, uint32(len(h.certificate.Raw))+3)
-	putBigEndianUint24(out[3:], uint32(len(h.certificate.Raw)))
+	putBigEndianUint24(out, uint32(len(raw))+3)
+	putBigEndianUint24(out[3:], uint32(len(raw)))
 
-	return append(out, h.certificate.Raw...), nil
+	return append(out, raw...), nil
 }
 
 func (h *handshakeMessageCertificate) Unmarshal(data []byte) error {
@@ -37,11 +38,12 @@ func (h *handshakeMessageCertificate) Unmarshal(data []byte) error {
 		return errLengthMismatch
 	}
 
-	cert, err := x509.ParseCertificate(data[6:])
-	if err != nil {
-		return err
+	if len(data) > 6 {
+		cert, err := x509.ParseCertificate(data[6:])
+		if err != nil {
+			return err
+		}
+		h.certificate = cert
 	}
-	h.certificate = cert
-
 	return nil
 }
diff --git a/server_handlers.go b/server_handlers.go
index 5d942e1..321789c 100644
--- a/server_handlers.go
+++ b/server_handlers.go
@@ -73,10 +73,24 @@ func serverHandshakeHandler(c *Conn) error {
 				handshakeCachePullRule{handshakeTypeClientKeyExchange, true},
 			)
 
-			if err := verifyCertificateVerify(plainText, h.hashAlgorithm, h.signature, c.state.remoteCertificate); err != nil {
-				return err
+			verified := false
+			if !c.insecureSkipVerify && c.clientAuth >= RequireAnyClientCert {
+				if err := verifyCertificateVerify(plainText, h.hashAlgorithm, h.signature, c.state.remoteCertificate); err != nil {
+					return err
+				}
+				if c.clientAuth >= VerifyClientCertIfGiven {
+					if err := verifyClientCert(c.state.remoteCertificate, c.rootCAs); err != nil {
+						return err
+					}
+					verified = true
+				}
+			}
+			if c.verifyPeerCertificate != nil {
+				if err := c.verifyPeerCertificate(c.state.remoteCertificate, verified); err != nil {
+					return err
+				}
 			}
-			c.remoteCertificateVerified = true
+			c.remoteCertificateVerified = verified
 
 		case *handshakeMessageCertificate:
 			c.state.remoteCertificate = h.certificate
@@ -200,10 +214,22 @@ func serverHandshakeHandler(c *Conn) error {
 			return err
 		}
 
-		if c.clientAuth == RequireAnyClientCert && c.state.remoteCertificate == nil {
-			return errClientCertificateRequired
-		} else if c.state.remoteCertificate != nil && !c.remoteCertificateVerified {
-			return errClientCertificateNotVerified
+		switch c.clientAuth {
+		case RequireAnyClientCert:
+			if c.state.remoteCertificate == nil {
+				return errClientCertificateRequired
+			}
+		case VerifyClientCertIfGiven:
+			if c.state.remoteCertificate != nil && !c.remoteCertificateVerified {
+				return errClientCertificateNotVerified
+			}
+		case RequireAndVerifyClientCert:
+			if c.state.remoteCertificate == nil {
+				return errClientCertificateRequired
+			}
+			if !c.remoteCertificateVerified {
+				return errClientCertificateNotVerified
+			}
 		}
 
 		switch {


Test patch:
diff --git a/bench_test.go b/bench_test.go
index e700d4a..d5d03c9 100644
--- a/bench_test.go
+++ b/bench_test.go
@@ -16,9 +16,10 @@ func TestSimpleReadWrite(t *testing.T) {
 		t.Fatal(err)
 	}
 	config := &Config{
-		Certificate:   certificate,
-		PrivateKey:    privateKey,
-		LoggerFactory: logging.NewDefaultLoggerFactory(),
+		Certificate:        certificate,
+		PrivateKey:         privateKey,
+		LoggerFactory:      logging.NewDefaultLoggerFactory(),
+		InsecureSkipVerify: true,
 	}
 	gotHello := make(chan struct{})
 
@@ -54,7 +55,7 @@ func benchmarkConn(b *testing.B, n int64) {
 	b.Run(fmt.Sprintf("%d", n), func(b *testing.B) {
 		ca, cb := net.Pipe()
 		certificate, privateKey, err := GenerateSelfSigned()
-		config := &Config{Certificate: certificate, PrivateKey: privateKey}
+		config := &Config{Certificate: certificate, PrivateKey: privateKey, InsecureSkipVerify: true}
 		server := make(chan *Conn)
 		go func() {
 			s, sErr := testServer(cb, config, false)
diff --git a/conn_test.go b/conn_test.go
index 64670cc..3317775 100644
--- a/conn_test.go
+++ b/conn_test.go
@@ -2,6 +2,7 @@ package dtls
 
 import (
 	"bytes"
+	"crypto/x509"
 	"errors"
 	"fmt"
 	"io"
@@ -112,6 +113,7 @@ func testClient(c net.Conn, cfg *Config, generateCertificate bool) (*Conn, error
 		cfg.PrivateKey = clientKey
 		cfg.Certificate = clientCert
 	}
+	cfg.InsecureSkipVerify = true
 	return Client(c, cfg)
 }
 
@@ -360,48 +362,239 @@ func TestSRTPConfiguration(t *testing.T) {
 }
 
 func TestClientCertificate(t *testing.T) {
-	ca, cb := net.Pipe()
-	type result struct {
-		c    *Conn
-		conf *Config
-		err  error
+	srvCert, srvKey, err := GenerateSelfSigned()
+	if err != nil {
+		t.Fatal(err)
 	}
-	c := make(chan result)
+	srvCAPool := x509.NewCertPool()
+	srvCAPool.AddCert(srvCert)
 
-	go func() {
-		conf := &Config{ClientAuth: RequireAnyClientCert}
-		client, err := testClient(ca, conf, true)
-		c <- result{client, conf, err}
-	}()
-
-	serverCfg := &Config{ClientAuth: RequireAnyClientCert}
-	server, err := testServer(cb, serverCfg, true)
+	cert, key, err := GenerateSelfSigned()
 	if err != nil {
-		t.Errorf("TestClientCertificate: Server failed(%v)", err)
+		t.Fatal(err)
 	}
-
-	res := <-c
-	if res.err != nil {
-		t.Errorf("TestClientCertificate: Client failed(%v)", res.err)
+	caPool := x509.NewCertPool()
+	caPool.AddCert(cert)
+
+	t.Parallel()
+	tests := map[string]struct {
+		clientCfg *Config
+		serverCfg *Config
+		wantErr   bool
+	}{
+		"NoClientCert": {
+			clientCfg: &Config{RootCAs: srvCAPool},
+			serverCfg: &Config{
+				Certificate: srvCert,
+				PrivateKey:  srvKey,
+				ClientAuth:  NoClientCert,
+				RootCAs:     caPool,
+			},
+		},
+		"NoClientCert_cert": {
+			clientCfg: &Config{RootCAs: srvCAPool, Certificate: cert, PrivateKey: key},
+			serverCfg: &Config{
+				Certificate: srvCert,
+				PrivateKey:  srvKey,
+				ClientAuth:  RequireAnyClientCert,
+			},
+		},
+		"RequestClientCert_cert": {
+			clientCfg: &Config{RootCAs: srvCAPool, Certificate: cert, PrivateKey: key},
+			serverCfg: &Config{
+				Certificate: srvCert,
+				PrivateKey:  srvKey,
+				ClientAuth:  RequestClientCert,
+			},
+		},
+		"RequestClientCert_no_cert": {
+			clientCfg: &Config{RootCAs: srvCAPool},
+			serverCfg: &Config{
+				Certificate: srvCert,
+				PrivateKey:  srvKey,
+				ClientAuth:  RequestClientCert,
+				RootCAs:     caPool,
+			},
+		},
+		"RequireAnyClientCert": {
+			clientCfg: &Config{RootCAs: srvCAPool, Certificate: cert, PrivateKey: key},
+			serverCfg: &Config{
+				Certificate: srvCert,
+				PrivateKey:  srvKey,
+				ClientAuth:  RequireAnyClientCert,
+			},
+		},
+		"RequireAnyClientCert_error": {
+			clientCfg: &Config{RootCAs: srvCAPool},
+			serverCfg: &Config{
+				Certificate: srvCert,
+				PrivateKey:  srvKey,
+				ClientAuth:  RequireAnyClientCert,
+			},
+			wantErr: true,
+		},
+		"VerifyClientCertIfGiven_no_cert": {
+			clientCfg: &Config{RootCAs: srvCAPool},
+			serverCfg: &Config{
+				Certificate: srvCert,
+				PrivateKey:  srvKey,
+				ClientAuth:  VerifyClientCertIfGiven,
+				RootCAs:     caPool,
+			},
+		},
+		"VerifyClientCertIfGiven_cert": {
+			clientCfg: &Config{RootCAs: srvCAPool, Certificate: cert, PrivateKey: key},
+			serverCfg: &Config{
+				Certificate: srvCert,
+				PrivateKey:  srvKey,
+				ClientAuth:  VerifyClientCertIfGiven,
+				RootCAs:     caPool,
+			},
+		},
+		"VerifyClientCertIfGiven_error": {
+			clientCfg: &Config{RootCAs: srvCAPool, Certificate: cert, PrivateKey: key},
+			serverCfg: &Config{
+				Certificate: srvCert,
+				PrivateKey:  srvKey,
+				ClientAuth:  VerifyClientCertIfGiven,
+			},
+			wantErr: true,
+		},
+		"RequireAndVerifyClientCert": {
+			clientCfg: &Config{RootCAs: srvCAPool, Certificate: cert, PrivateKey: key},
+			serverCfg: &Config{
+				Certificate: srvCert,
+				PrivateKey:  srvKey,
+				ClientAuth:  RequireAndVerifyClientCert,
+				RootCAs:     caPool,
+			},
+		},
 	}
+	for name, tt := range tests {
+		tt := tt
+		t.Run(name, func(t *testing.T) {
+			ca, cb := net.Pipe()
+			type result struct {
+				c   *Conn
+				err error
+			}
+			c := make(chan result)
 
-	actualClientCert := server.RemoteCertificate()
-	if actualClientCert == nil {
-		t.Errorf("TestClientCertificate: Client did not provide a certificate")
-	}
+			go func() {
+				client, err := Client(ca, tt.clientCfg)
+				c <- result{client, err}
+			}()
+
+			server, err := Server(cb, tt.serverCfg)
+			res := <-c
+
+			if tt.wantErr {
+				if err != nil {
+					// Error expected, test succeeded
+					return
+				}
+				t.Error("Error expected")
+			}
+			if err != nil {
+				t.Errorf("TestClientCertificate: Server failed(%v)", err)
+			}
+
+			if res.err != nil {
+				t.Errorf("TestClientCertificate: Client failed(%v)", res.err)
+			}
+			actualClientCert := server.RemoteCertificate()
+			if tt.serverCfg.ClientAuth == RequireAnyClientCert || tt.serverCfg.ClientAuth == RequireAndVerifyClientCert {
+				if actualClientCert == nil {
+					t.Errorf("TestClientCertificate: Client did not provide a certificate")
+				}
 
-	actualServerCert := res.c.RemoteCertificate()
-	if actualServerCert == nil {
-		t.Errorf("TestClientCertificate: Server did not provide a certificate")
+				if !actualClientCert.Equal(tt.clientCfg.Certificate) {
+					t.Errorf("TestClientCertificate: Client certificate was not communicated correctly")
+				}
+			}
+			if tt.serverCfg.ClientAuth == NoClientCert {
+				if actualClientCert != nil {
+					t.Errorf("TestClientCertificate: Client certificate wasn't expected")
+				}
+			}
+
+			actualServerCert := res.c.RemoteCertificate()
+			if actualServerCert == nil {
+				t.Errorf("TestClientCertificate: Server did not provide a certificate")
+			}
+
+			if !actualServerCert.Equal(tt.serverCfg.Certificate) {
+				t.Errorf("TestClientCertificate: Server certificate was not communicated correctly")
+			}
+		})
 	}
+}
+
+func TestServerCertificate(t *testing.T) {
+	t.Parallel()
 
-	if !actualServerCert.Equal(serverCfg.Certificate) {
-		t.Errorf("TestClientCertificate: Server certificate was not communicated correctly")
+	cert, key, err := GenerateSelfSigned()
+	if err != nil {
+		t.Fatal(err)
 	}
+	caPool := x509.NewCertPool()
+	caPool.AddCert(cert)
 
-	if !actualClientCert.Equal(res.conf.Certificate) {
-		t.Errorf("TestClientCertificate: Server certificate was not communicated correctly")
+	tests := map[string]struct {
+		clientCfg *Config
+		serverCfg *Config
+		wantErr   bool
+	}{
+		"no_ca": {
+			clientCfg: &Config{},
+			serverCfg: &Config{Certificate: cert, PrivateKey: key, ClientAuth: NoClientCert},
+			wantErr:   true,
+		},
+		"good_ca": {
+			clientCfg: &Config{RootCAs: caPool},
+			serverCfg: &Config{Certificate: cert, PrivateKey: key, ClientAuth: NoClientCert},
+		},
+		"no_ca_skip_verify": {
+			clientCfg: &Config{InsecureSkipVerify: true},
+			serverCfg: &Config{Certificate: cert, PrivateKey: key, ClientAuth: NoClientCert},
+		},
+		"good_ca_custom_verify_peer": {
+			clientCfg: &Config{
+				RootCAs: caPool,
+				VerifyPeerCertificate: func(*x509.Certificate, bool) error {
+					return errors.New("wrong cert")
+				},
+			},
+			serverCfg: &Config{Certificate: cert, PrivateKey: key, ClientAuth: NoClientCert},
+			wantErr:   true,
+		},
+		"server_name": {
+			clientCfg: &Config{RootCAs: caPool, ServerName: cert.Subject.CommonName},
+			serverCfg: &Config{Certificate: cert, PrivateKey: key, ClientAuth: NoClientCert},
+		},
+		"server_name_error": {
+			clientCfg: &Config{RootCAs: caPool, ServerName: "barfoo"},
+			serverCfg: &Config{Certificate: cert, PrivateKey: key, ClientAuth: NoClientCert},
+			wantErr:   true,
+		},
 	}
+	for name, tt := range tests {
+		tt := tt
+		t.Run(name, func(t *testing.T) {
+			ca, cb := net.Pipe()
+			go func() {
+				_, _ = Server(cb, tt.serverCfg)
+			}()
+			_, err := Client(ca, tt.clientCfg)
+			if !tt.wantErr && err != nil {
+				t.Errorf("TestClientCertificate: Client failed(%v)", err)
+			}
+			if tt.wantErr && err == nil {
+				t.Fatal("Error expected")
+			}
+		})
+	}
+
 }
 
 func TestCipherSuiteConfiguration(t *testing.T) {
diff --git a/e2e/e2e_lossy_test.go b/e2e/e2e_lossy_test.go
index 9085c9e..0e92fde 100644
--- a/e2e/e2e_lossy_test.go
+++ b/e2e/e2e_lossy_test.go
@@ -89,8 +89,9 @@ func TestPionE2ELossy(t *testing.T) {
 
 		go func() {
 			cfg := &dtls.Config{
-				FlightInterval: flightInterval,
-				CipherSuites:   test.CipherSuites,
+				FlightInterval:     flightInterval,
+				CipherSuites:       test.CipherSuites,
+				InsecureSkipVerify: true,
 			}
 			if test.DoClientAuth {
 				cfg.Certificate = clientCert
diff --git a/e2e/e2e_test.go b/e2e/e2e_test.go
index ce862e8..4ef45b7 100644
--- a/e2e/e2e_test.go
+++ b/e2e/e2e_test.go
@@ -172,7 +172,12 @@ func TestPionE2ESimple(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		cfg := &dtls.Config{Certificate: cert, PrivateKey: key, CipherSuites: []dtls.CipherSuiteID{cipherSuite}}
+		cfg := &dtls.Config{
+			Certificate:        cert,
+			PrivateKey:         key,
+			CipherSuites:       []dtls.CipherSuiteID{cipherSuite},
+			InsecureSkipVerify: true,
+		}
 		assertE2ECommunication(cfg, cfg, t)
 
 	}
diff --git a/resume_test.go b/resume_test.go
index a2f2b45..bc469a5 100644
--- a/resume_test.go
+++ b/resume_test.go
@@ -41,7 +41,7 @@ func DoTestResume(t *testing.T, newLocal, newRemote func(net.Conn, *Config) (*Co
 			t.Fatal(err)
 		}
 	}()
-	config := &Config{Certificate: certificate, PrivateKey: privateKey}
+	config := &Config{Certificate: certificate, PrivateKey: privateKey, InsecureSkipVerify: true}
 	go func() {
 		var remote *Conn
 		var errR error