Skip to content

Commit

Permalink
Remove length limit on PKINIT PKCS#12 prompt
Browse files Browse the repository at this point in the history
Long pathnames can trigger the 128-byte prompt length limit in
pkinit_get_certs_pkcs12.  Use asprintf instead of snprintf.  Also
check the result of the prompter invocation.

(cherry picked from commit 3c330ea)

ticket: 8011
version_fixed: 1.13.1
status: resolved
  • Loading branch information
greghudson authored and tlyu committed Dec 16, 2014
1 parent db81478 commit bbff4de
Showing 1 changed file with 10 additions and 7 deletions.
17 changes: 10 additions & 7 deletions src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
Expand Up @@ -4107,6 +4107,7 @@ pkinit_get_certs_pkcs12(krb5_context context,
krb5_principal princ)
{
krb5_error_code retval = KRB5KDC_ERR_PREAUTH_FAILED;
char *prompt_string = NULL;
X509 *x = NULL;
PKCS12 *p12 = NULL;
int ret;
Expand Down Expand Up @@ -4147,8 +4148,7 @@ pkinit_get_certs_pkcs12(krb5_context context,
krb5_data rdat;
krb5_prompt kprompt;
krb5_prompt_type prompt_type;
int r = 0;
char prompt_string[128];
krb5_error_code r;
char prompt_reply[128];
char *prompt_prefix = _("Pass phrase for");
char *p12name = reassemble_pkcs12_name(idopts->cert_filename);
Expand Down Expand Up @@ -4180,11 +4180,9 @@ pkinit_get_certs_pkcs12(krb5_context context,
rdat.data = prompt_reply;
rdat.length = sizeof(prompt_reply);

r = snprintf(prompt_string, sizeof(prompt_string), "%s %s",
prompt_prefix, idopts->cert_filename);
if (r >= (int)sizeof(prompt_string)) {
pkiDebug("Prompt string, '%s %s', is too long!\n",
prompt_prefix, idopts->cert_filename);
if (asprintf(&prompt_string, "%s %s", prompt_prefix,
idopts->cert_filename) < 0) {
prompt_string = NULL;

This comment has been minimized.

Copy link
@wfiveash

wfiveash Jan 15, 2015

Why set prompt_string NULL here? The Solaris man page for asprintf() states:
If sufficient space cannot be allocated, the asprintf() function returns -1 and sets ret to be a NULL pointer.

This comment has been minimized.

Copy link
@greghudson

greghudson Jan 15, 2015

Author Member

Other asprintf documentation (e.g. the man page on most Linux systems) indicates that the value of the string is undefined on error. It's unfortunate, but we need to code to that to be safe.

goto cleanup;
}
kprompt.prompt = prompt_string;
Expand All @@ -4196,6 +4194,10 @@ pkinit_get_certs_pkcs12(krb5_context context,
r = (*id_cryptoctx->prompter)(context, id_cryptoctx->prompter_data,
NULL, NULL, 1, &kprompt);
k5int_set_prompt_types(context, 0);
if (r) {
pkiDebug("Failed to prompt for PKCS12 password");
goto cleanup;
}
}

ret = PKCS12_parse(p12, rdat.data, &y, &x, NULL);
Expand All @@ -4220,6 +4222,7 @@ pkinit_get_certs_pkcs12(krb5_context context,
retval = 0;

cleanup:
free(prompt_string);
if (p12)
PKCS12_free(p12);
if (retval) {
Expand Down

0 comments on commit bbff4de

Please sign in to comment.