Skip to content

Commit

Permalink
Revert "Move password buffer into the library"
Browse files Browse the repository at this point in the history
This reverts commit 3db6e798830ca51b179909e8c8cff7e77c2f494e.
  • Loading branch information
sjaeckel committed Feb 26, 2024
1 parent 452c7a7 commit d259135
Show file tree
Hide file tree
Showing 14 changed files with 69 additions and 76 deletions.
23 changes: 13 additions & 10 deletions demos/openssh-privkey.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,15 @@ static void die_(int err, int line)
#define die(i) do { die_(i, __LINE__); } while(0)
#define DIE(s, ...) do { print_err("%3d: " s "\n", __LINE__, ##__VA_ARGS__); exit(EXIT_FAILURE); } while(0)

static int getpassword(const char *prompt, char *pass, unsigned long *len)
static char* getpassword(const char *prompt, size_t maxlen)
{
char *wr, *end, *pass = XCALLOC(1, maxlen + 1);
struct termios tio;
tcflag_t c_lflag;
unsigned long maxlen = *len, wr = 0;
if (pass == NULL)
return NULL;
wr = pass;
end = pass + maxlen;

tcgetattr(0, &tio);
c_lflag = tio.c_lflag;
Expand All @@ -42,25 +46,24 @@ static int getpassword(const char *prompt, char *pass, unsigned long *len)

printf("%s", prompt);
fflush(stdout);
while (1) {
while (pass < end) {
int c = getchar();
if (c == '\r' || c == '\n' || c == -1)
break;
if (wr < maxlen)
pass[wr] = c;
wr++;
*wr++ = c;
}
*len = wr;
tio.c_lflag = c_lflag;
tcsetattr(0, TCSAFLUSH, &tio);
printf("\n");
return wr <= maxlen;
return pass;
}

static int password_get(void *p, unsigned long *l, void *u)
static int password_get(void **p, unsigned long *l, void *u)
{
(void)u;
return getpassword("Enter passphrase: ", p, l);
*p = getpassword("Enter passphrase: ", 256);
*l = strlen(*p);
return 0;
}

static void print(ltc_pka_key *k)
Expand Down
4 changes: 0 additions & 4 deletions src/headers/tomcrypt_custom.h
Original file line number Diff line number Diff line change
Expand Up @@ -619,10 +619,6 @@
#define LTC_PBES
#endif

#if defined(LTC_PEM) || defined(LTC_PKCS_8) && !defined(LTC_MAX_PASSWORD_LEN)
#define LTC_MAX_PASSWORD_LEN 256
#endif

#if defined(LTC_CLEAN_STACK)
/* if you're sure that you want to use it, remove the line below */
#error LTC_CLEAN_STACK is considered as broken
Expand Down
11 changes: 8 additions & 3 deletions src/headers/tomcrypt_pk.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,17 @@ typedef struct {
/**
Callback function that is called when a password is required.
@param str Pointer to where the password shall be stored.
@param len [in/out] The max length resp. resulting length of the password.
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.
@param str Pointer to pointer where the password will be stored.
@param len Pointer to the length of the password.
@param userdata `userdata` that was passed in the `password_ctx` struct.
@return CRYPT_OK on success
*/
int (*callback)(void *str, unsigned long *len, void *userdata);
int (*callback)(void **str, unsigned long *len, void *userdata);
/** Opaque `userdata` pointer passed when the callback is called */
void *userdata;
} password_ctx;
Expand Down
19 changes: 10 additions & 9 deletions src/headers/tomcrypt_private.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,18 +83,11 @@ typedef struct {
unsigned long blocklen;
} pbes_properties;

struct password {
/* usually a `char*` but could also contain binary data
* so use a `void*` + length to be on the safe side.
*/
unsigned char pw[LTC_MAX_PASSWORD_LEN];
unsigned long l;
};

typedef struct
{
pbes_properties type;
struct password pwd;
void *pwd;
unsigned long pwdlen;
ltc_asn1_list *enc_data;
ltc_asn1_list *salt;
ltc_asn1_list *iv;
Expand Down Expand Up @@ -266,6 +259,14 @@ 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;
Expand Down
3 changes: 0 additions & 3 deletions src/misc/crypt/crypt.c
Original file line number Diff line number Diff line change
Expand Up @@ -467,9 +467,6 @@ const char *crypt_build_settings =
" PBES1 "
" PBES2 "
#endif
#if defined(LTC_MAX_PASSWORD_LEN)
" " NAME_VALUE(LTC_MAX_PASSWORD_LEN) " "
#endif
#if defined(LTC_PEM)
" PEM "
" " NAME_VALUE(LTC_PEM_DECODE_BUFSZ) " "
Expand Down
2 changes: 1 addition & 1 deletion src/misc/pbes/pbes.c
Original file line number Diff line number Diff line change
Expand Up @@ -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.pw, arg->pwd.l, arg->salt->data, arg->salt->size, arg->iterations, hid, k, &klen)) != CRYPT_OK) goto LBL_ERROR;
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 = 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;
Expand Down
11 changes: 7 additions & 4 deletions src/misc/pem/pem_pkcs.c
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ static int s_decrypt_pem(unsigned char *pem, unsigned long *l, const struct pem_
if (hdr->info.keylen > sizeof(key)) {
return CRYPT_BUFFER_OVERFLOW;
}
if (!hdr->pw->pw) {
return CRYPT_INVALID_ARG;
}

ivlen = sizeof(iv);
if ((err = base16_decode(hdr->info.iv, XSTRLEN(hdr->info.iv), iv, &ivlen)) != CRYPT_OK) {
Expand Down Expand Up @@ -199,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 = { 0 };
struct password pw;
enum ltc_pka_id pka;
XMEMSET(k, 0, sizeof(*k));
w = LTC_PEM_READ_BUFSIZE * 2;
Expand Down Expand Up @@ -238,8 +241,7 @@ static int s_decode(struct get_char *g, ltc_pka_key *k, const password_ctx *pw_c
}

hdr.pw = &pw;
hdr.pw->l = LTC_MAX_PASSWORD_LEN;
if (pw_ctx->callback(hdr.pw->pw, &hdr.pw->l, pw_ctx->userdata)) {
if (pw_ctx->callback(&hdr.pw->pw, &hdr.pw->l, pw_ctx->userdata)) {
err = CRYPT_ERROR;
goto cleanup;
}
Expand All @@ -264,7 +266,8 @@ static int s_decode(struct get_char *g, ltc_pka_key *k, const password_ctx *pw_c

cleanup:
if (hdr.pw) {
zeromem(hdr.pw->pw, sizeof(hdr.pw->pw));
zeromem(hdr.pw->pw, hdr.pw->l);
XFREE(hdr.pw->pw);
}
XFREE(pem);
return err;
Expand Down
7 changes: 3 additions & 4 deletions src/misc/pem/pem_ssh.c
Original file line number Diff line number Diff line change
Expand Up @@ -570,8 +570,7 @@ static int s_decode_openssh(struct get_char *g, ltc_pka_key *k, const password_c
err = CRYPT_PW_CTX_MISSING;
goto cleanup;
}
opts.pw.l = LTC_MAX_PASSWORD_LEN;
if (pw_ctx->callback(opts.pw.pw, &opts.pw.l, pw_ctx->userdata)) {
if (pw_ctx->callback(&opts.pw.pw, &opts.pw.l, pw_ctx->userdata)) {
err = CRYPT_ERROR;
goto cleanup;
}
Expand All @@ -590,8 +589,8 @@ static int s_decode_openssh(struct get_char *g, ltc_pka_key *k, const password_c
}

cleanup:
if (opts.pw.l) {
zeromem(&opts.pw, sizeof(opts.pw));
if (opts.pw.pw) {
XFREE(opts.pw.pw);
}
if (privkey) {
zeromem(privkey, privkey_len);
Expand Down
12 changes: 7 additions & 5 deletions src/pk/asn1/pkcs8/pkcs8_decode_flexi.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,13 @@ int pkcs8_decode_flexi(const unsigned char *in, unsigned long inlen,
unsigned char *dec_data = NULL;
ltc_asn1_list *l = NULL;
int err;
pbes_arg pbes = { 0 };
pbes_arg pbes;

LTC_ARGCHK(in != NULL);
LTC_ARGCHK(decoded_list != NULL);

XMEMSET(&pbes, 0, sizeof(pbes));

*decoded_list = NULL;
if ((err = der_decode_sequence_flexi(in, &len, &l)) == CRYPT_OK) {
/* the following "if" detects whether it is encrypted or not */
Expand Down Expand Up @@ -61,8 +63,7 @@ int pkcs8_decode_flexi(const unsigned char *in, unsigned long inlen,
goto LBL_DONE;
}

pbes.pwd.l = LTC_MAX_PASSWORD_LEN;
if (pw_ctx->callback(pbes.pwd.pw, &pbes.pwd.l, pw_ctx->userdata)) {
if (pw_ctx->callback(&pbes.pwd, &pbes.pwdlen, pw_ctx->userdata)) {
err = CRYPT_ERROR;
goto LBL_DONE;
}
Expand Down Expand Up @@ -94,8 +95,9 @@ int pkcs8_decode_flexi(const unsigned char *in, unsigned long inlen,

LBL_DONE:
if (l) der_free_sequence_flexi(l);
if (pbes.pwd.l) {
zeromem(&pbes.pwd, sizeof(pbes.pwd));
if (pbes.pwd) {
zeromem(pbes.pwd, pbes.pwdlen);
XFREE(pbes.pwd);
}
if (dec_data) {
zeromem(dec_data, dec_size);
Expand Down
8 changes: 3 additions & 5 deletions tests/ecc_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -660,14 +660,12 @@ static int s_ecc_new_api(void)
}


static int password_get(void *p, unsigned long *l, void *u)
static int password_get(void **p, unsigned long *l, void *u)
{
int ret = *l < 6;
LTC_UNUSED_PARAM(u);
if (!ret)
XMEMCPY(p, "secret", 6);
*p = strdup("secret");
*l = 6;
return ret;
return 0;
}

static int s_ecc_import_export(void) {
Expand Down
8 changes: 3 additions & 5 deletions tests/ed25519_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,12 @@ static void xor_shuffle(unsigned char *buf, unsigned long size, unsigned char ch
buf[i] ^= change;
}

static int password_get(void *p, unsigned long *l, void *u)
static int password_get(void **p, unsigned long *l, void *u)
{
int ret = *l < 6;
LTC_UNUSED_PARAM(u);
if (!ret)
XMEMCPY(p, "123456", 6);
*p = strdup("123456");
*l = 6;
return ret;
return 0;
}

static int s_rfc_8410_10_test(void)
Expand Down
16 changes: 6 additions & 10 deletions tests/pem_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,12 @@

#ifdef LTC_SSH

static int password_get_ssh(void *p, unsigned long *l, void *u)
static int password_get_ssh(void **p, unsigned long *l, void *u)
{
int ret = *l < 6;
LTC_UNUSED_PARAM(u);
if (!ret)
XMEMCPY(p, "abc123", 6);
*p = strdup("abc123");
*l = 6;
return ret;
return 0;
}
static int s_pem_decode_ssh(const void *in, unsigned long inlen, void *key)
{
Expand All @@ -30,14 +28,12 @@ static int s_pem_decode_ssh_f(FILE *f, void *key)

#endif

static int password_get(void *p, unsigned long *l, void *u)
static int password_get(void **p, unsigned long *l, void *u)
{
int ret = *l < 6;
LTC_UNUSED_PARAM(u);
if (!ret)
XMEMCPY(p, "secret", 6);
*p = strdup("secret");
*l = 6;
return ret;
return 0;
}

#if defined(LTC_MDSA)
Expand Down
8 changes: 3 additions & 5 deletions tests/rsa_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -432,14 +432,12 @@ static int s_rsa_import_x509(const void *in, unsigned long inlen, void *key)
}

#if defined(LTC_MD2) && defined(LTC_MD5) && defined(LTC_RC2)
static int password_get(void *p, unsigned long *l, void *u)
static int password_get(void **p, unsigned long *l, void *u)
{
int ret = *l < 6;
LTC_UNUSED_PARAM(u);
if (!ret)
XMEMCPY(p, "secret", 6);
*p = strdup("secret");
*l = 6;
return ret;
return 0;
}

static int s_rsa_import_pkcs8(const void *in, unsigned long inlen, void *key)
Expand Down
13 changes: 5 additions & 8 deletions tests/x25519_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -139,14 +139,11 @@ static int s_rfc_8410_10_test(void)
return CRYPT_OK;
}

static int password_get(void *p, unsigned long *l, void *u)
static int password_get(void **p, unsigned long *l, void *u)
{
unsigned long sl = strlen(u);
int ret = *l < sl;
if (!ret)
XMEMCPY(p, u, sl);
*l = sl;
return ret;
*p = strdup(u);
*l = strlen(*p);
return 0;
}

static int s_x25519_pkcs8_test(void)
Expand All @@ -165,7 +162,7 @@ static int s_x25519_pkcs8_test(void)
/* `openssl genpkey -algorithm x25519 -pass stdin` */
{
"MC4CAQAwBQYDK2VuBCIEIEAInaUdx+fQFfghpCzw/WdItRT3+FnPSkrU9TcIZTZW",
""
NULL
},
};
unsigned n;
Expand Down

0 comments on commit d259135

Please sign in to comment.