Correct null cipher key sizes and be more defensive #589
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There are two major changes here:
Fix a bug introduced in Use a full-length key even with null ciphers #559: While
full_key_lengthcorrectly returnedAES_ICM_128_KEYLEN_W_SALTfor the key size of a null cipher, the policy setting methods set the key size to16. As a result, a caller using a null cipher and providing a key of the length recommended by the crypto policy would experience a 14-byte over-read of the buffer.In general, be more cautious about key lengths to avoid over-reads. Unfortunately, the existing API doesn't provide an explicit length from the key, so we are left to presume that this length is reflected in the RTP / RTCP crypto policies. However, these are settable by the caller, so we need to check that the provided values are correct for the cipher types in use.
The PR also includes fixes to the
srtp_drivertest, which provided incorrect key lengths for its policies that use null ciphers. The null cipher validation test did not need updating because the key buffer provided actually had enough data, so there was no over-read.