From ca14af11a7334dc40d25804b78832b55d8285524 Mon Sep 17 00:00:00 2001 From: Paul Driver Date: Tue, 18 Aug 2020 10:31:50 -0700 Subject: [PATCH] cleaning up a bit, rethinking things --- pkg/urbit/jets/e/aes_siv.c | 2 -- pkg/urcrypt/urcrypt.c | 47 ++++++++++++++++++-------------------- pkg/urcrypt/urcrypt.h | 31 +++++++++++++++++++++++-- 3 files changed, 51 insertions(+), 29 deletions(-) diff --git a/pkg/urbit/jets/e/aes_siv.c b/pkg/urbit/jets/e/aes_siv.c index b31ab84cf3..80ad62ab6b 100644 --- a/pkg/urbit/jets/e/aes_siv.c +++ b/pkg/urbit/jets/e/aes_siv.c @@ -3,8 +3,6 @@ */ #include "all.h" -#include - #include "aes_siv.h" /* functions diff --git a/pkg/urcrypt/urcrypt.c b/pkg/urcrypt/urcrypt.c index f878f6a9ce..d15ad2ae22 100644 --- a/pkg/urcrypt/urcrypt.c +++ b/pkg/urcrypt/urcrypt.c @@ -5,12 +5,12 @@ static urcrypt_malloc_t urcrypt_malloc_ptr; static urcrypt_realloc_t urcrypt_realloc_ptr; static urcrypt_free_t urcrypt_free_ptr; -void* urcrypt_malloc(size_t len) +static void* _urcrypt_malloc(size_t len) { return (*urcrypt_malloc_ptr)(len); } -void* urcrypt_realloc(void *ptr, size_t len) +static void* _urcrypt_realloc(void *ptr, size_t len) { return (*urcrypt_realloc_ptr)(ptr, len); } @@ -21,48 +21,47 @@ void urcrypt_free(void *ptr) } static void* -urcrypt_malloc_ssl(size_t len +_urcrypt_malloc_ssl(size_t len #if OPENSSL_VERSION_NUMBER >= 0x10100000L , const char* file, int line #endif -) { return urcrypt_malloc(len); } +) { return _urcrypt_malloc(len); } static void* -urcrypt_realloc_ssl(void* ptr, size_t len +_urcrypt_realloc_ssl(void* ptr, size_t len #if OPENSSL_VERSION_NUMBER >= 0x10100000L , const char* file, int line #endif -) { return urcrypt_realloc(ptr, len); } +) { return _urcrypt_realloc(ptr, len); } static void -urcrypt_free_ssl(void* ptr +_urcrypt_free_ssl(void* ptr #if OPENSSL_VERSION_NUMBER >= 0x10100000L , const char* file, int line #endif ) { urcrypt_free(ptr); } -/* IMPORTANT: it is an error (undefined behavior) to call functions in - * this library without first calling urcrypt_init() exactly once. - */ int -urcrypt_init(urcrypt_malloc_t m, urcrypt_realloc_t r, urcrypt_free_t f) +urcrypt_init(_urcrypt_malloc_t m, _urcrypt_realloc_t r, urcrypt_free_t f) { if ( initialized ) { return -1; } + else if ( (NULL == m) || (NULL == r) || (NULL == f) ) { + return -2; + } else { initialized = true; - urcrypt_malloc_ptr = ( NULL == m ) ? &malloc : m; - urcrypt_realloc_ptr = ( NULL == r ) ? &realloc : r; - urcrypt_free_ptr = ( NULL == f ) ? &free : f; - - if ( CRYPTO_set_mem_functions(&urcrypt_malloc_ssl, - &urcrypt_realloc_ssl, - &urcrypt_free_ssl) ) { + urcrypt_malloc_ptr = m; + urcrypt_realloc_ptr = r; + urcrypt_free_ptr = f; + if ( CRYPTO_set_mem_functions(&_urcrypt_malloc_ssl, + &_urcrypt_realloc_ssl, + &_urcrypt_free_ssl) ) { return 0; } else { - return -2; + return -3; } } } @@ -252,7 +251,7 @@ _urcrypt_reverse_copy(size_t size, const uint8_t *in, uint8_t *out) { static uint8_t* _urcrypt_reverse_alloc(size_t size, const uint8_t *in) { - uint8_t *out = urcrypt_malloc(size); + uint8_t *out = _urcrypt_malloc(size); _urcrypt_reverse_copy(size, in, out); return out; } @@ -401,7 +400,7 @@ _urcrypt_cbc_pad(size_t *length_ptr, const uint8_t *message) rem = length % 16, padding = rem ? 16 - rem : 0, padded = length + padding; - uint8_t *buf = urcrypt_malloc(padded); + uint8_t *buf = _urcrypt_malloc(padded); memset(buf, 0, padding); _urcrypt_reverse_copy(length, message, buf + padding); @@ -422,7 +421,7 @@ _urcrypt_cbc_help(const uint8_t *message, _urcrypt_reverse_copy(16, ivec, riv); in = _urcrypt_cbc_pad(&length, message); - out = urcrypt_malloc(length); + out = _urcrypt_malloc(length); AES_cbc_encrypt(in, out, length, key, riv, enc); urcrypt_free(in); @@ -584,7 +583,7 @@ urcrypt_aes_cbcc_de(const uint8_t *message, static int _urcrypt_argon2_alloc(uint8_t** output, size_t bytes) { - *output = urcrypt_malloc(bytes); + *output = _urcrypt_malloc(bytes); return (NULL != output); } @@ -598,8 +597,6 @@ _urcrypt_argon2_free(uint8_t* memory, size_t bytes) // in uint32_t, so here's a helper macro for ensuring equivalence. #define SZ_32(s) ( sizeof(size_t) <= sizeof(uint32_t) || s <= 0xFFFFFFFF ) -// returns a constant error message string or NULL for success -// (so user doesn't have to call argon2_error_message) const char* urcrypt_argon2(urcrypt_argon2_type type, uint32_t version, diff --git a/pkg/urcrypt/urcrypt.h b/pkg/urcrypt/urcrypt.h index faa474cbf4..a57278f79a 100644 --- a/pkg/urcrypt/urcrypt.h +++ b/pkg/urcrypt/urcrypt.h @@ -1,6 +1,8 @@ #ifndef URCRYPT_H #define URCRYPT_H +// XX most of these should be moved to urcrypt.c + #include #include #include @@ -19,10 +21,26 @@ typedef void *(*urcrypt_malloc_t)(size_t); typedef void *(*urcrypt_realloc_t)(void*, size_t); typedef void (*urcrypt_free_t)(void*); +/* We depend on OpenSSL for various reasons, which doesn't promise not to + * allocate memory and has the annoying CRYPTO_set_mem_functions api. We + * are therefore forced to use it, so we adopt a similar approach with + * urcrypt_init(). + * + * This creates an issue for certain link configurations when the client of + * urcrypt also uses the CRYPTO_set_mem_functions() api. The simplest thing + * that is guaranteed to work is to call first CRYPTO_set_mem_functions() and + * then urcrypt_init() with the same arguments during process initialization. + * + * You should call this function once. It will return 0 on success. Calling + * any other library functions without exactly one successful call to + * urcrypt_init() will result in undefined behavior. + */ int urcrypt_init(urcrypt_malloc_t, urcrypt_realloc_t, urcrypt_free_t); -void *urcrypt_malloc(size_t); -void *urcrypt_realloc(void*, size_t); +/* We can transparently deal with padding since we already use memory + * allocation; in cases where we return allocated memory, it should + * be freed with urcrypt_free() by the caller. + */ void urcrypt_free(void*); int urcrypt_ed_point_add(const uint8_t a[32], @@ -57,6 +75,10 @@ bool urcrypt_ed_veri(const uint8_t *message, const uint8_t signature[64], const uint8_t public[32]); +// XX let's de-const the reversed arrays and promise to trash them instead, +// it will save us a malloc in some cases and it's a better API since +// it puts the decision to copy in the caller's hands. + int urcrypt_aes_ecba_en(const uint8_t key[16], const uint8_t block[16], uint8_t out[16]); @@ -76,6 +98,10 @@ int urcrypt_aes_ecbc_de(const uint8_t key[32], const uint8_t block[16], uint8_t out[16]); +/* return an alloc'd output pointer and write its length to out_length + * caller should urcrypt_free() the returned pointer. The return value + * is NULL on an error. */ + uint8_t* urcrypt_aes_cbca_en(const uint8_t *message, size_t length, const uint8_t key[16], @@ -114,6 +140,7 @@ typedef enum urcrypt_argon2_type { urcrypt_argon2_u = 10, } urcrypt_argon2_type; +/* returns a constant error message string or NULL for success */ const char* urcrypt_argon2(urcrypt_argon2_type type, uint32_t version, uint32_t threads,