Properly support null crypto and null auth scenario

Currently, it is silently assumed that we always have master key provided,
even when the policy says we don't use encryption and authentication. This
results in streams with such policy to fail.

Fix that by properly calculating the expected master key length by taking
in account authentication type too. Correctly set expected key length for
null crypto policy to 0. Do not assume we have HMAC if the policy is set to
not have.

Add a test case for null/null policy while at it.

Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>

Author: freemangordon

srtp/srtp.c

@@ -1176,6 +1176,7 @@ static inline size_t full_key_length(const srtp_cipher_type_t *cipher)
 {
     switch (cipher->id) {
     case SRTP_NULL_CIPHER:
+        return 0;
     case SRTP_AES_ICM_128:
         return SRTP_AES_ICM_128_KEY_LEN_WSALT;
     case SRTP_AES_ICM_192:
@@ -1191,6 +1192,19 @@ static inline size_t full_key_length(const srtp_cipher_type_t *cipher)
     }
 }
 
+/* Get the key length that the application should supply for the given auth */
+static inline int full_auth_key_length(const srtp_auth_type_t *auth)
+{
+    switch (auth->id) {
+    case SRTP_NULL_AUTH:
+        return 0;
+    case SRTP_HMAC_SHA1:
+        return SRTP_AES_ICM_128_KEY_LEN_WSALT;
+    default:
+        return 0;
+    }
+}
+
 srtp_err_status_t srtp_get_session_keys(srtp_stream_ctx_t *stream,
                                         size_t mki_index,
                                         srtp_session_keys_t **session_keys)
@@ -1224,7 +1238,7 @@ srtp_err_status_t srtp_stream_init_keys(srtp_session_keys_t *session_keys,
     srtp_err_status_t stat;
     srtp_kdf_t kdf;
     uint8_t tmp_key[MAX_SRTP_KEY_LEN];
-    size_t input_keylen, input_keylen_rtcp;
+    size_t input_keylen, full_keylen;
     size_t kdf_keylen = 30, rtp_keylen, rtcp_keylen;
     size_t rtp_base_key_len, rtp_salt_len;
     size_t rtcp_base_key_len, rtcp_salt_len;
@@ -1252,9 +1266,17 @@ srtp_err_status_t srtp_stream_init_keys(srtp_session_keys_t *session_keys,
     }
 
     input_keylen = full_key_length(session_keys->rtp_cipher->type);
-    input_keylen_rtcp = full_key_length(session_keys->rtcp_cipher->type);
-    if (input_keylen_rtcp > input_keylen) {
-        input_keylen = input_keylen_rtcp;
+    full_keylen = full_auth_key_length(session_keys->rtp_auth->type);
+    if (full_keylen > input_keylen) {
+        input_keylen = full_keylen;
+    }
+    full_keylen = full_key_length(session_keys->rtcp_cipher->type);
+    if (full_keylen > input_keylen) {
+        input_keylen = full_keylen;
+    }
+    full_keylen = full_auth_key_length(session_keys->rtcp_auth->type);
+    if (full_keylen > input_keylen) {
+        input_keylen = full_keylen;
     }
 
     rtp_keylen = srtp_cipher_get_key_length(session_keys->rtp_cipher);
@@ -3713,8 +3735,7 @@ void srtp_crypto_policy_set_null_cipher_hmac_null(srtp_crypto_policy_t *p)
      */
 
     p->cipher_type = SRTP_NULL_CIPHER;
-    p->cipher_key_len =
-        SRTP_AES_ICM_128_KEY_LEN_WSALT; /* 128 bit key, 112 bit salt */
+    p->cipher_key_len = 0;
     p->auth_type = SRTP_NULL_AUTH;
     p->auth_key_len = 0;
     p->auth_tag_len = 0;

test/srtp_driver.c

@@ -64,7 +64,9 @@ srtp_err_status_t srtp_validate(void);
 
 srtp_err_status_t srtp_validate_mki(void);
 
-srtp_err_status_t srtp_validate_null(void);
+srtp_err_status_t srtp_validate_null_sha1_80(void);
+
+srtp_err_status_t srtp_validate_null_null(void);
 
 srtp_err_status_t srtp_validate_cryptex(void);
 
@@ -702,8 +704,17 @@ int main(int argc, char *argv[])
         }
 
         printf("testing srtp_protect and srtp_unprotect against "
-               "reference packet using null cipher and HMAC\n");
-        if (srtp_validate_null() == srtp_err_status_ok) {
+               "reference packet using null cipher and SHA1-80 HMAC\n");
+        if (srtp_validate_null_sha1_80() == srtp_err_status_ok) {
+            printf("passed\n\n");
+        } else {
+            printf("failed\n");
+            exit(1);
+        }
+
+        printf("testing srtp_protect and srtp_unprotect against "
+               "reference packet using null cipher and null HMAC\n");
+        if (srtp_validate_null_null() == srtp_err_status_ok) {
             printf("passed\n\n");
         } else {
             printf("failed\n");
@@ -2614,13 +2625,13 @@ srtp_err_status_t srtp_validate_mki(void)
 }
 
 /*
- * srtp_validate_null() verifies the correctness of libsrtp by comparing
+ * srtp_validate_null_sha1_80() verifies the correctness of libsrtp by comparing
  * some computed packets against some pre-computed reference values.
  * These packets were made with a policy that applies null encryption
  * and HMAC authentication.
  */
 
-srtp_err_status_t srtp_validate_null(void)
+srtp_err_status_t srtp_validate_null_sha1_80(void)
 {
     // clang-format off
     uint8_t srtp_plaintext_ref[28] = {
@@ -2772,6 +2783,176 @@ srtp_err_status_t srtp_validate_null(void)
     return srtp_err_status_ok;
 }
 
+/*
+ * srtp_validate_null_null() verifies the correctness of libsrtp by comparing
+ * some computed packets against some pre-computed reference values.
+ * These packets were made with a policy that applies null encryption
+ * and null authentication.
+ */
+
+srtp_err_status_t srtp_validate_null_null(void)
+{
+    // clang-format off
+    uint8_t srtp_plaintext_ref[28] = {
+        0x80, 0x0f, 0x12, 0x34, 0xde, 0xca, 0xfb, 0xad,
+        0xca, 0xfe, 0xba, 0xbe, 0xab, 0xab, 0xab, 0xab,
+        0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab,
+        0xab, 0xab, 0xab, 0xab
+    };
+    uint8_t srtp_plaintext[28] = {
+        0x80, 0x0f, 0x12, 0x34, 0xde, 0xca, 0xfb, 0xad,
+        0xca, 0xfe, 0xba, 0xbe, 0xab, 0xab, 0xab, 0xab,
+        0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab,
+        0xab, 0xab, 0xab, 0xab,
+    };
+    uint8_t srtp_ciphertext[28] = {
+        0x80, 0x0f, 0x12, 0x34, 0xde, 0xca, 0xfb, 0xad,
+        0xca, 0xfe, 0xba, 0xbe, 0xab, 0xab, 0xab, 0xab,
+        0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab,
+        0xab, 0xab, 0xab, 0xab,
+    };
+    uint8_t rtcp_plaintext_ref[24] = {
+        0x81, 0xc8, 0x00, 0x0b, 0xca, 0xfe, 0xba, 0xbe,
+        0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab,
+        0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab,
+    };
+    uint8_t rtcp_plaintext[28] = {
+        0x81, 0xc8, 0x00, 0x0b, 0xca, 0xfe, 0xba, 0xbe,
+        0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab,
+        0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab,
+        0x00, 0x00, 0x00, 0x00,
+    };
+    uint8_t srtcp_ciphertext[28] = {
+        0x81, 0xc8, 0x00, 0x0b, 0xca, 0xfe, 0xba, 0xbe,
+        0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab,
+        0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab,
+        0x00, 0x00, 0x00, 0x01,
+    };
+    // clang-format on
+
+    srtp_t srtp_snd, srtp_recv;
+    srtp_err_status_t status;
+    size_t len;
+    srtp_policy_t policy;
+
+    /*
+     * create a session with a single stream using the default srtp
+     * policy and with the SSRC value 0xcafebabe
+     */
+    memset(&policy, 0, sizeof(policy));
+    srtp_crypto_policy_set_null_cipher_hmac_null(&policy.rtp);
+    srtp_crypto_policy_set_null_cipher_hmac_null(&policy.rtcp);
+    policy.ssrc.type = ssrc_specific;
+    policy.ssrc.value = 0xcafebabe;
+    /*
+     * We need some non-zero value set here
+     */
+    policy.key = (void *)(uintptr_t)-1;
+    policy.window_size = 128;
+    policy.allow_repeat_tx = false;
+    policy.next = NULL;
+
+    status = srtp_create(&srtp_snd, &policy);
+    if (status) {
+        return status;
+    }
+
+    /*
+     * protect plaintext, then compare with ciphertext
+     */
+    len = 28;
+    status = call_srtp_protect(srtp_snd, srtp_plaintext, &len, 0);
+    if (!status && (len != 28)) {
+        status = srtp_err_status_fail;
+    }
+    if (status) {
+        return status;
+    }
+
+    debug_print(mod_driver, "ciphertext:\n  %s",
+                octet_string_hex_string(srtp_plaintext, len));
+    debug_print(mod_driver, "ciphertext reference:\n  %s",
+                octet_string_hex_string(srtp_ciphertext, len));
+
+    if (!srtp_octet_string_equal(srtp_plaintext, srtp_ciphertext, len)) {
+        return srtp_err_status_fail;
+    }
+
+    /*
+     * protect plaintext rtcp, then compare with srtcp ciphertext
+     */
+    len = 24;
+    status = call_srtp_protect_rtcp(srtp_snd, rtcp_plaintext, &len, 0);
+    if (!status && (len != 28)) {
+        status = srtp_err_status_fail;
+    }
+    if (status) {
+        return status;
+    }
+
+    debug_print(mod_driver, "srtcp ciphertext:\n  %s",
+                octet_string_hex_string(rtcp_plaintext, len));
+    debug_print(mod_driver, "srtcp ciphertext reference:\n  %s",
+                octet_string_hex_string(srtcp_ciphertext, len));
+
+    if (!srtp_octet_string_equal(rtcp_plaintext, srtcp_ciphertext, len)) {
+        return srtp_err_status_fail;
+    }
+
+    /*
+     * create a receiver session context comparable to the one created
+     * above - we need to do this so that the replay checking doesn't
+     * complain
+     */
+    status = srtp_create(&srtp_recv, &policy);
+    if (status) {
+        return status;
+    }
+
+    /*
+     * unprotect ciphertext, then compare with plaintext
+     */
+    status = call_srtp_unprotect(srtp_recv, srtp_ciphertext, &len);
+    if (!status && (len != 28)) {
+        status = srtp_err_status_fail;
+    }
+    if (status) {
+        return status;
+    }
+
+    if (!srtp_octet_string_equal(srtp_ciphertext, srtp_plaintext_ref, len)) {
+        return srtp_err_status_fail;
+    }
+
+    /*
+     * unprotect srtcp ciphertext, then compare with rtcp plaintext
+     */
+    len = 28;
+    status = call_srtp_unprotect_rtcp(srtp_recv, srtcp_ciphertext, &len);
+    if (!status && (len != 24)) {
+        status = srtp_err_status_fail;
+    }
+    if (status) {
+        return status;
+    }
+
+    if (!srtp_octet_string_equal(srtcp_ciphertext, rtcp_plaintext_ref, len)) {
+        return srtp_err_status_fail;
+    }
+
+    status = srtp_dealloc(srtp_snd);
+    if (status) {
+        return status;
+    }
+
+    status = srtp_dealloc(srtp_recv);
+    if (status) {
+        return status;
+    }
+
+    return srtp_err_status_ok;
+}
+
 /*
  * srtp_validate_cryptex() verifies the correctness of libsrtp by comparing
  * some computed packets against some pre-computed reference values.