From 651b949d85f847318ece301b64c099ad7959e9d0 Mon Sep 17 00:00:00 2001
From: Sverker Eriksson
Date: Wed, 12 Feb 2014 15:05:52 +0100
Subject: crypto: Fix bug when using old hmac context
Symptom: Using an old context (that had already been passed as argument to either
hmac_upgrade or hmac_final) could cause VM crash or worse.
Reason: Only a shallow copy (memcpy) of the context was made causing
a new context to partly mutate its parent context into an inconsistent
state.
Problem: Only OpenSSL v1.0 supports deep copy of hmac context. It is thus
not possible to implement a functional interface for older OpenSSL
versions (0.9.8) which still have wide spread use.
Solution: Change hmac contexts into a mutable state using NIF resources.
Document reuse of old contexts as undefined.
Future: If a need for reusing old context variables arise, a new hmac_copy
function could be introduced that is only supported for OpenSSL v1.0.
---
lib/crypto/c_src/crypto.c | 87 ++++++++++++++++++++++++++++---------------
lib/crypto/doc/src/crypto.xml | 8 +++-
2 files changed, 64 insertions(+), 31 deletions(-)
(limited to 'lib/crypto')
diff --git a/lib/crypto/c_src/crypto.c b/lib/crypto/c_src/crypto.c
index 925ad0c091..2e1a21178f 100644
--- a/lib/crypto/c_src/crypto.c
+++ b/lib/crypto/c_src/crypto.c
@@ -1,7 +1,7 @@
/*
* %CopyrightBegin%
*
- * Copyright Ericsson AB 2010-2013. All Rights Reserved.
+ * Copyright Ericsson AB 2010-2014. All Rights Reserved.
*
* The contents of this file are subject to the Erlang Public License,
* Version 1.1, (the "License"); you may not use this file except in
@@ -444,6 +444,15 @@ static ERL_NIF_TERM atom_ppbasis;
static ERL_NIF_TERM atom_onbasis;
#endif
+static ErlNifResourceType* hmac_context_rtype;
+struct hmac_context
+{
+ ErlNifMutex* mtx;
+ int alive;
+ HMAC_CTX ctx;
+};
+static void hmac_context_dtor(ErlNifEnv* env, struct hmac_context*);
+
/*
#define PRINTF_ERR0(FMT) enif_fprintf(stderr, FMT "\n")
#define PRINTF_ERR1(FMT, A1) enif_fprintf(stderr, FMT "\n", A1)
@@ -498,6 +507,15 @@ static int init(ErlNifEnv* env, ERL_NIF_TERM load_info)
return 0;
}
+ hmac_context_rtype = enif_open_resource_type(env, NULL, "hmac_context",
+ (ErlNifResourceDtor*) hmac_context_dtor,
+ ERL_NIF_RT_CREATE|ERL_NIF_RT_TAKEOVER,
+ NULL);
+ if (!hmac_context_rtype) {
+ PRINTF_ERR0("CRYPTO: Could not open resource type 'hmac_context'");
+ return 0;
+ }
+
if (library_refc > 0) {
/* Repeated loading of this library (module upgrade).
* Atoms and callbacks are already set, we are done.
@@ -1280,11 +1298,19 @@ static ERL_NIF_TERM sha512_mac_nif(ErlNifEnv* env, int argc, const ERL_NIF_TERM
#endif
}
+static void hmac_context_dtor(ErlNifEnv* env, struct hmac_context *obj)
+{
+ if (obj->alive) {
+ HMAC_CTX_cleanup(&obj->ctx);
+ obj->alive = 0;
+ }
+ enif_mutex_destroy(obj->mtx);
+}
+
static ERL_NIF_TERM hmac_init(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[])
{/* (Type, Key) */
ErlNifBinary key;
- ERL_NIF_TERM ret;
- unsigned char * ctx_buf;
+ struct hmac_context* obj;
const EVP_MD *md;
if (argv[0] == atom_sha) md = EVP_sha1();
@@ -1309,57 +1335,60 @@ static ERL_NIF_TERM hmac_init(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[
return enif_make_badarg(env);
}
- ctx_buf = enif_make_new_binary(env, sizeof(HMAC_CTX), &ret);
- HMAC_CTX_init((HMAC_CTX *) ctx_buf);
- HMAC_Init((HMAC_CTX *) ctx_buf, key.data, key.size, md);
+ obj = enif_alloc_resource(hmac_context_rtype, sizeof(struct hmac_context));
+ obj->mtx = enif_mutex_create("crypto.hmac");
+ obj->alive = 1;
+ HMAC_CTX_init(&obj->ctx);
+ HMAC_Init(&obj->ctx, key.data, key.size, md);
- return ret;
+ return enif_make_resource(env, obj);
}
static ERL_NIF_TERM hmac_update(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[])
{/* (Context, Data) */
- ErlNifBinary context, data;
- ERL_NIF_TERM ret;
- unsigned char * ctx_buf;
+ ErlNifBinary data;
+ struct hmac_context* obj;
- if (!enif_inspect_binary(env, argv[0], &context)
- || !enif_inspect_iolist_as_binary(env, argv[1], &data)
- || context.size != sizeof(HMAC_CTX)) {
+ if (!enif_get_resource(env, argv[0], hmac_context_rtype, (void**)&obj)
+ || !enif_inspect_iolist_as_binary(env, argv[1], &data)) {
+ return enif_make_badarg(env);
+ }
+ enif_mutex_lock(obj->mtx);
+ if (!obj->alive) {
+ enif_mutex_unlock(obj->mtx);
return enif_make_badarg(env);
}
+ HMAC_Update(&obj->ctx, data.data, data.size);
+ enif_mutex_unlock(obj->mtx);
- ctx_buf = enif_make_new_binary(env, sizeof(HMAC_CTX), &ret);
- memcpy(ctx_buf, context.data, context.size);
- HMAC_Update((HMAC_CTX *)ctx_buf, data.data, data.size);
CONSUME_REDS(env,data);
-
- return ret;
+ return argv[0];
}
static ERL_NIF_TERM hmac_final(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[])
{/* (Context) or (Context, HashLen) */
- ErlNifBinary context;
ERL_NIF_TERM ret;
- HMAC_CTX ctx;
+ struct hmac_context* obj;
unsigned char mac_buf[EVP_MAX_MD_SIZE];
unsigned char * mac_bin;
unsigned int req_len = 0;
unsigned int mac_len;
- if (!enif_inspect_binary(env, argv[0], &context)) {
- return enif_make_badarg(env);
- }
- if (argc == 2 && !enif_get_uint(env, argv[1], &req_len)) {
+ if (!enif_get_resource(env,argv[0],hmac_context_rtype, (void**)&obj)
+ || (argc == 2 && !enif_get_uint(env, argv[1], &req_len))) {
return enif_make_badarg(env);
}
- if (context.size != sizeof(ctx)) {
- return enif_make_badarg(env);
+ enif_mutex_lock(obj->mtx);
+ if (!obj->alive) {
+ enif_mutex_unlock(obj->mtx);
+ return enif_make_badarg(env);
}
- memcpy(&ctx, context.data, context.size);
- HMAC_Final(&ctx, mac_buf, &mac_len);
- HMAC_CTX_cleanup(&ctx);
+ HMAC_Final(&obj->ctx, mac_buf, &mac_len);
+ HMAC_CTX_cleanup(&obj->ctx);
+ obj->alive = 0;
+ enif_mutex_unlock(obj->mtx);
if (argc == 2 && req_len < mac_len) {
/* Only truncate to req_len bytes if asked. */
diff --git a/lib/crypto/doc/src/crypto.xml b/lib/crypto/doc/src/crypto.xml
index 40f829e704..c95827c371 100644
--- a/lib/crypto/doc/src/crypto.xml
+++ b/lib/crypto/doc/src/crypto.xml
@@ -4,7 +4,7 @@
- 19992013
+ 19992014
Ericsson AB. All Rights Reserved.
@@ -366,7 +366,11 @@
or to one of the functions hmac_final and
hmac_final_n
-
+ Do not use a Context as argument in more than one
+ call to hmac_update or hmac_final. The semantics of reusing old contexts
+ in any way is undefined and could even crash the VM in earlier releases.
+ The reason for this limitation is a lack of support in the underlying
+ OpenSSL API.
--
cgit v1.2.3