Skip to content
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

support running against nss >= 3.52 #524

Merged
merged 1 commit into from
Jan 28, 2021

Conversation

pabuhler
Copy link
Member

Fix for #523

This is described in more detail at:
https://bugzilla.mozilla.org/show_bug.cgi?id=1637488#c3

Have decided to go with using NSS_PKCS11_2_0_COMPAT for now
as it provides the most fexibility.
Consider moving to the new PK11_AEADOp api in the future but
that will put a requirment on nss version at both compile and
run time.

Fix for cisco#523

This is described in more detail at:
https://bugzilla.mozilla.org/show_bug.cgi?id=1637488#c3

Have decided to go with using NSS_PKCS11_2_0_COMPAT for now
as it provides the most fexibility.
Consider moving to the new PK11_AEADOp api in the future but
that will put a requirment on nss version at both compile and
run time.
Copy link

@martinthomson martinthomson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The alternative change, which would be to deal with the PKCS#11 change, might be better. I would recommend opening an issue to track that.

Copy link
Contributor

@bifurcation bifurcation left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer an approach here that's more future-oriented. For example, perhaps we could offer consumers a switch to enable old NSS, and map things over internally to libsrtp. (So that we could delete the old path easily when compat is no longer needed.)

If that's too hard, this is a fine hack. But as @martinthomson suggests, we should file an issue to move to the new API.

@@ -42,6 +42,9 @@
#include "alloc.h"
#include "err.h" /* for srtp_debug */
#include "auth_test_cases.h"

#define NSS_PKCS11_2_0_COMPAT 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this flag outside of the GCM case?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

likely not i was just being consistent with setting it before all includes

@pabuhler
Copy link
Member Author

#525 is a follow on issues. Until there is value in placing the burden of a version check (build/runtime) on uses of libsrtp I think this is the best solution.

@pabuhler pabuhler merged commit 7073623 into cisco:master Jan 28, 2021
@pabuhler pabuhler deleted the nss-3.51-runtime-fix branch January 28, 2021 17:07
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.

3 participants