cleaning up a bit, rethinking things

pkg/urbit/jets/e/aes_siv.c

@@ -3,8 +3,6 @@
 */
 #include "all.h"
 
-#include <openssl/aes.h>
-
 #include "aes_siv.h"
 
 /* functions

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,

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 <stdint.h>
 #include <stdbool.h>
 #include <stdlib.h>
@@ -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,