cisco / libsrtp

Library for SRTP (Secure Realtime Transport Protocol)
Other
1.21k stars 474 forks source link

Feature/additional error checks #498

Closed mfroeschl closed 4 years ago

mfroeschl commented 4 years ago

Add additional error checks to srtp.c and aes_gcm_ossl.c as outlined in #497

mfroeschl commented 4 years ago

Update: It seems adding the checks for EVP_CIPHER_CTX_ctrl() return values causes the cipher_driver.c test to fail:

https://travis-ci.org/github/cisco/libsrtp/jobs/705650070

Build done. Please run 'make runtest' to run self tests.
The command "make" exited with 0.
0.01s$ make runtest
Build done. Please run 'make runtest' to run self tests.
running libsrtp2 test applications...
crypto/test/cipher_driver -v >/dev/null
Makefile:46: recipe for target 'runtest' failed
make: *** [runtest] Error 11

11 is srtp_err_status_algo_fail

pabuhler commented 4 years ago

Hi, looks good so far. I think setting the dummy_tag with EVP_CTRL_GCM_SET_TAG was needed in older versions of openssl, testing with new version it does not seam to be needed. Regardless EVP_CTRL_GCM_SET_TAG can only be used when decrypting so it should only be used when c->dir == srtp_direction_decrypt, so we can protect the setting of the dummy_tag in an if check. I did a test and it seams to work. Will you make that change or if you prefer I can push that change to your branch.

mfroeschl commented 4 years ago

@pabuhler Thank you for the quick response! I could confirm that after implementing your suggestion (only set the dummy tag when decrypting) the tests pass again. I will update my PR accordingly.

mfroeschl commented 4 years ago

@pabuhler FYI, I have updated the PR with your suggestions (but forgot to send you a notification).