Bug 1437169: Improved error checking in the the fingerprint parsing, r=bwc
Improved the error checking in the fingerprint parsing
Changed the way the fingeerprint stores the hash algorihtm to an enum.
Extended the rust unit test for fingerprint
Fixed C++ unit tests
MozReview-Commit-ID: AS2FroZxDNv
--- a/media/webrtc/signaling/gtest/sdp_unittests.cpp
+++ b/media/webrtc/signaling/gtest/sdp_unittests.cpp
@@ -1868,17 +1868,17 @@ const std::string kBasicAudioVideoOffer
"a=candidate:2 2 UDP 1694236670 24.6.134.204 55428 typ srflx raddr 10.0.0.36 rport 55428" CRLF
"a=candidate:6 2 UDP 16515070 162.222.183.171 50340 typ relay raddr 162.222.183.171 rport 50340" CRLF
"a=candidate:0 2 UDP 2130379006 10.0.0.36 55428 typ host" CRLF
"a=rtcp:62454 IN IP4 162.222.183.171" CRLF
"a=end-of-candidates" CRLF
"a=ssrc:5150" CRLF
"m=video 9 RTP/SAVPF 120 121 122 123" CRLF
"c=IN IP6 ::1" CRLF
-"a=fingerprint:sha-1 DF:FA:FB:08:3B:3C:54:1D:D7:D4:05:77:A0:72:9B:14:08:6D:0F:4C:2E:AC:8A:FD:0A:8E:99:BF:5D:E8:3C:E7" CRLF
+"a=fingerprint:sha-1 DF:FA:FB:08:3B:3C:54:1D:D7:D4:05:77:A0:72:9B:14:08:6D:0F:4C" CRLF
"a=mid:second" CRLF
"a=rtpmap:120 VP8/90000" CRLF
"a=fmtp:120 max-fs=3600;max-fr=30" CRLF
"a=rtpmap:121 VP9/90000" CRLF
"a=fmtp:121 max-fs=3600;max-fr=30" CRLF
"a=rtpmap:122 red/90000" CRLF
"a=rtpmap:123 ulpfec/90000" CRLF
"a=recvonly" CRLF
@@ -2051,17 +2051,17 @@ TEST_P(NewSdpTest, CheckFingerprint) {
ASSERT_TRUE(mSdp->GetMediaSection(1).GetAttributeList().HasAttribute(
SdpAttribute::kFingerprintAttribute));
fingerprints = mSdp->GetMediaSection(1).GetAttributeList().GetFingerprint();
ASSERT_EQ(1U, fingerprints.mFingerprints.size());
ASSERT_EQ(SdpFingerprintAttributeList::kSha1,
fingerprints.mFingerprints[0].hashFunc)
<< "Wrong hash function";
ASSERT_EQ("DF:FA:FB:08:3B:3C:54:1D:D7:D4:05:77:A0:72:9B:14:"
- "08:6D:0F:4C:2E:AC:8A:FD:0A:8E:99:BF:5D:E8:3C:E7",
+ "08:6D:0F:4C",
SdpFingerprintAttributeList::FormatFingerprint(
fingerprints.mFingerprints[0].fingerprint))
<< "Wrong fingerprint";
ASSERT_EQ(0xdfU, fingerprints.mFingerprints[0].fingerprint[0])
<< "first fingerprint element is iffy";
}
TEST_P(NewSdpTest, CheckIdentity) {
@@ -2488,17 +2488,17 @@ TEST_P(NewSdpTest, CheckTelephoneEventBa
static const std::string kVideoWithRedAndUlpfecSdp =
"v=0" CRLF
"o=- 4294967296 2 IN IP4 127.0.0.1" CRLF
"s=SIP Call" CRLF
"c=IN IP4 198.51.100.7" CRLF
"t=0 0" CRLF
"m=video 9 RTP/SAVPF 97 120 121 122 123" CRLF
"c=IN IP6 ::1" CRLF
- "a=fingerprint:sha-1 DF:FA:FB:08:3B:3C:54:1D:D7:D4:05:77:A0:72:9B:14:08:6D:0F:4C:2E:AC:8A:FD:0A:8E:99:BF:5D:E8:3C:E7" CRLF
+ "a=fingerprint:sha-1 DF:FA:FB:08:3B:3C:54:1D:D7:D4:05:77:A0:72:9B:14:08:6D:0F:4C" CRLF
"a=rtpmap:97 H264/90000" CRLF
"a=fmtp:97 profile-level-id=42a01e" CRLF
"a=rtpmap:120 VP8/90000" CRLF
"a=fmtp:120 max-fs=3600;max-fr=30" CRLF
"a=rtpmap:121 VP9/90000" CRLF
"a=fmtp:121 max-fs=3600;max-fr=30" CRLF
"a=rtpmap:122 red/90000" CRLF
"a=rtpmap:123 ulpfec/90000" CRLF;
--- a/media/webrtc/signaling/src/sdp/RsdparsaSdpAttributeList.cpp
+++ b/media/webrtc/signaling/src/sdp/RsdparsaSdpAttributeList.cpp
@@ -568,27 +568,39 @@ RsdparsaSdpAttributeList::LoadFingerprin
size_t nFp = sdp_get_fingerprint_count(attributeList);
if (nFp == 0) {
return;
}
auto rustFingerprints = MakeUnique<RustSdpAttributeFingerprint[]>(nFp);
sdp_get_fingerprints(attributeList, nFp, rustFingerprints.get());
auto fingerprints = MakeUnique<SdpFingerprintAttributeList>();
for(size_t i = 0; i < nFp; i++) {
- RustSdpAttributeFingerprint& fingerprint = rustFingerprints[i];
- std::string algorithm = convertStringView(fingerprint.hashAlgorithm);
- std::string fingerprintToken = convertStringView(fingerprint.fingerprint);
+ const RustSdpAttributeFingerprint& fingerprint = rustFingerprints[i];
+ std::string algorithm;
+ switch(fingerprint.hashAlgorithm) {
+ case RustSdpAttributeFingerprintHashAlgorithm::kSha1:
+ algorithm = "sha-1";
+ break;
+ case RustSdpAttributeFingerprintHashAlgorithm::kSha224:
+ algorithm = "sha-224";
+ break;
+ case RustSdpAttributeFingerprintHashAlgorithm::kSha256:
+ algorithm = "sha-256";
+ break;
+ case RustSdpAttributeFingerprintHashAlgorithm::kSha384:
+ algorithm = "sha-384";
+ break;
+ case RustSdpAttributeFingerprintHashAlgorithm::kSha512:
+ algorithm = "sha-512";
+ break;
+ }
+
std::vector<uint8_t> fingerprintBytes =
- SdpFingerprintAttributeList::ParseFingerprint(fingerprintToken);
- if (fingerprintBytes.size() == 0) {
- // TODO: Should we load fingerprint earlier to detect if it is malformed
- // and throw a proper error?
- // TODO: We should improve our error checking. See Bug 1437169.
- continue;
- }
+ convertU8Vec(fingerprint.fingerprint);
+
fingerprints->PushEntry(algorithm, fingerprintBytes);
}
SetAttribute(fingerprints.release());
}
void
RsdparsaSdpAttributeList::LoadDtlsMessage(RustAttributeList* attributeList)
{
--- a/media/webrtc/signaling/src/sdp/RsdparsaSdpInc.h
+++ b/media/webrtc/signaling/src/sdp/RsdparsaSdpInc.h
@@ -66,19 +66,27 @@ enum class RustSdpProtocolValue {
kRustTcpDtlsSctp,
};
enum class RustSdpFormatType {
kRustIntegers,
kRustStrings
};
+enum class RustSdpAttributeFingerprintHashAlgorithm : uint16_t {
+ kSha1,
+ kSha224,
+ kSha256,
+ kSha384,
+ kSha512,
+};
+
struct RustSdpAttributeFingerprint {
- StringView hashAlgorithm;
- StringView fingerprint;
+ RustSdpAttributeFingerprintHashAlgorithm hashAlgorithm;
+ U8Vec* fingerprint;
};
enum class RustSdpSetup {
kRustActive,
kRustActpass,
kRustHoldconn,
kRustPassive
};
--- a/media/webrtc/signaling/src/sdp/rsdparsa/src/attribute_type.rs
+++ b/media/webrtc/signaling/src/sdp/rsdparsa/src/attribute_type.rs
@@ -276,22 +276,31 @@ pub struct SdpAttributeFmtpParameters {
#[derive(Clone)]
#[cfg_attr(feature="serialize", derive(Serialize))]
pub struct SdpAttributeFmtp {
pub payload_type: u8,
pub parameters: SdpAttributeFmtpParameters,
}
+#[derive(Clone, Copy)]
+#[cfg_attr(feature="serialize", derive(Serialize))]
+pub enum SdpAttributeFingerprintHashType {
+ Sha1,
+ Sha224,
+ Sha256,
+ Sha384,
+ Sha512,
+}
+
#[derive(Clone)]
#[cfg_attr(feature="serialize", derive(Serialize))]
pub struct SdpAttributeFingerprint {
- // TODO turn the supported hash algorithms into an enum?
- pub hash_algorithm: String,
- pub fingerprint: String,
+ pub hash_algorithm: SdpAttributeFingerprintHashType,
+ pub fingerprint: Vec<u8>
}
#[derive(Clone)]
#[cfg_attr(feature="serialize", derive(Serialize))]
pub struct SdpAttributeSctpmap {
pub port: u16,
pub channels: u32,
}
@@ -954,19 +963,66 @@ fn parse_extmap(to_parse: &str) -> Resul
}
fn parse_fingerprint(to_parse: &str) -> Result<SdpAttribute, SdpParserInternalError> {
let tokens: Vec<&str> = to_parse.split_whitespace().collect();
if tokens.len() != 2 {
return Err(SdpParserInternalError::Generic("Fingerprint needs to have two tokens"
.to_string()));
}
+
+ let fingerprint_token = tokens[1].to_string();
+ let parse_tokens = |expected_len| -> Result<Vec<u8>, SdpParserInternalError>{
+ let bytes = fingerprint_token.split(":")
+ .map(|byte_token| {
+ if byte_token.len() != 2 {
+ return Err(SdpParserInternalError::Generic(
+ "fingerpint's byte tokens must have 2 hexdigits"
+ .to_string()
+ ))
+ }
+ Ok(u8::from_str_radix(byte_token, 16)?)
+ })
+ .collect::<Result<Vec<u8>,_>>()?;
+
+ if bytes.len() != expected_len {
+ return Err(SdpParserInternalError::Generic(
+ format!("fingerprint has {} bytes but should have {} bytes",
+ bytes.len(), expected_len)
+ ))
+ }
+
+ Ok(bytes)
+ };
+
+
+ let hash_algorithm = match tokens[0] {
+ "sha-1" => SdpAttributeFingerprintHashType::Sha1,
+ "sha-224" => SdpAttributeFingerprintHashType::Sha224,
+ "sha-256" => SdpAttributeFingerprintHashType::Sha256,
+ "sha-384" => SdpAttributeFingerprintHashType::Sha384,
+ "sha-512" => SdpAttributeFingerprintHashType::Sha512,
+ unknown => {
+ return Err(SdpParserInternalError::Unsupported(
+ format!("fingerprint contains an unsupported hash algorithm '{}'", unknown)
+ ))
+ }
+ };
+
+ let fingerprint = match hash_algorithm {
+ SdpAttributeFingerprintHashType::Sha1 => parse_tokens(20)?,
+ SdpAttributeFingerprintHashType::Sha224 => parse_tokens(28)?,
+ SdpAttributeFingerprintHashType::Sha256 => parse_tokens(32)?,
+ SdpAttributeFingerprintHashType::Sha384 => parse_tokens(48)?,
+ SdpAttributeFingerprintHashType::Sha512 => parse_tokens(64)?,
+ };
+
Ok(SdpAttribute::Fingerprint(SdpAttributeFingerprint {
- hash_algorithm: tokens[0].to_string(),
- fingerprint: tokens[1].to_string(),
+ hash_algorithm,
+ fingerprint,
}))
}
fn parse_fmtp(to_parse: &str) -> Result<SdpAttribute, SdpParserInternalError> {
let tokens: Vec<&str> = to_parse.splitn(2," ").collect();
if tokens.len() != 2 {
return Err(SdpParserInternalError::Unsupported(
@@ -1797,17 +1853,40 @@ fn test_parse_attribute_extmap() {
let mut bad_char = String::from("extmap:3 http://www.webrtc.org/experiments/rtp-hdrext/abs-send-time ",);
bad_char.push(0x00 as char);
assert!(parse_attribute(&bad_char).is_err());
}
#[test]
fn test_parse_attribute_fingerprint() {
- assert!(parse_attribute("fingerprint:sha-256 CD:34:D1:62:16:95:7B:B7:EB:74:E2:39:27:97:EB:0B:23:73:AC:BC:BF:2F:E3:91:CB:57:A9:9D:4A:A2:0B:40").is_ok())
+ assert!(parse_attribute("fingerprint:sha-1 CD:34:D1:62:16:95:7B:B7:EB:74:E2:39:27:97:EB:0B:23:73:AC:BC").is_ok());
+ assert!(parse_attribute("fingerprint:sha-224 CD:34:D1:62:16:95:7B:B7:EB:74:E2:39:27:97:EB:0B:23:73:AC:BC:\
+ 27:97:EB:0B:23:73:AC:BC").is_ok());
+ assert!(parse_attribute("fingerprint:sha-256 CD:34:D1:62:16:95:7B:B7:EB:74:E2:39:27:97:EB:0B:23:73:AC:BC:\
+ 27:97:EB:0B:23:73:AC:BC:CD:34:D1:62").is_ok());
+ assert!(parse_attribute("fingerprint:sha-384 CD:34:D1:62:16:95:7B:B7:EB:74:E2:39:27:97:EB:0B:23:73:AC:BC:\
+ 27:97:EB:0B:23:73:AC:BC:CD:34:D1:62:16:95:7B:B7:EB:74:E2:39:\
+ 27:97:EB:0B:23:73:AC:BC").is_ok());
+ assert!(parse_attribute("fingerprint:sha-512 CD:34:D1:62:16:95:7B:B7:EB:74:E2:39:27:97:EB:0B:23:73:AC:BC:\
+ 97:EB:0B:23:73:AC:BC:CD:34:D1:62:16:95:7B:B7:EB:74:E2:39:27:\
+ EB:0B:23:73:AC:BC:27:97:EB:0B:23:73:AC:BC:27:97:EB:0B:23:73:\
+ BC:EB:0B:23").is_ok());
+
+ assert!(parse_attribute("fingerprint:sha-1 CX:34:D1:62:16:95:7B:B7:EB:74:E1:39:27:97:EB:0B:23:73:AC:BC").is_err());
+ assert!(parse_attribute("fingerprint:sha-1 CDA:34:D1:62:16:95:7B:B7:EB:74:E1:39:27:97:EB:0B:23:73:AC:BC").is_err());
+ assert!(parse_attribute("fingerprint:sha-1 CD:34:D1:62:16:95:7B:B7:EB:74:E1:39:27:97:EB:0B:23:73:AC:").is_err());
+ assert!(parse_attribute("fingerprint:sha-1 CD:34:D1:62:16:95:7B:B7:EB:74:E1:39:27:97:EB:0B:23:73:AC").is_err());
+ assert!(parse_attribute("fingerprint:sha-1 CX:34:D1:62:16:95:7B:B7:EB:74:E1:39:27:97:EB:0B:23:73:AC:BC").is_err());
+
+ assert!(parse_attribute("fingerprint:sha-1 0xCD:34:D1:62:16:95:7B:B7:EB:74:E2:39:27:97:EB:0B:23:73:AC:BC").is_err());
+ assert!(parse_attribute("fingerprint:sha-1 CD:0x34:D1:62:16:95:7B:B7:EB:74:E2:39:27:97:EB:0B:23:73:AC:BC").is_err());
+ assert!(parse_attribute("fingerprint:sha-1 CD::D1:62:16:95:7B:B7:EB:74:E2:39:27:97:EB:0B:23:73:AC:BC").is_err());
+ assert!(parse_attribute("fingerprint:sha-1 CD:0000A:D1:62:16:95:7B:B7:EB:74:E2:39:27:97:EB:0B:23:73:AC:BC").is_err());
+ assert!(parse_attribute("fingerprint:sha-1 CD:B:D1:62:16:95:7B:B7:EB:74:E2:39:27:97:EB:0B:23:73:AC:BC").is_err());
}
#[test]
fn test_parse_attribute_fmtp() {
assert!(parse_attribute("fmtp:109 maxplaybackrate=48000;stereo=1;useinbandfec=1").is_ok());
assert!(parse_attribute("fmtp:66 0-15").is_ok());
assert!(parse_attribute("fmtp:109 0-15,66").is_ok());
assert!(parse_attribute("fmtp:66 111/115").is_ok());
--- a/media/webrtc/signaling/src/sdp/rsdparsa_capi/src/attribute.rs
+++ b/media/webrtc/signaling/src/sdp/rsdparsa_capi/src/attribute.rs
@@ -232,32 +232,29 @@ pub unsafe extern "C" fn sdp_get_maxptim
return NS_OK;
}
NS_ERROR_INVALID_ARG
}
#[repr(C)]
#[derive(Clone, Copy)]
pub struct RustSdpAttributeFingerprint {
- hash_algorithm: StringView,
- fingerprint: StringView
+ hash_algorithm: uint16_t,
+ fingerprint: *const Vec<u8>
}
impl<'a> From<&'a SdpAttributeFingerprint> for RustSdpAttributeFingerprint {
fn from(other: &SdpAttributeFingerprint) -> Self {
RustSdpAttributeFingerprint {
- hash_algorithm: StringView::from(other.hash_algorithm.as_str()),
- fingerprint: StringView::from(other.fingerprint.as_str())
+ hash_algorithm: other.hash_algorithm as uint16_t,
+ fingerprint: &other.fingerprint,
}
}
}
-
-
-
#[no_mangle]
pub unsafe extern "C" fn sdp_get_fingerprint_count(attributes: *const Vec<SdpAttribute>) -> size_t {
count_attribute((*attributes).as_slice(), RustSdpAttributeType::Fingerprint)
}
#[no_mangle]
pub unsafe extern "C" fn sdp_get_fingerprints(attributes: *const Vec<SdpAttribute>, ret_size: size_t, ret_fingerprints: *mut RustSdpAttributeFingerprint) {
let attrs: Vec<_> = (*attributes).iter().filter_map(|x| if let SdpAttribute::Fingerprint(ref data) = *x {