Add `free()` function to `password_ctx`

The user can now pass a `free()` function pointer that will be used to
free the memory that has been allocated by the `callback()`.
If `free()` is NULL, the library will still call `XFREE()`.

Signed-off-by: Steffen Jaeckel <[email protected]>

helper.pl

@@ -47,7 +47,7 @@ sub check_source {
       push @{$troubles->{unwanted_malloc}},  $lineno if $file =~ /^src\/.*\.c$/ && $l =~ /\bmalloc\s*\(/;
       push @{$troubles->{unwanted_realloc}}, $lineno if $file =~ /^src\/.*\.c$/ && $l =~ /\brealloc\s*\(/;
       push @{$troubles->{unwanted_calloc}},  $lineno if $file =~ /^src\/.*\.c$/ && $l =~ /\bcalloc\s*\(/;
-      push @{$troubles->{unwanted_free}},    $lineno if $file =~ /^src\/.*\.c$/ && $l =~ /\bfree\s*\(/;
+      push @{$troubles->{unwanted_free}},    $lineno if $file =~ /^src\/.*\.c$/ && $l =~ /[^>.]\bfree\s*\(/;
       push @{$troubles->{unwanted_memset}},  $lineno if $file =~ /^src\/.*\.c$/ && $l =~ /\bmemset\s*\(/;
       push @{$troubles->{unwanted_memcpy}},  $lineno if $file =~ /^src\/.*\.c$/ && $l =~ /\bmemcpy\s*\(/;
       push @{$troubles->{unwanted_memmove}}, $lineno if $file =~ /^src\/.*\.c$/ && $l =~ /\bmemmove\s*\(/;

src/headers/tomcrypt_pk.h

@@ -1,14 +1,14 @@
 /* LibTomCrypt, modular cryptographic library -- Tom St Denis */
 /* SPDX-License-Identifier: Unlicense */
 
-typedef struct {
+typedef struct password_ctx {
    /**
       Callback function that is called when a password is required.
 
       Please be aware that the library takes ownership of the pointer that is
       returned to the library via `str`.
-      `str` shall be allocated via the same function as `XMALLOC` points to.
-      The data will be zeroed and `XFREE`'d as soon as it isn't required anymore.
+      The data will be zeroed and `free()`'d as soon as it isn't required anymore.
+      c.f. the documentation of the `free()` function pointer for details.
 
       @param str        Pointer to pointer where the password will be stored.
       @param len        Pointer to the length of the password.
@@ -16,6 +16,16 @@ typedef struct {
       @return CRYPT_OK on success
    */
    int (*callback)(void **str, unsigned long *len, void *userdata);
+   /**
+      Optional free function to free the allocated buffer.
+
+      At the point where the value returned by `callback()` is not required
+      anymore the library will free it by either calling this `free()` function
+      or `XFREE()` in case this `free()` function is set to `NULL`.
+
+      @param str        Pointer to the buffer to be free'd.
+   */
+   void (*free)(void *str);
    /** Opaque `userdata` pointer passed when the callback is called */
    void *userdata;
 } password_ctx;

src/headers/tomcrypt_private.h

@@ -66,8 +66,16 @@ typedef struct {
 } ltc_dh_set_type;
 
 
-typedef int (*fn_kdf_t)(const unsigned char *password, unsigned long password_len,
-                              const unsigned char *salt,     unsigned long salt_len,
+struct password {
+   /* usually a `char*` but could also contain binary data
+    * so use a `void*` + length to be on the safe side.
+    */
+   void *pw;
+   unsigned long l;
+};
+
+typedef int (*fn_kdf_t)(const struct password *pwd,
+                        const unsigned char *salt,  unsigned long salt_len,
                               int iteration_count,  int hash_idx,
                               unsigned char *out,   unsigned long *outlen);
 
@@ -86,8 +94,7 @@ typedef struct {
 typedef struct
 {
    pbes_properties type;
-   void *pwd;
-   unsigned long pwdlen;
+   struct password pw;
    ltc_asn1_list *enc_data;
    ltc_asn1_list *salt;
    ltc_asn1_list *iv;
@@ -259,14 +266,6 @@ enum cipher_mode {
    cm_none, cm_cbc, cm_cfb, cm_ctr, cm_ofb, cm_stream, cm_gcm
 };
 
-struct password {
-   /* usually a `char*` but could also contain binary data
-    * so use a `void*` + length to be on the safe side.
-    */
-   void *pw;
-   unsigned long l;
-};
-
 struct blockcipher_info {
    const char *name;
    const char *algo;
@@ -341,6 +340,7 @@ struct get_char {
 /* others */
 
 void copy_or_zeromem(const unsigned char* src, unsigned char* dest, unsigned long len, int coz);
+void password_free(struct password *pw, const struct password_ctx *ctx);
 
 int pbes_decrypt(const pbes_arg  *arg, unsigned char *dec_data, unsigned long *dec_size);
 

src/misc/password_free.c

@@ -0,0 +1,28 @@
+/* LibTomCrypt, modular cryptographic library -- Tom St Denis */
+/* SPDX-License-Identifier: Unlicense */
+#include "tomcrypt_private.h"
+
+/**
+   @file password_free.c
+   Free the password inside a `struct password`, Steffen Jaeckel
+*/
+
+/**
+   Free a password
+   @param pw   The password to be free'd
+   @param ctx  The password context
+*/
+void password_free(struct password *pw, const struct password_ctx *ctx)
+{
+   if (!ctx || !pw || !pw->pw)
+      return;
+
+   zeromem(pw->pw, pw->l);
+   if (ctx->free) {
+      ctx->free(pw->pw);
+   } else {
+      XFREE(pw->pw);
+   }
+   pw->pw = NULL;
+   pw->l = 0;
+}

src/misc/pbes/pbes.c

@@ -50,7 +50,7 @@ int pbes_decrypt(const pbes_arg  *arg, unsigned char *dec_data, unsigned long *d
 
    if (klen > sizeof(k)) return CRYPT_INVALID_ARG;
 
-   if ((err = arg->type.kdf(arg->pwd, arg->pwdlen, arg->salt->data, arg->salt->size, arg->iterations, hid, k, &klen)) != CRYPT_OK) goto LBL_ERROR;
+   if ((err = arg->type.kdf(&arg->pw, arg->salt->data, arg->salt->size, arg->iterations, hid, k, &klen)) != CRYPT_OK) goto LBL_ERROR;
    if ((err = cbc_start(cid, iv, k, keylen, 0, &cbc)) != CRYPT_OK) goto LBL_ERROR;
    if ((err = cbc_decrypt(arg->enc_data->data, dec_data, arg->enc_data->size, &cbc)) != CRYPT_OK) goto LBL_ERROR;
    if ((err = cbc_done(&cbc)) != CRYPT_OK) goto LBL_ERROR;

src/misc/pbes/pbes1.c

@@ -4,28 +4,28 @@
 
 #ifdef LTC_PBES
 
-static int s_pkcs_5_alg1_wrap(const unsigned char *password, unsigned long password_len,
-                              const unsigned char *salt,     unsigned long salt_len,
-                              int iteration_count,  int hash_idx,
-                              unsigned char *out,   unsigned long *outlen)
+static int s_pkcs_5_alg1_wrap(const struct password *pwd,
+                              const unsigned char *salt,  unsigned long salt_len,
+                                    int iteration_count,  int hash_idx,
+                                    unsigned char *out,   unsigned long *outlen)
 {
    LTC_UNUSED_PARAM(salt_len);
-   return pkcs_5_alg1(password, password_len, salt, iteration_count, hash_idx, out, outlen);
+   return pkcs_5_alg1(pwd->pw, pwd->l, salt, iteration_count, hash_idx, out, outlen);
 }
 
-static int s_pkcs_12_wrap(const unsigned char *password, unsigned long password_len,
-                              const unsigned char *salt,     unsigned long salt_len,
-                              int iteration_count,  int hash_idx,
-                              unsigned char *out,   unsigned long *outlen)
+static int s_pkcs_12_wrap(const struct password *pwd,
+                          const unsigned char *salt,  unsigned long salt_len,
+                                int iteration_count,  int hash_idx,
+                                unsigned char *out,   unsigned long *outlen)
 {
    int err;
    /* convert password to unicode/utf16-be */
-   unsigned long pwlen = password_len * 2;
+   unsigned long pwlen = pwd->l * 2;
    unsigned char* pw;
    if (*outlen < 32) return CRYPT_INVALID_ARG;
    pw = XMALLOC(pwlen + 2);
    if (pw == NULL) return CRYPT_MEM;
-   if ((err = pkcs12_utf8_to_utf16(password, password_len, pw, &pwlen)) != CRYPT_OK) goto LBL_ERROR;
+   if ((err = pkcs12_utf8_to_utf16(pwd->pw, pwd->l, pw, &pwlen)) != CRYPT_OK) goto LBL_ERROR;
    pw[pwlen++] = 0;
    pw[pwlen++] = 0;
    /* derive KEY */

src/misc/pbes/pbes2.c

@@ -22,13 +22,21 @@ static const oid_id_st s_hmac_oid_names[] = {
    { "1.2.840.113549.2.13", "sha512-256" },
 };
 
+static int s_pkcs_5_alg2_wrap(const struct password *pwd,
+                              const unsigned char *salt,  unsigned long salt_len,
+                                    int iteration_count,  int hash_idx,
+                                    unsigned char *out,   unsigned long *outlen)
+{
+   return pkcs_5_alg2(pwd->pw, pwd->l, salt, salt_len, iteration_count, hash_idx, out, outlen);
+}
+
 static const pbes_properties s_pbes2_default_types[] = {
-   { pkcs_5_alg2, "sha1",   "des",   8, 0 },
-   { pkcs_5_alg2, "sha1",   "rc2",   4, 0 },
-   { pkcs_5_alg2, "sha1",   "3des", 24, 0 },
-   { pkcs_5_alg2, "sha1",   "aes",  16, 0 },
-   { pkcs_5_alg2, "sha1",   "aes",  24, 0 },
-   { pkcs_5_alg2, "sha1",   "aes",  32, 0 },
+   { s_pkcs_5_alg2_wrap, "sha1",   "des",   8, 0 },
+   { s_pkcs_5_alg2_wrap, "sha1",   "rc2",   4, 0 },
+   { s_pkcs_5_alg2_wrap, "sha1",   "3des", 24, 0 },
+   { s_pkcs_5_alg2_wrap, "sha1",   "aes",  16, 0 },
+   { s_pkcs_5_alg2_wrap, "sha1",   "aes",  24, 0 },
+   { s_pkcs_5_alg2_wrap, "sha1",   "aes",  32, 0 },
 };
 
 typedef struct {

src/misc/pem/pem_pkcs.c

@@ -202,7 +202,7 @@ static int s_decode(struct get_char *g, ltc_pka_key *k, const password_ctx *pw_c
    unsigned long w, l, n;
    int err = CRYPT_ERROR;
    struct pem_headers hdr = { 0 };
-   struct password pw;
+   struct password pw = { 0 };
    enum ltc_pka_id pka;
    XMEMSET(k, 0, sizeof(*k));
    w = LTC_PEM_READ_BUFSIZE * 2;
@@ -265,10 +265,7 @@ retry:
    }
 
 cleanup:
-   if (hdr.pw) {
-      zeromem(hdr.pw->pw, hdr.pw->l);
-      XFREE(hdr.pw->pw);
-   }
+   password_free(hdr.pw, pw_ctx);
    XFREE(pem);
    return err;
 }

src/misc/pem/pem_ssh.c

@@ -589,9 +589,7 @@ retry:
    }
 
 cleanup:
-   if (opts.pw.pw) {
-      XFREE(opts.pw.pw);
-   }
+   password_free(&opts.pw, pw_ctx);
    if (privkey) {
       zeromem(privkey, privkey_len);
       XFREE(privkey);

src/pk/asn1/pkcs8/pkcs8_decode_flexi.c

@@ -63,7 +63,7 @@ int pkcs8_decode_flexi(const unsigned char  *in,  unsigned long inlen,
             goto LBL_DONE;
          }
 
-         if (pw_ctx->callback(&pbes.pwd, &pbes.pwdlen, pw_ctx->userdata)) {
+         if (pw_ctx->callback(&pbes.pw.pw, &pbes.pw.l, pw_ctx->userdata)) {
             err = CRYPT_ERROR;
             goto LBL_DONE;
          }
@@ -94,15 +94,12 @@ int pkcs8_decode_flexi(const unsigned char  *in,  unsigned long inlen,
    }
 
 LBL_DONE:
-   if (l) der_free_sequence_flexi(l);
-   if (pbes.pwd) {
-      zeromem(pbes.pwd, pbes.pwdlen);
-      XFREE(pbes.pwd);
-   }
    if (dec_data) {
       zeromem(dec_data, dec_size);
       XFREE(dec_data);
    }
+   password_free(&pbes.pw, pw_ctx);
+   if (l) der_free_sequence_flexi(l);
    return err;
 }
 

tests/ecc_test.c

@@ -1329,8 +1329,7 @@ static int s_ecc_import_export(void) {
       0x9d, 0x7b, 0x70, 0x3e, 0xf5, 0x7d, 0xa4, 0xfd, 0x3c, 0xc6, 0x49, 0x93, 0xd3, 0x5b, 0xef, 0xc9,
       0xae, 0x97, 0xaf, 0x64, 0x64, 0xf9, 0x69, 0xd8
    };
-   password_ctx pw_ctx;
-   pw_ctx.callback = password_get;
+   password_ctx pw_ctx = { .callback = password_get };
 
    if (ltc_mp.sqrtmod_prime == NULL) return CRYPT_NOP; /* we need compressed points which requires sqrtmod_prime */
 

tests/ed25519_test.c

@@ -65,8 +65,7 @@ static int s_rfc_8410_10_test(void)
    unsigned char buf[1024];
    char tmp[512];
    unsigned long buflen, tmplen;
-   password_ctx pw_ctx;
-   pw_ctx.callback = password_get;
+   password_ctx pw_ctx = { .callback = password_get };
    for (n = 0; n < sizeof(rfc_8410_10)/sizeof(rfc_8410_10[0]); ++n) {
       buflen = sizeof(buf);
       DO(base64_decode(rfc_8410_10[n].b64, XSTRLEN(rfc_8410_10[n].b64), buf, &buflen));

tests/pem_test.c

@@ -15,14 +15,12 @@ static int password_get_ssh(void **p, unsigned long *l, void *u)
 }
 static int s_pem_decode_ssh(const void *in, unsigned long inlen, void *key)
 {
-   password_ctx pw_ctx;
-   pw_ctx.callback = password_get_ssh;
+   password_ctx pw_ctx = { .callback = password_get_ssh };
    return pem_decode_openssh(in, inlen, key, &pw_ctx);
 }
 static int s_pem_decode_ssh_f(FILE *f, void *key)
 {
-   password_ctx pw_ctx;
-   pw_ctx.callback = password_get_ssh;
+   password_ctx pw_ctx = { .callback = password_get_ssh };
    return pem_decode_openssh_filehandle(f, key, &pw_ctx);
 }
 
@@ -76,8 +74,7 @@ static int s_key_cmp(ltc_pka_key *key)
 
 static int s_pem_only_decode_pkcs(const void *in, unsigned long inlen, void *key)
 {
-   password_ctx pw_ctx;
-   pw_ctx.callback = password_get;
+   password_ctx pw_ctx = { .callback = password_get };
    return pem_decode_pkcs(in, inlen, key, &pw_ctx);
 }
 
@@ -92,9 +89,8 @@ static int s_pem_decode_pkcs(const void *in, unsigned long inlen, void *key)
 
 static int s_pem_decode_pkcs_f(FILE *f, void *key)
 {
-   password_ctx pw_ctx;
    int err;
-   pw_ctx.callback = password_get;
+   password_ctx pw_ctx = { .callback = password_get };
    if ((err = pem_decode_pkcs_filehandle(f, key, &pw_ctx)) != CRYPT_OK) {
       return err;
    }
@@ -103,8 +99,7 @@ static int s_pem_decode_pkcs_f(FILE *f, void *key)
 
 static int s_pem_only_decode(const void *in, unsigned long inlen, void *key)
 {
-   password_ctx pw_ctx;
-   pw_ctx.callback = password_get;
+   password_ctx pw_ctx = { .callback = password_get };
    if ((strcmp(ltc_mp.name, "TomsFastMath") == 0) && (inlen > 2048)) {
 #if defined(LTC_TEST_DBG) && LTC_TEST_DBG > 1
       fprintf(stderr, "Skipping testcase because of TomsFastMath\n");
@@ -116,8 +111,7 @@ static int s_pem_only_decode(const void *in, unsigned long inlen, void *key)
 
 static int s_pem_only_decode_f(FILE *f, void *key)
 {
-   password_ctx pw_ctx;
-   pw_ctx.callback = password_get;
+   password_ctx pw_ctx = { .callback = password_get };
    return pem_decode_filehandle(f, key, &pw_ctx);
 }
 

tests/rsa_test.c

@@ -442,8 +442,7 @@ static int password_get(void **p, unsigned long *l, void *u)
 
 static int s_rsa_import_pkcs8(const void *in, unsigned long inlen, void *key)
 {
-   password_ctx pw_ctx;
-   pw_ctx.callback = password_get;
+   password_ctx pw_ctx = { .callback = password_get };
    return rsa_import_pkcs8(in, inlen, &pw_ctx, key);
 }
 #endif

tests/x25519_test.c

@@ -169,8 +169,7 @@ static int s_x25519_pkcs8_test(void)
    curve25519_key key;
    unsigned char buf[1024];
    unsigned long buflen;
-   password_ctx pw_ctx, *p_pw_ctx;
-   pw_ctx.callback = password_get;
+   password_ctx *p_pw_ctx, pw_ctx = { .callback = password_get };
    for (n = 0; n < sizeof(s_x25519_pkcs8)/sizeof(s_x25519_pkcs8[0]); ++n) {
       buflen = sizeof(buf);
       DO(base64_decode(s_x25519_pkcs8[n].b64, XSTRLEN(s_x25519_pkcs8[n].b64), buf, &buflen));