Minor refactors suggested by @martinthomson
diff --git a/configure b/configure
index 6268ca6..e7484bf 100755
--- a/configure
+++ b/configure
@@ -5724,7 +5724,7 @@
# Check whether --with-nss-dir was given.
if test "${with_nss_dir+set}" = set; then :
withval=$with_nss_dir; if test -d $with_nss_dir/lib; then
- CFLAGS="$CFLAGS -I$with_nss_dir/include/nspr/"
+ CFLAGS="$CFLAGS -I$with_nss_dir/include/"
CFLAGS="$CFLAGS -I$with_nss_dir/../public/nss/"
if test "x$LDFLAGS" = "x"; then
LDFLAGS="-L$with_nss_dir/lib"
@@ -5739,7 +5739,7 @@
$as_echo "invalid" >&6; }
{ { $as_echo "$as_me:${as_lineno-$LINENO}: error: in \`$ac_pwd':" >&5
$as_echo "$as_me: error: in \`$ac_pwd':" >&2;}
-as_fn_error $? "Invalid OpenSSL location: $with_nss_dir
+as_fn_error $? "Invalid NSS location: $with_nss_dir
See \`config.log' for more details" "$LINENO" 5; }
fi
else
diff --git a/configure.ac b/configure.ac
index f2dd794..0913213 100644
--- a/configure.ac
+++ b/configure.ac
@@ -302,7 +302,7 @@
AC_ARG_WITH([nss-dir],
[AS_HELP_STRING([--with-nss-dir], [Location of NSS installation])],
[if test -d $with_nss_dir/lib; then
- CFLAGS="$CFLAGS -I$with_nss_dir/include/nspr/"
+ CFLAGS="$CFLAGS -I$with_nss_dir/include/"
CFLAGS="$CFLAGS -I$with_nss_dir/../public/nss/"
if test "x$LDFLAGS" = "x"; then
LDFLAGS="-L$with_nss_dir/lib"
@@ -313,7 +313,7 @@
AC_MSG_RESULT([$with_nss_dir])
else
AC_MSG_RESULT([invalid])
- AC_MSG_FAILURE([Invalid OpenSSL location: $with_nss_dir])
+ AC_MSG_FAILURE([Invalid NSS location: $with_nss_dir])
fi],
[AC_MSG_RESULT([no])])
diff --git a/crypto/cipher/aes_gcm_nss.c b/crypto/cipher/aes_gcm_nss.c
index 73ba785..6591186 100644
--- a/crypto/cipher/aes_gcm_nss.c
+++ b/crypto/cipher/aes_gcm_nss.c
@@ -136,13 +136,14 @@
gcm->tag_size = tlen;
gcm->params.ulTagBits = 8*tlen;
break;
+ default:
+ /* this should never hit, but to be sure... */
+ return (srtp_err_status_bad_param);
}
/* set key size and tag size*/
(*c)->key_len = key_len;
- // TODO(RLB): Make sure NSS is initialized
-
return (srtp_err_status_ok);
}
@@ -155,6 +156,11 @@
ctx = (srtp_aes_gcm_ctx_t *)c->state;
if (ctx) {
+ /* release NSS resources */
+ if (ctx->key) {
+ PK11_FreeSymKey(ctx->key);
+ }
+
/* zeroize the key material */
octet_string_set_to_zero(ctx, sizeof(srtp_aes_gcm_ctx_t));
srtp_crypto_free(ctx);
@@ -182,7 +188,20 @@
debug_print(srtp_mod_aes_gcm, "key: %s",
srtp_octet_string_hex_string(key, c->key_size));
- memcpy(c->key, key, c->key_size);
+ PK11SlotInfo *slot = PK11_GetBestSlot(CKM_AES_GCM, NULL);
+ if (!slot) {
+ return (srtp_err_status_cipher_fail);
+ }
+
+ SECItem key_item = { siBuffer, (unsigned char*) key, c->key_size };
+ c->key = PK11_ImportSymKey(slot, CKM_AES_GCM, PK11_OriginUnwrap,
+ CKA_ENCRYPT, &key_item, NULL);
+ PK11_FreeSlot(slot);
+
+ if (!c->key) {
+ return (srtp_err_status_cipher_fail);
+ }
+
return (srtp_err_status_ok);
}
@@ -252,27 +271,14 @@
// Reset AAD
c->aad_size = 0;
- PK11SlotInfo *slot = PK11_GetInternalSlot();
- if (!slot) {
- return (srtp_err_status_cipher_fail);
- }
-
- SECItem key_item = { siBuffer, c->key, c->key_size };
- PK11SymKey *key = PK11_ImportSymKey(slot, CKM_AES_GCM, PK11_OriginUnwrap,
- CKA_ENCRYPT, &key_item, NULL);
- if (!key) {
- PK11_FreeSlot(slot);
- return (srtp_err_status_cipher_fail);
- }
-
int rv;
SECItem param = { siBuffer, (unsigned char *) &c->params, sizeof(CK_GCM_PARAMS) };
if (encrypt) {
- rv = PK11_Encrypt(key, CKM_AES_GCM, ¶m,
+ rv = PK11_Encrypt(c->key, CKM_AES_GCM, ¶m,
buf, enc_len, *enc_len + 16,
buf, *enc_len);
} else {
- rv = PK11_Decrypt(key, CKM_AES_GCM, ¶m,
+ rv = PK11_Decrypt(c->key, CKM_AES_GCM, ¶m,
buf, enc_len, *enc_len,
buf, *enc_len);
}
@@ -282,8 +288,6 @@
status = (srtp_err_status_cipher_fail);
}
- PK11_FreeSymKey(key);
- PK11_FreeSlot(slot);
return status;
}
diff --git a/crypto/cipher/aes_icm_nss.c b/crypto/cipher/aes_icm_nss.c
index 5858b36..63bba7a 100644
--- a/crypto/cipher/aes_icm_nss.c
+++ b/crypto/cipher/aes_icm_nss.c
@@ -137,11 +137,6 @@
return srtp_err_status_alloc_fail;
}
- icm->slot = PK11_GetInternalSlot();
- if (!icm->slot) {
- return srtp_err_status_cipher_fail;
- }
-
icm->key = NULL;
icm->ctx = NULL;
@@ -183,11 +178,6 @@
ctx = (srtp_aes_icm_ctx_t *)c->state;
if (ctx) {
/* free any PK11 values that have been created */
- if (ctx->slot) {
- PK11_FreeSlot(ctx->slot);
- ctx->slot = NULL;
- }
-
if (ctx->key) {
PK11_FreeSymKey(ctx->key);
ctx->key = NULL;
@@ -240,13 +230,16 @@
srtp_octet_string_hex_string(key, c->key_size));
debug_print(srtp_mod_aes_icm, "offset: %s", v128_hex_string(&c->offset));
- if (!c->slot) {
+ PK11SlotInfo *slot = PK11_GetBestSlot(CKM_AES_CTR, NULL);
+ if (!slot) {
return srtp_err_status_bad_param;
}
- SECItem keyItem = { siBuffer, key, c->key_size };
- c->key = PK11_ImportSymKey(c->slot, CKM_AES_CTR, PK11_OriginUnwrap,
+ SECItem keyItem = { siBuffer, (unsigned char*)key, c->key_size };
+ c->key = PK11_ImportSymKey(slot, CKM_AES_CTR, PK11_OriginUnwrap,
CKA_ENCRYPT, &keyItem, NULL);
+ PK11_FreeSlot(slot);
+
if (!c->key) {
return srtp_err_status_cipher_fail;
}
diff --git a/crypto/include/aes_gcm.h b/crypto/include/aes_gcm.h
index c33f356..f59d04d 100644
--- a/crypto/include/aes_gcm.h
+++ b/crypto/include/aes_gcm.h
@@ -74,7 +74,7 @@
int key_size;
int tag_size;
srtp_cipher_direction_t dir;
- uint8_t key[32];
+ PK11SymKey *key;
uint8_t iv[12];
uint8_t aad[MAX_AD_SIZE];
int aad_size;
diff --git a/crypto/include/aes_icm_ext.h b/crypto/include/aes_icm_ext.h
index da00c9d..6518d40 100644
--- a/crypto/include/aes_icm_ext.h
+++ b/crypto/include/aes_icm_ext.h
@@ -72,7 +72,6 @@
v128_t offset;
int key_size;
uint8_t iv[16];
- PK11SlotInfo *slot;
PK11SymKey *key;
PK11Context *ctx;
} srtp_aes_icm_ctx_t;
diff --git a/srtp/srtp.c b/srtp/srtp.c
index e985dd0..5d9da7b 100644
--- a/srtp/srtp.c
+++ b/srtp/srtp.c
@@ -3544,7 +3544,6 @@
*/
/* Note: This would need to change for optional mikey data */
auth_tag = (uint8_t *)hdr + *pkt_octet_len;
- octet_string_set_to_zero(auth_tag, tag_len);
/*
* check sequence number for overruns, and copy it into the packet