Skip to content

Commit a3ec2e6

Browse files
committed
Fix packet length validation, new cipher tests
This is v2 CP of two commits from master: Fix packet length validation Missing packet length validations could cause crash. New SRTP cipher test suite Moved tests from srtp_cipher_aead_aes_gcm_test.go to new file, cleaned up their code to make it more DRY. Added tests for AES CM ciphers there too. This new cipher test suite will make it easier to add tests for new SRTP features without lots of copy/paste.
1 parent f598d3b commit a3ec2e6

File tree

9 files changed

+256
-169
lines changed

9 files changed

+256
-169
lines changed

errors.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ var (
1818
errNoConfig = errors.New("no config provided")
1919
errNoConn = errors.New("no conn provided")
2020
errFailedToVerifyAuthTag = errors.New("failed to verify auth tag")
21-
errTooShortRTCP = errors.New("packet is too short to be rtcp packet")
21+
errTooShortRTP = errors.New("packet is too short to be RTP packet")
22+
errTooShortRTCP = errors.New("packet is too short to be RTCP packet")
2223
errPayloadDiffers = errors.New("payload differs")
2324
errStartedChannelUsedIncorrectly = errors.New("started channel used incorrectly, should only be closed")
2425
errBadIVLength = errors.New("bad iv length in xorBytesCTR")

protection_profile.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,3 +84,18 @@ func (p ProtectionProfile) authKeyLen() (int, error) {
8484
return 0, fmt.Errorf("%w: %#v", errNoSuchSRTPProfile, p)
8585
}
8686
}
87+
88+
func (p ProtectionProfile) String() string {
89+
switch p {
90+
case ProtectionProfileAes128CmHmacSha1_80:
91+
return "SRTP_AES128_CM_HMAC_SHA1_80"
92+
case ProtectionProfileAes128CmHmacSha1_32:
93+
return "SRTP_AES128_CM_HMAC_SHA1_32"
94+
case ProtectionProfileAeadAes128Gcm:
95+
return "SRTP_AEAD_AES_128_GCM"
96+
case ProtectionProfileAeadAes256Gcm:
97+
return "SRTP_AEAD_AES_256_GCM"
98+
default:
99+
return fmt.Sprintf("Unknown SRTP profile: %#v", p)
100+
}
101+
}

srtcp.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,10 @@ func (c *Context) DecryptRTCP(dst, encrypted []byte, header *rtcp.Header) ([]byt
6363
}
6464

6565
func (c *Context) encryptRTCP(dst, decrypted []byte) ([]byte, error) {
66+
if len(decrypted) < 8 {
67+
return nil, fmt.Errorf("%w: %d", errTooShortRTCP, len(decrypted))
68+
}
69+
6670
ssrc := binary.BigEndian.Uint32(decrypted[4:])
6771
s := c.getSRTCPSSRCState(ssrc)
6872

srtcp_test.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -594,3 +594,27 @@ func TestRTCPReplayDetectorFactory(t *testing.T) {
594594
}
595595
assert.Equal(1, cntFactory)
596596
}
597+
598+
func TestDecryptInvalidSRTCP(t *testing.T) {
599+
assert := assert.New(t)
600+
key := []byte{0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01}
601+
salt := []byte{0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01}
602+
decryptContext, err := CreateContext(key, salt, ProtectionProfileAes128CmHmacSha1_80)
603+
assert.NoError(err)
604+
605+
packet := []byte{0x8f, 0x48, 0xff, 0xff, 0xec, 0x77, 0xb0, 0x43, 0xf9, 0x04, 0x51, 0xff, 0xfb, 0xdf}
606+
_, err = decryptContext.DecryptRTCP(nil, packet, nil)
607+
assert.Error(err)
608+
}
609+
610+
func TestEncryptInvalidRTCP(t *testing.T) {
611+
assert := assert.New(t)
612+
key := []byte{0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01}
613+
salt := []byte{0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01}
614+
decryptContext, err := CreateContext(key, salt, ProtectionProfileAes128CmHmacSha1_80)
615+
assert.NoError(err)
616+
617+
packet := []byte{0xbb, 0xbb, 0x0a, 0x2f}
618+
_, err = decryptContext.EncryptRTCP(nil, packet, nil)
619+
assert.Error(err)
620+
}

srtp.go

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,15 @@ import (
99
)
1010

1111
func (c *Context) decryptRTP(dst, ciphertext []byte, header *rtp.Header, headerLen int) ([]byte, error) {
12+
authTagLen, err := c.cipher.rtpAuthTagLen()
13+
if err != nil {
14+
return nil, err
15+
}
16+
17+
if len(ciphertext) < headerLen+authTagLen {
18+
return nil, errTooShortRTP
19+
}
20+
1221
s := c.getSRTPSSRCState(header.SSRC)
1322

1423
roc, diff, _ := s.nextRolloverCount(header.SequenceNumber)
@@ -21,10 +30,6 @@ func (c *Context) decryptRTP(dst, ciphertext []byte, header *rtp.Header, headerL
2130
}
2231
}
2332

24-
authTagLen, err := c.cipher.rtpAuthTagLen()
25-
if err != nil {
26-
return nil, err
27-
}
2833
dst = growBufferSize(dst, len(ciphertext)-authTagLen)
2934

3035
dst, err = c.cipher.decryptRTP(dst, ciphertext, header, headerLen, roc)

srtp_cipher_aead_aes_gcm_test.go

Lines changed: 0 additions & 164 deletions
This file was deleted.

srtp_cipher_aes_cm_hmac_sha1.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,9 @@ func (s *srtpCipherAesCmHmacSha1) decryptRTCP(out, encrypted []byte, index, ssrc
161161
return nil, err
162162
}
163163
tailOffset := len(encrypted) - (authTagLen + srtcpIndexSize)
164+
if tailOffset < 8 {
165+
return nil, errTooShortRTCP
166+
}
164167
out = out[0:tailOffset]
165168

166169
expectedTag, err := s.generateSrtcpAuthTag(encrypted[:len(encrypted)-authTagLen])

0 commit comments

Comments
 (0)