From f580f6525b6d40c879f1594d187be5d5238f4da9 Mon Sep 17 00:00:00 2001 From: Jesper Louis Andersen Date: Thu, 6 Feb 2020 11:48:57 +0100 Subject: [PATCH] Streamline _open style calls Those now return {ok, Msg} or {error, term()} so you are kind of forced to match on them. This is likely to help with correctnes. --- CHANGELOG.md | 5 ++- c_src/enacl.c | 4 ++ c_src/enacl.h | 1 + c_src/secret.c | 4 +- c_src/sign.c | 92 +++++++++++++----------------------------- eqc_test/enacl_eqc.erl | 18 --------- src/enacl.erl | 24 +++-------- 7 files changed, 45 insertions(+), 103 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 576310d..1b6adab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,9 +9,12 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Go through all calls and make them return streamlined exceptions if applicable. Pretty large change, but OTOH, this ought to happen before a 1.0 release as well. - - sign + - Generichashes must support the finalized state - Implement missing EQC tests + - stream_chacha20... + - stream_xor... + - generichash... ## [Unreleased] diff --git a/c_src/enacl.c b/c_src/enacl.c index 5497180..a27dd35 100644 --- a/c_src/enacl.c +++ b/c_src/enacl.c @@ -9,4 +9,8 @@ ERL_NIF_TERM enacl_error_tuple(ErlNifEnv *env, char *error_atom) { ERL_NIF_TERM enacl_internal_error(ErlNifEnv *env) { return enif_raise_exception(env, enif_make_atom(env, "enacl_internal_error")); +} + +ERL_NIF_TERM enacl_error_finalized(ErlNifEnv *env) { + return enif_raise_exception(env, enif_make_atom(env, "enacl_finalized")); } \ No newline at end of file diff --git a/c_src/enacl.h b/c_src/enacl.h index 3dc3a59..ef84b9e 100644 --- a/c_src/enacl.h +++ b/c_src/enacl.h @@ -9,6 +9,7 @@ #define ATOM_FALSE "false" ERL_NIF_TERM enacl_error_tuple(ErlNifEnv *, char *); +ERL_NIF_TERM enacl_error_finalized(ErlNifEnv *); ERL_NIF_TERM enacl_internal_error(ErlNifEnv *); #endif diff --git a/c_src/secret.c b/c_src/secret.c index fb7b11a..627d444 100644 --- a/c_src/secret.c +++ b/c_src/secret.c @@ -130,9 +130,11 @@ ERL_NIF_TERM enacl_crypto_secretbox_open(ErlNifEnv *env, int argc, return enacl_error_tuple(env, "failed_verification"); } - return enif_make_sub_binary( + ERL_NIF_TERM ret_ok = enif_make_atom(env, ATOM_OK); + ERL_NIF_TERM ret_bin = enif_make_sub_binary( env, enif_make_binary(env, &padded_msg), crypto_secretbox_ZEROBYTES, padded_ciphertext.size - crypto_secretbox_ZEROBYTES); + return enif_make_tuple2(env, ret_ok, ret_bin); } ERL_NIF_TERM enacl_crypto_stream_chacha20(ErlNifEnv *env, int argc, diff --git a/c_src/sign.c b/c_src/sign.c index c37cf63..6a4e00a 100644 --- a/c_src/sign.c +++ b/c_src/sign.c @@ -56,24 +56,20 @@ ERL_NIF_TERM enacl_crypto_sign_init(ErlNifEnv *env, int argc, if ((obj = enif_alloc_resource(enacl_sign_ctx_rtype, sizeof(enacl_sign_ctx))) == NULL) { - ret = enacl_error_tuple(env, "alloc_failed"); - goto done; + goto err; } obj->alive = 0; obj->state = enif_alloc(crypto_sign_statebytes()); if (obj->state == NULL) { - ret = enacl_error_tuple(env, "state_malloc"); goto release; } obj->alive = 1; if ((obj->mtx = enif_mutex_create("enacl.sign")) == NULL) { - ret = enacl_error_tuple(env, "mutex_create"); goto free; } if (0 != crypto_sign_init(obj->state)) { - ret = enacl_error_tuple(env, "sign_init_error"); goto free; } @@ -94,6 +90,8 @@ free: release: // This also frees the mutex via the destructor enif_release_resource(obj); +err: + ret = enacl_internal_error(env); done: return ret; } @@ -120,12 +118,12 @@ ERL_NIF_TERM enacl_crypto_sign_update(ErlNifEnv *env, int argc, enif_mutex_lock(obj->mtx); if (!obj->alive) { - ret = enacl_error_tuple(env, "finalized"); + ret = enacl_error_finalized(env); goto done; } if (0 != crypto_sign_update(obj->state, data.data, data.size)) { - ret = enacl_error_tuple(env, "sign_update_error"); + ret = enacl_internal_error(env); // This should never be hit goto done; } @@ -157,19 +155,16 @@ ERL_NIF_TERM enacl_crypto_sign_final_create(ErlNifEnv *env, int argc, enif_mutex_lock(obj->mtx); if (!obj->alive) { - ret = enacl_error_tuple(env, "finalized"); + ret = enacl_error_finalized(env); goto done; } if (!enif_alloc_binary(crypto_sign_BYTES, &sig)) { - ret = enacl_error_tuple(env, "alloc_failed"); + ret = enacl_internal_error(env); goto done; } - if (0 != crypto_sign_final_create(obj->state, sig.data, &siglen, sk.data)) { - ret = enacl_error_tuple(env, "sign_error"); - goto release; - } + crypto_sign_final_create(obj->state, sig.data, &siglen, sk.data); ERL_NIF_TERM ok = enif_make_atom(env, ATOM_OK); ERL_NIF_TERM signature = enif_make_binary(env, &sig); @@ -179,8 +174,6 @@ ERL_NIF_TERM enacl_crypto_sign_final_create(ErlNifEnv *env, int argc, bad_arg: return enif_make_badarg(env); -release: - enif_release_binary(&sig); cleanup: obj->alive = 0; sodium_memzero(obj->state, crypto_sign_statebytes()); @@ -210,7 +203,7 @@ ERL_NIF_TERM enacl_crypto_sign_final_verify(ErlNifEnv *env, int argc, enif_mutex_lock(obj->mtx); if (!obj->alive) { - ret = enacl_error_tuple(env, "finalized"); + ret = enacl_error_finalized(env); goto done; } @@ -246,12 +239,12 @@ enacl_crypto_sign_ed25519_keypair(ErlNifEnv *env, int argc, } if (!enif_alloc_binary(crypto_sign_ed25519_PUBLICKEYBYTES, &pk)) { - return enacl_error_tuple(env, "alloc_failed"); + return enacl_internal_error(env); } if (!enif_alloc_binary(crypto_sign_ed25519_SECRETKEYBYTES, &sk)) { enif_release_binary(&pk); - return enacl_error_tuple(env, "alloc_failed"); + return enacl_internal_error(env); } crypto_sign_ed25519_keypair(pk.data, sk.data); @@ -271,13 +264,10 @@ enacl_crypto_sign_ed25519_sk_to_pk(ErlNifEnv *env, int argc, } if (!enif_alloc_binary(crypto_sign_ed25519_PUBLICKEYBYTES, &pk)) { - return enacl_error_tuple(env, "alloc_failed"); + return enacl_internal_error(env); } - if (crypto_sign_ed25519_sk_to_pk(pk.data, sk.data) != 0) { - enif_release_binary(&pk); - return enacl_error_tuple(env, "crypto_sign_ed25519_sk_to_pk_failed"); - } + crypto_sign_ed25519_sk_to_pk(pk.data, sk.data); return enif_make_binary(env, &pk); } @@ -293,14 +283,10 @@ enacl_crypto_sign_ed25519_public_to_curve25519(ErlNifEnv *env, int argc, } if (!enif_alloc_binary(crypto_scalarmult_curve25519_BYTES, &curve25519_pk)) { - return enacl_error_tuple(env, "alloc_failed"); + return enacl_internal_error(env); } - if (crypto_sign_ed25519_pk_to_curve25519(curve25519_pk.data, - ed25519_pk.data) != 0) { - enif_release_binary(&curve25519_pk); - return enacl_error_tuple(env, "ed25519_public_to_curve25519_failed"); - } + crypto_sign_ed25519_pk_to_curve25519(curve25519_pk.data, ed25519_pk.data); return enif_make_binary(env, &curve25519_pk); } @@ -316,14 +302,10 @@ enacl_crypto_sign_ed25519_secret_to_curve25519(ErlNifEnv *env, int argc, } if (!enif_alloc_binary(crypto_scalarmult_curve25519_BYTES, &curve25519_sk)) { - return enacl_error_tuple(env, "alloc_failed"); + return enacl_internal_error(env); } - if (crypto_sign_ed25519_sk_to_curve25519(curve25519_sk.data, - ed25519_sk.data) != 0) { - enif_release_binary(&curve25519_sk); - return enacl_error_tuple(env, "ed25519_secret_to_curve25519_failed"); - } + crypto_sign_ed25519_sk_to_curve25519(curve25519_sk.data, ed25519_sk.data); return enif_make_binary(env, &curve25519_sk); } @@ -364,12 +346,12 @@ ERL_NIF_TERM enacl_crypto_sign_keypair(ErlNifEnv *env, int argc, } if (!enif_alloc_binary(crypto_sign_PUBLICKEYBYTES, &pk)) { - return enacl_error_tuple(env, "alloc_failed"); + return enacl_internal_error(env); } if (!enif_alloc_binary(crypto_sign_SECRETKEYBYTES, &sk)) { enif_release_binary(&pk); - return enacl_error_tuple(env, "alloc_failed"); + return enacl_internal_error(env); } crypto_sign_keypair(pk.data, sk.data); @@ -387,12 +369,12 @@ ERL_NIF_TERM enacl_crypto_sign_seed_keypair(ErlNifEnv *env, int argc, } if (!enif_alloc_binary(crypto_sign_PUBLICKEYBYTES, &pk)) { - return enacl_error_tuple(env, "alloc_failed"); + return enacl_internal_error(env); } if (!enif_alloc_binary(crypto_sign_SECRETKEYBYTES, &sk)) { enif_release_binary(&pk); - return enacl_error_tuple(env, "alloc_failed"); + return enacl_internal_error(env); } crypto_sign_seed_keypair(pk.data, sk.data, seed.data); @@ -401,11 +383,6 @@ ERL_NIF_TERM enacl_crypto_sign_seed_keypair(ErlNifEnv *env, int argc, enif_make_binary(env, &sk)); } -/* -int crypto_sign(unsigned char *sm, unsigned long long *smlen, - const unsigned char *m, unsigned long long mlen, - const unsigned char *sk); - */ ERL_NIF_TERM enacl_crypto_sign(ErlNifEnv *env, int argc, ERL_NIF_TERM const argv[]) { ErlNifBinary m, sk, sm; @@ -421,7 +398,7 @@ ERL_NIF_TERM enacl_crypto_sign(ErlNifEnv *env, int argc, } if (!enif_alloc_binary(m.size + crypto_sign_BYTES, &sm)) { - return enacl_error_tuple(env, "alloc_failed"); + return enacl_internal_error(env); } crypto_sign(sm.data, &smlen, m.data, m.size, sk.data); @@ -429,11 +406,6 @@ ERL_NIF_TERM enacl_crypto_sign(ErlNifEnv *env, int argc, return enif_make_sub_binary(env, enif_make_binary(env, &sm), 0, smlen); } -/* -int crypto_sign_open(unsigned char *m, unsigned long long *mlen, - const unsigned char *sm, unsigned long long smlen, - const unsigned char *pk); - */ ERL_NIF_TERM enacl_crypto_sign_open(ErlNifEnv *env, int argc, ERL_NIF_TERM const argv[]) { ErlNifBinary m, sm, pk; @@ -449,22 +421,20 @@ ERL_NIF_TERM enacl_crypto_sign_open(ErlNifEnv *env, int argc, } if (!enif_alloc_binary(sm.size, &m)) { - return enacl_error_tuple(env, "alloc_failed"); + return enacl_internal_error(env); } if (0 == crypto_sign_open(m.data, &mlen, sm.data, sm.size, pk.data)) { - return enif_make_sub_binary(env, enif_make_binary(env, &m), 0, mlen); + ERL_NIF_TERM ret_ok = enif_make_atom(env, ATOM_OK); + ERL_NIF_TERM ret_bin = + enif_make_sub_binary(env, enif_make_binary(env, &m), 0, mlen); + return enif_make_tuple2(env, ret_ok, ret_bin); } else { enif_release_binary(&m); return enacl_error_tuple(env, "failed_verification"); } } -/* -int crypto_sign_detached(unsigned char *sig, unsigned long long *siglen, - const unsigned char *m, unsigned long long mlen, - const unsigned char *sk); - */ ERL_NIF_TERM enacl_crypto_sign_detached(ErlNifEnv *env, int argc, ERL_NIF_TERM const argv[]) { ErlNifBinary m, sk, sig; @@ -480,7 +450,7 @@ ERL_NIF_TERM enacl_crypto_sign_detached(ErlNifEnv *env, int argc, } if (!enif_alloc_binary(crypto_sign_BYTES, &sig)) { - return enacl_error_tuple(env, "alloc_failed"); + return enacl_internal_error(env); } crypto_sign_detached(sig.data, &siglen, m.data, m.size, sk.data); @@ -488,12 +458,6 @@ ERL_NIF_TERM enacl_crypto_sign_detached(ErlNifEnv *env, int argc, return enif_make_binary(env, &sig); } -/* -int crypto_sign_verify_detached(const unsigned char *sig, - const unsigned char *m, - unsigned long long mlen, - const unsigned char *pk); - */ ERL_NIF_TERM enacl_crypto_sign_verify_detached(ErlNifEnv *env, int argc, ERL_NIF_TERM const argv[]) { diff --git a/eqc_test/enacl_eqc.erl b/eqc_test/enacl_eqc.erl index 210e4d0..4371ff8 100644 --- a/eqc_test/enacl_eqc.erl +++ b/eqc_test/enacl_eqc.erl @@ -607,24 +607,6 @@ xor_bytes(<>, <>) -> [A bxor B | xor_bytes(As, Bs)]; xor_bytes(<<>>, <<>>) -> []. -%% prop_stream_xor_correct() -> -%% ?FORALL({Msg, Nonce, Key}, -%% {?FAULT_RATE(1, 40, g_iodata()), -%% ?FAULT_RATE(1, 40, nonce()), -%% ?FAULT_RATE(1, 40, secret_key())}, -%% case v_iodata(Msg) andalso nonce_valid(Nonce) andalso secret_key_valid(Key) of -%% true -> -%% Stream = enacl:stream(iolist_size(Msg), Nonce, Key), -%% CipherText = enacl:stream_xor(Msg, Nonce, Key), -%% StreamXor = enacl:stream_xor(CipherText, Nonce, Key), -%% conjunction([ -%% {'xor', equals(iolist_to_binary(Msg), StreamXor)}, -%% {stream, equals(iolist_to_binary(xor_bytes(Stream, iolist_to_binary(Msg))), CipherText)} -%% ]); -%% false -> -%% badargs(fun() -> enacl:stream_xor(Msg, Nonce, Key) end) -%% end). - %% CRYPTO AUTH %% ------------------------------------------------------------ %% * auth/2 diff --git a/src/enacl.erl b/src/enacl.erl index d94013f..7b712ad 100644 --- a/src/enacl.erl +++ b/src/enacl.erl @@ -30,7 +30,6 @@ box_secret_key_bytes/0, box_beforenm_bytes/0, - %% EQC sign_keypair_public_size/0, sign_keypair_secret_size/0, sign_keypair_seed_size/0, @@ -46,7 +45,6 @@ sign_final_create/2, sign_final_verify/3, - %% EQC box_seal/2, box_seal_open/3 ]). @@ -73,7 +71,6 @@ aead_chacha20poly1305_ietf_ABYTES/0, aead_chacha20poly1305_ietf_MESSAGEBYTES_MAX/0, - %% No Tests! aead_xchacha20poly1305_ietf_encrypt/4, aead_xchacha20poly1305_ietf_decrypt/4, aead_xchacha20poly1305_ietf_KEYBYTES/0, @@ -111,16 +108,14 @@ generichash_update/2, generichash_final/1, - %% No Tests! + %% EQC! shorthash_key_size/0, shorthash_size/0, shorthash/2, - %% No Tests! pwhash/4, pwhash_str/3, - %% EQC pwhash/2, pwhash_str/1, pwhash_str_verify/2 @@ -602,10 +597,7 @@ sign(M, SK) -> PK :: binary(), M :: binary(). sign_open(SM, PK) -> - case enacl_nif:crypto_sign_open(SM, PK) of - M when is_binary(M) -> {ok, M}; - {error, Err} -> {error, Err} - end. + enacl_nif:crypto_sign_open(SM, PK). %% @doc sign_detached/2 computes a digital signature given a message and a secret key. %% @@ -742,17 +734,11 @@ secretbox(Msg, Nonce, Key) -> secretbox_open(CipherText, Nonce, Key) -> case iolist_size(CipherText) of K when K =< ?SECRETBOX_SIZE -> - R = case enacl_nif:crypto_secretbox_open_b([?S_BOXZEROBYTES, CipherText], - Nonce, Key) of - {error, Err} -> {error, Err}; - Bin when is_binary(Bin) -> {ok, Bin} - end, + R = enacl_nif:crypto_secretbox_open_b([?S_BOXZEROBYTES, CipherText], + Nonce, Key), bump(R, ?SECRETBOX_OPEN_REDUCTIONS, ?SECRETBOX_SIZE, K); _ -> - case enacl_nif:crypto_secretbox_open([?S_BOXZEROBYTES, CipherText], Nonce, Key) of - {error, Err} -> {error, Err}; - Bin when is_binary(Bin) -> {ok, Bin} - end + enacl_nif:crypto_secretbox_open([?S_BOXZEROBYTES, CipherText], Nonce, Key) end. %% @doc secretbox_nonce_size/0 returns the size of the secretbox nonce