From 545613dcdeb5dedb01cce94175f40bcbe045df2e Mon Sep 17 00:00:00 2001
From: Sean DuBois <sean@siobud.com>
Date: Wed, 17 Mar 2021 16:21:52 -0700
Subject: [PATCH] Close DTLS when fingerprint verification fails

Before we would set the PeerConnection to failed, but we would leave the
DTLSTransport. This means that a user could still interact with the
other transports.

Relates to #1708
---
 dtlstransport.go      | 19 +++++++++++++++----
 dtlstransport_test.go | 10 ++++++++++
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/dtlstransport.go b/dtlstransport.go
index d110092900f..cc25889a789 100644
--- a/dtlstransport.go
+++ b/dtlstransport.go
@@ -17,6 +17,7 @@ import (
 
 	"github.com/pion/dtls/v2"
 	"github.com/pion/dtls/v2/pkg/crypto/fingerprint"
+	"github.com/pion/logging"
 	"github.com/pion/srtp/v2"
 	"github.com/pion/webrtc/v3/internal/mux"
 	"github.com/pion/webrtc/v3/internal/util"
@@ -49,6 +50,7 @@ type DTLSTransport struct {
 	dtlsMatcher mux.MatchFunc
 
 	api *API
+	log logging.LeveledLogger
 }
 
 // NewDTLSTransport creates a new DTLSTransport.
@@ -61,6 +63,7 @@ func (api *API) NewDTLSTransport(transport *ICETransport, certificates []Certifi
 		state:        DTLSTransportStateNew,
 		dtlsMatcher:  mux.MatchDTLS,
 		srtpReady:    make(chan struct{}),
+		log:          api.settingEngine.LoggerFactory.NewLogger("DTLSTransport"),
 	}
 
 	if len(certificates) > 0 {
@@ -324,15 +327,12 @@ func (t *DTLSTransport) Start(remoteParameters DTLSParameters) error {
 		return ErrNoSRTPProtectionProfile
 	}
 
-	t.conn = dtlsConn
-	t.onStateChange(DTLSTransportStateConnected)
-
 	if t.api.settingEngine.disableCertificateFingerprintVerification {
 		return nil
 	}
 
 	// Check the fingerprint if a certificate was exchanged
-	remoteCerts := t.conn.ConnectionState().PeerCertificates
+	remoteCerts := dtlsConn.ConnectionState().PeerCertificates
 	if len(remoteCerts) == 0 {
 		t.onStateChange(DTLSTransportStateFailed)
 		return errNoRemoteCertificate
@@ -341,15 +341,26 @@ func (t *DTLSTransport) Start(remoteParameters DTLSParameters) error {
 
 	parsedRemoteCert, err := x509.ParseCertificate(t.remoteCertificate)
 	if err != nil {
+		if closeErr := dtlsConn.Close(); closeErr != nil {
+			t.log.Error(err.Error())
+		}
+
 		t.onStateChange(DTLSTransportStateFailed)
 		return err
 	}
 
 	if err = t.validateFingerPrint(parsedRemoteCert); err != nil {
+		if closeErr := dtlsConn.Close(); closeErr != nil {
+			t.log.Error(err.Error())
+		}
+
 		t.onStateChange(DTLSTransportStateFailed)
 		return err
 	}
 
+	t.conn = dtlsConn
+	t.onStateChange(DTLSTransportStateConnected)
+
 	return t.startSRTP()
 }
 
diff --git a/dtlstransport_test.go b/dtlstransport_test.go
index 14de9701a1e..a4aabe53891 100644
--- a/dtlstransport_test.go
+++ b/dtlstransport_test.go
@@ -29,6 +29,10 @@ func TestInvalidFingerprintCausesFailed(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	pcAnswer.OnDataChannel(func(_ *DataChannel) {
+		t.Fatal("A DataChannel must not be created when Fingerprint verification fails")
+	})
+
 	defer closePairNow(t, pcOffer, pcAnswer)
 
 	offerChan := make(chan SessionDescription)
@@ -83,6 +87,12 @@ func TestInvalidFingerprintCausesFailed(t *testing.T) {
 
 	offerConnectionHasFailed.Wait()
 	answerConnectionHasFailed.Wait()
+
+	assert.Equal(t, pcOffer.SCTP().Transport().State(), DTLSTransportStateFailed)
+	assert.Nil(t, pcOffer.SCTP().Transport().conn)
+
+	assert.Equal(t, pcAnswer.SCTP().Transport().State(), DTLSTransportStateFailed)
+	assert.Nil(t, pcAnswer.SCTP().Transport().conn)
 }
 
 func TestPeerConnection_DTLSRoleSettingEngine(t *testing.T) {