generic-library/webrtc
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Fix the SrtpFilter crash caused by two local offers.

Browse Source 
BUG=http://crbug.com/421774
R=juberti@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/30789004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@7530 4adac7df-926f-26a2-2b94-8c16560cd09d
This commit is contained in:
pthatcher@webrtc.org 2014-10-27 16:10:29 +00:00
parent efc82c2c73
commit 2e7ee4b28b
2 changed files with 51 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
talk/session/media/srtpfilter.cc
View File

@ -150,7 +150,7 @@ bool SrtpFilter::SetRtpParams(const std::string& send_cs,
const uint8* send_key, int send_key_len,
const std::string& recv_cs,
const uint8* recv_key, int recv_key_len) {
if (state_ == ST_ACTIVE) {
if (IsActive()) {
LOG(LS_ERROR) << "Tried to set SRTP Params when filter already active";
return false;
}
@ -212,6 +212,7 @@ bool SrtpFilter::ProtectRtp(void* p, int in_len, int max_len, int* out_len) {
LOG(LS_WARNING) << "Failed to ProtectRtp: SRTP not active";
return false;
}
ASSERT(send_session_ != NULL);
return send_session_->ProtectRtp(p, in_len, max_len, out_len);
}
@ -221,7 +222,7 @@ bool SrtpFilter::ProtectRtp(void* p, int in_len, int max_len, int* out_len,
LOG(LS_WARNING) << "Failed to ProtectRtp: SRTP not active";
return false;
}
ASSERT(send_session_ != NULL);
return send_session_->ProtectRtp(p, in_len, max_len, out_len, index);
}
@ -233,6 +234,7 @@ bool SrtpFilter::ProtectRtcp(void* p, int in_len, int max_len, int* out_len) {
if (send_rtcp_session_) {
return send_rtcp_session_->ProtectRtcp(p, in_len, max_len, out_len);
} else {
ASSERT(send_session_ != NULL);
return send_session_->ProtectRtcp(p, in_len, max_len, out_len);
}
}
@ -242,6 +244,7 @@ bool SrtpFilter::UnprotectRtp(void* p, int in_len, int* out_len) {
LOG(LS_WARNING) << "Failed to UnprotectRtp: SRTP not active";
return false;
}
ASSERT(recv_session_ != NULL);
return recv_session_->UnprotectRtp(p, in_len, out_len);
}
@ -253,6 +256,7 @@ bool SrtpFilter::UnprotectRtcp(void* p, int in_len, int* out_len) {
if (recv_rtcp_session_) {
return recv_rtcp_session_->UnprotectRtcp(p, in_len, out_len);
} else {
ASSERT(recv_session_ != NULL);
return recv_session_->UnprotectRtcp(p, in_len, out_len);
}
}
@ -263,13 +267,16 @@ bool SrtpFilter::GetRtpAuthParams(uint8** key, int* key_len, int* tag_len) {
return false;
}
ASSERT(send_session_ != NULL);
return send_session_->GetRtpAuthParams(key, key_len, tag_len);
}
void SrtpFilter::set_signal_silent_time(uint32 signal_silent_time_in_ms) {
signal_silent_time_in_ms_ = signal_silent_time_in_ms;
if (state_ == ST_ACTIVE) {
if (IsActive()) {
ASSERT(send_session_ != NULL);
send_session_->set_signal_silent_time(signal_silent_time_in_ms);
ASSERT(recv_session_ != NULL);
recv_session_->set_signal_silent_time(signal_silent_time_in_ms);
if (send_rtcp_session_)
send_rtcp_session_->set_signal_silent_time(signal_silent_time_in_ms);
@ -292,7 +299,7 @@ bool SrtpFilter::StoreParams(const std::vector<CryptoParams>& params,
offer_params_ = params;
if (state_ == ST_INIT) {
state_ = (source == CS_LOCAL) ? ST_SENTOFFER : ST_RECEIVEDOFFER;
} else { // state >= ST_ACTIVE
} else if (state_ == ST_ACTIVE) {
state_ =
(source == CS_LOCAL) ? ST_SENTUPDATEDOFFER : ST_RECEIVEDUPDATEDOFFER;
}

40
talk/session/media/srtpfilter_unittest.cc
View File

@ -82,9 +82,12 @@ class SrtpFilterTest : public testing::Test {
const std::vector<CryptoParams>& params2) {
EXPECT_TRUE(f1_.SetOffer(params1, CS_LOCAL));
EXPECT_TRUE(f2_.SetOffer(params1, CS_REMOTE));
EXPECT_FALSE(f1_.IsActive());
EXPECT_FALSE(f2_.IsActive());
EXPECT_TRUE(f2_.SetAnswer(params2, CS_LOCAL));
EXPECT_TRUE(f1_.SetAnswer(params2, CS_REMOTE));
EXPECT_TRUE(f1_.IsActive());
EXPECT_TRUE(f2_.IsActive());
}
void TestProtectUnprotect(const std::string& cs1, const std::string& cs2) {
char rtp_packet[sizeof(kPcmuFrame) + 10];
@ -139,6 +142,7 @@ class SrtpFilterTest : public testing::Test {
// Test that we can set up the session and keys properly.
TEST_F(SrtpFilterTest, TestGoodSetupOneCipherSuite) {
EXPECT_TRUE(f1_.SetOffer(MakeVector(kTestCryptoParams1), CS_LOCAL));
EXPECT_FALSE(f1_.IsActive());
EXPECT_TRUE(f1_.SetAnswer(MakeVector(kTestCryptoParams2), CS_REMOTE));
EXPECT_TRUE(f1_.IsActive());
}
@ -153,6 +157,7 @@ TEST_F(SrtpFilterTest, TestGoodSetupMultipleCipherSuites) {
answer[0].tag = 2;
answer[0].cipher_suite = CS_AES_CM_128_HMAC_SHA1_32;
EXPECT_TRUE(f1_.SetOffer(offer, CS_LOCAL));
EXPECT_FALSE(f1_.IsActive());
EXPECT_TRUE(f1_.SetAnswer(answer, CS_REMOTE));
EXPECT_TRUE(f1_.IsActive());
}
@ -188,6 +193,7 @@ TEST_F(SrtpFilterTest, TestBadSetup) {
TEST_F(SrtpFilterTest, TestGoodSetupMultipleOffers) {
EXPECT_TRUE(f1_.SetOffer(MakeVector(kTestCryptoParams1), CS_LOCAL));
EXPECT_TRUE(f1_.SetOffer(MakeVector(kTestCryptoParams2), CS_LOCAL));
EXPECT_FALSE(f1_.IsActive());
EXPECT_TRUE(f1_.SetAnswer(MakeVector(kTestCryptoParams2), CS_REMOTE));
EXPECT_TRUE(f1_.IsActive());
EXPECT_TRUE(f1_.SetOffer(MakeVector(kTestCryptoParams1), CS_LOCAL));
@ -196,6 +202,7 @@ TEST_F(SrtpFilterTest, TestGoodSetupMultipleOffers) {
EXPECT_TRUE(f2_.SetOffer(MakeVector(kTestCryptoParams1), CS_REMOTE));
EXPECT_TRUE(f2_.SetOffer(MakeVector(kTestCryptoParams2), CS_REMOTE));
EXPECT_FALSE(f2_.IsActive());
EXPECT_TRUE(f2_.SetAnswer(MakeVector(kTestCryptoParams2), CS_LOCAL));
EXPECT_TRUE(f2_.IsActive());
EXPECT_TRUE(f2_.SetOffer(MakeVector(kTestCryptoParams1), CS_REMOTE));
@ -206,6 +213,7 @@ TEST_F(SrtpFilterTest, TestGoodSetupMultipleOffers) {
TEST_F(SrtpFilterTest, TestBadSetupMultipleOffers) {
EXPECT_TRUE(f1_.SetOffer(MakeVector(kTestCryptoParams1), CS_LOCAL));
EXPECT_FALSE(f1_.SetOffer(MakeVector(kTestCryptoParams2), CS_REMOTE));
EXPECT_FALSE(f1_.IsActive());
EXPECT_TRUE(f1_.SetAnswer(MakeVector(kTestCryptoParams1), CS_REMOTE));
EXPECT_TRUE(f1_.IsActive());
EXPECT_TRUE(f1_.SetOffer(MakeVector(kTestCryptoParams2), CS_LOCAL));
@ -214,6 +222,7 @@ TEST_F(SrtpFilterTest, TestBadSetupMultipleOffers) {
EXPECT_TRUE(f2_.SetOffer(MakeVector(kTestCryptoParams2), CS_REMOTE));
EXPECT_FALSE(f2_.SetOffer(MakeVector(kTestCryptoParams1), CS_LOCAL));
EXPECT_FALSE(f2_.IsActive());
EXPECT_TRUE(f2_.SetAnswer(MakeVector(kTestCryptoParams2), CS_LOCAL));
EXPECT_TRUE(f2_.IsActive());
EXPECT_TRUE(f2_.SetOffer(MakeVector(kTestCryptoParams2), CS_REMOTE));
@ -402,6 +411,8 @@ TEST_F(SrtpFilterTest, TestProvisionalAnswer) {
EXPECT_TRUE(f1_.SetOffer(offer, CS_LOCAL));
EXPECT_TRUE(f2_.SetOffer(offer, CS_REMOTE));
EXPECT_FALSE(f1_.IsActive());
EXPECT_FALSE(f2_.IsActive());
EXPECT_TRUE(f2_.SetProvisionalAnswer(answer, CS_LOCAL));
EXPECT_TRUE(f1_.SetProvisionalAnswer(answer, CS_REMOTE));
EXPECT_TRUE(f1_.IsActive());
@ -425,6 +436,8 @@ TEST_F(SrtpFilterTest, TestProvisionalAnswerWithoutCrypto) {
EXPECT_TRUE(f1_.SetOffer(offer, CS_LOCAL));
EXPECT_TRUE(f2_.SetOffer(offer, CS_REMOTE));
EXPECT_FALSE(f1_.IsActive());
EXPECT_FALSE(f2_.IsActive());
EXPECT_TRUE(f2_.SetProvisionalAnswer(answer, CS_LOCAL));
EXPECT_TRUE(f1_.SetProvisionalAnswer(answer, CS_REMOTE));
EXPECT_FALSE(f1_.IsActive());
@ -438,6 +451,33 @@ TEST_F(SrtpFilterTest, TestProvisionalAnswerWithoutCrypto) {
TestProtectUnprotect(CS_AES_CM_128_HMAC_SHA1_80, CS_AES_CM_128_HMAC_SHA1_80);
}
// Test that if we get a new local offer after a provisional answer
// with no crypto, that we are in an inactive state.
TEST_F(SrtpFilterTest, TestLocalOfferAfterProvisionalAnswerWithoutCrypto) {
std::vector<CryptoParams> offer(MakeVector(kTestCryptoParams1));
std::vector<CryptoParams> answer;
EXPECT_TRUE(f1_.SetOffer(offer, CS_LOCAL));
EXPECT_TRUE(f2_.SetOffer(offer, CS_REMOTE));
EXPECT_TRUE(f1_.SetProvisionalAnswer(answer, CS_REMOTE));
EXPECT_TRUE(f2_.SetProvisionalAnswer(answer, CS_LOCAL));
EXPECT_FALSE(f1_.IsActive());
EXPECT_FALSE(f2_.IsActive());
// The calls to set an offer after a provisional answer fail, so the
// state doesn't change.
EXPECT_FALSE(f1_.SetOffer(offer, CS_LOCAL));
EXPECT_FALSE(f2_.SetOffer(offer, CS_REMOTE));
EXPECT_FALSE(f1_.IsActive());
EXPECT_FALSE(f2_.IsActive());
answer.push_back(kTestCryptoParams2);
EXPECT_TRUE(f2_.SetAnswer(answer, CS_LOCAL));
EXPECT_TRUE(f1_.SetAnswer(answer, CS_REMOTE));
EXPECT_TRUE(f1_.IsActive());
EXPECT_TRUE(f2_.IsActive());
TestProtectUnprotect(CS_AES_CM_128_HMAC_SHA1_80, CS_AES_CM_128_HMAC_SHA1_80);
}
// Test that we can disable encryption.
TEST_F(SrtpFilterTest, TestDisableEncryption) {
std::vector<CryptoParams> offer(MakeVector(kTestCryptoParams1));