Skip to content

Commit

Permalink
Close DTLS when fingerprint verification fails
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Sean-Der committed Mar 17, 2021
1 parent c901d6f commit 545613d
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 4 deletions.
19 changes: 15 additions & 4 deletions dtlstransport.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -49,6 +50,7 @@ type DTLSTransport struct {
dtlsMatcher mux.MatchFunc

api *API
log logging.LeveledLogger
}

// NewDTLSTransport creates a new DTLSTransport.
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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()
}

Expand Down
10 changes: 10 additions & 0 deletions dtlstransport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 545613d

Please sign in to comment.