Skip to content

SRTP/SRTCP KDF: add implementation #6888

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 7, 2023

Conversation

SparkiDev
Copy link
Contributor

Description

Add implementation of SRTP KDF and SRTCP KDF.
One shot APIs compatible with SP 800-135 and ACVP testing. Tests added to test.c.
Benchmarking added.
Doxygen added.

Testing

./configure --enable-srtp-kdf
Tests added to test.c

Checklist

  • added tests
  • [x updated/added doxygen
  • updated appropriate READMEs
  • Updated manual and documentation

@SparkiDev SparkiDev self-assigned this Oct 19, 2023
@SparkiDev
Copy link
Contributor Author

retest this please

@guidovranken
Copy link
Contributor

guidovranken commented Oct 19, 2023

Output keys remain (partially) uninitialized if too large:

#include <wolfssl/options.h>
#include <wolfssl/wolfcrypt/kdf.h>

#define CF_CHECK_EQ(expr, res) if ( (expr) != (res) ) { goto end; }

int main(void)
{
    uint8_t key[32] = {0};
    uint8_t salt[10] = {0};
    uint8_t index[6] = {0};
#define KEYSIZE 256
    uint8_t* key1 = malloc(KEYSIZE);
    uint8_t* key2 = malloc(KEYSIZE);
    uint8_t* key3 = malloc(KEYSIZE);

    CF_CHECK_EQ(
            wc_SRTP_KDF(
                key, sizeof(key),
                salt, sizeof(salt),
                -1,
                index,
                key1, KEYSIZE,
                key2, KEYSIZE,
                key3, KEYSIZE), 0);

    {
        FILE* fp = fopen("/dev/null", "wb");
        fwrite(key1, KEYSIZE, 1, fp);
        fwrite(key2, KEYSIZE, 1, fp);
        fwrite(key3, KEYSIZE, 1, fp);
        fclose(fp);
    }

    free(key1);
    free(key2);
    free(key3);
end:
    return 0;
}
$ valgrind ./a.out
==1899383== Syscall param write(buf) points to uninitialised byte(s)
==1899383==    at 0x4B28077: write (write.c:26)
==1899383==    by 0x4AA8E8C: _IO_file_write@@GLIBC_2.2.5 (fileops.c:1181)
==1899383==    by 0x4AAA950: new_do_write (fileops.c:449)
==1899383==    by 0x4AAA950: _IO_new_do_write (fileops.c:426)
==1899383==    by 0x4AAA950: _IO_do_write@@GLIBC_2.2.5 (fileops.c:423)
==1899383==    by 0x4AA9F37: _IO_file_close_it@@GLIBC_2.2.5 (fileops.c:136)
==1899383==    by 0x4A9BF35: fclose@@GLIBC_2.2.5 (iofclose.c:53)
==1899383==    by 0x155575: main (in /home/jhg/cf-wolf-srtp-kdf/cryptofuzz/a.out)
==1899383==  Address 0x4c0fb10 is 0 bytes inside a block of size 4,096 alloc'd
==1899383==    at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==1899383==    by 0x4A9BD03: _IO_file_doallocate (filedoalloc.c:101)
==1899383==    by 0x4AABECF: _IO_doallocbuf (genops.c:347)
==1899383==    by 0x4AAAF2F: _IO_file_overflow@@GLIBC_2.2.5 (fileops.c:745)
==1899383==    by 0x4AA96B4: _IO_new_file_xsputn (fileops.c:1244)
==1899383==    by 0x4AA96B4: _IO_file_xsputn@@GLIBC_2.2.5 (fileops.c:1197)
==1899383==    by 0x4A9D3C0: fwrite (iofwrite.c:39)
==1899383==    by 0x155543: main (in /home/jhg/cf-wolf-srtp-kdf/cryptofuzz/a.out)

Would it be possible to return error in this case to prevent accidental misuse?

@SparkiDev
Copy link
Contributor Author

Thanks Guido.

I used the wrong type for the keySz in the internal function.
This has been fixed now.

Let us know if there is anything else.

Sean :-)

@guidovranken
Copy link
Contributor

Confirmed fixed, and I cannot find any other bugs with this code at the moment.

Add implementation of SRTP KDF and SRTCP KDF.
One shot APIs compatible with SP 800-135 and ACVP testing.
Tests added to test.c.
Benchmarking added.
Doxygen added.
@JacobBarthelmeh JacobBarthelmeh merged commit 8921a72 into wolfSSL:master Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants