[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Use of PKINIT from PAM
So your patch seems mostly ok, I've applied it with modification to the
prompter function. I have issue with this:
> + char buffer[BUFSIZ];
[...]
> + password_data.data = buffer;
> + password_data.length = UI_get_result_maxsize(uis);
I would think it should be sizeof(buffer) there, I rewrote it to use malloc
and the length UI_get_result_maxsize() instead.
Also, I wonder if the code should resolve private key when calling
krb5_get_init_creds_opt_set_pkinit. instead maybe it should wait until
krb5_get_init_creds is called.
Love
"Douglas E. Engert" <deengert@anl.gov> writes:
> Attached are changes against heimdal-20050426.
>
> I have run across a number of minor problem while using
> pkinit from PAM. The main set of problems requires the use
> of a krb5_prompter_fct callback routine. The prompter needs
> to be passed its data in a void *data pointer.
>
> The krb5_get_init_creds_opt_set_pkinit was defined with
> a krb5_prompter_fct, but not void* data. The change to
> krb5-protos.h fixes this.
>
> This requires a change to kdc/pkinit.c and kuser/kinit.c
> which both call krb5_get_init_creds_opt_set_pkinit
> Since they are using the terminal, the value passed is NULL.
>
>
> A SSL ENGINE UI_METHOD callback with its data is needed
> to be called so it can call the krb5_prompter_fct that
> was passed in from PAM so the ENGINE_load_private_key
> routine. A krb5_ui_data structure is created which
> has the krb5_prompter_fct, its data and a pointer to the
> krb5_context. These are passed to ENGINE_load_private_key.
>
> All the routines in pkinit.c that passed in the
> krb5_prompter_fct, need to be expanded to pass in its data.
> This includes the load_openssl_file, load_openssl_engine,
> and load_pair.
>
> A single GDM process may call PAM multiple times, and care
> must be taken to clean up the ENGINE code and any shared
> libs it has loaded. So the pkinit.c code has a few code
> changes, including a call to ENGINE_finish and making sure
> some pointers are set to NULL after a free.
>
> The pkinit.c also has the ENGINE_ctrl_cmd ("LOAD_CERT_CTRL"
> which has been accepted and committed into the OpenSC source.
> I sent this in a few weeks ago to both projects.
> (Also note that if pkinit is run against and older version of
> OpenSC if the call fails, the code it will continue on, and
> see if the cert is in a file.)
>
> The pkinit.c also has the high order bit of the pk_nonce
> turned off for Windows, and will try both the nonce and
> pk_nonce when trying to extract the ticket.
>
> (I will now try and get my mods to the SOurceforge
> pam_krb5-1.3-rc7 together which I was using to test
> all of this.)
>
> --
>
> Douglas E. Engert <DEEngert@anl.gov>
> Argonne National Laboratory
> 9700 South Cass Avenue
> Argonne, Illinois 60439
> (630) 252-5444
> --- ./kdc/,pkinit.c Wed Mar 9 09:49:23 2005
> +++ ./kdc/pkinit.c Wed Apr 27 13:28:52 2005
> @@ -1122,6 +1122,7 @@
> user_id,
> x509_anchors,
> NULL,
> + NULL,
> NULL);
> if (ret) {
> krb5_warn(context, ret, "PKINIT: failed to load");
> --- ./kuser/,kinit.c Sat Apr 23 15:29:16 2005
> +++ ./kuser/kinit.c Tue Apr 26 16:42:33 2005
> @@ -470,6 +470,7 @@
> pk_x509_anchors,
> flags,
> NULL,
> + NULL,
> NULL);
> if (ret)
> --- ./lib/krb5/,init_creds_pw.c Thu Apr 7 15:15:18 2005
> +++ ./lib/krb5/init_creds_pw.c Tue Apr 26 16:53:02 2005
> @@ -1210,9 +1210,9 @@
> krb5_generate_random_block (&ctx->nonce, sizeof(ctx->nonce));
> ctx->nonce &= 0xffffffff;
> ctx->as_req.req_body.nonce = ctx->nonce;
> -#if 0
> +#if 1
> krb5_generate_random_block (&ctx->pk_nonce, sizeof(ctx->pk_nonce));
> - ctx->pk_nonce &= 0xffffffff;
> + ctx->pk_nonce &= 0x7fffffff; /* WIN2K wants top bit off */
> #else
> ctx->pk_nonce = ctx->nonce;
> #endif
> @@ -1335,6 +1335,22 @@
> ctx->flags.b.request_anonymous,
> NULL,
> NULL);
> + if (ret == KRB5KRB_AP_ERR_MODIFIED) { /* Windows PKINIT hack */
> + ret = _krb5_extract_ticket(context,
> + &rep,
> + creds,
> + key,
> + NULL,
> + KRB5_KU_AS_REP_ENC_PART,
> + NULL,
> + ctx->pk_nonce,
> + FALSE,
> + ctx->flags.b.request_anonymous,
> + NULL,
> + NULL);
> + }
> +
> +
> krb5_free_keyblock(context, key);
> }
> out:
> --- ./lib/krb5/,krb5-private.h Mon Apr 25 19:17:09 2005
> +++ ./lib/krb5/krb5-private.h Tue Apr 26 16:54:43 2005
> @@ -275,6 +275,7 @@
> const char */*user_id*/,
> const char */*x509_anchors*/,
> krb5_prompter_fct /*prompter*/,
> + void */*prompter_data*/,
> char */*password*/);
>
> krb5_error_code KRB5_LIB_FUNCTION
> --- ./lib/krb5/,pkinit.c Sun Apr 24 09:14:49 2005
> +++ ./lib/krb5/pkinit.c Wed Apr 27 09:34:41 2005
> @@ -44,6 +44,7 @@
> #include <openssl/dh.h>
> #include <openssl/bn.h>
> #include <openssl/engine.h>
> +#include <openssl/ui.h>
>
> #ifdef HAVE_DIRENT_H
> #include <dirent.h>
> @@ -84,6 +85,19 @@
> } \
> }
>
> +/* ENGING_load_private_key requires a UI_METHOD and data
> + * if to be usable from PAM
> + */
> +
> +struct krb5_ui_data {
> + krb5_context context;
> + krb5_prompter_fct prompter;
> + void * prompter_data;
> +};
> +
> +static int krb5_ui_method_read_string(UI *ui, UI_STRING *uis);
> +
> +
> struct krb5_pk_identity {
> EVP_PKEY *private_key;
> STACK_OF(X509) *cert;
> @@ -1789,6 +1803,7 @@
> load_openssl_file(krb5_context context,
> char *password,
> krb5_prompter_fct prompter,
> + void *prompter_data,
> const char *user_id,
> struct krb5_pk_identity *id)
> {
> @@ -1969,6 +1984,7 @@
> load_openssl_engine(krb5_context context,
> char *password,
> krb5_prompter_fct prompter,
> + void *prompter_data,
> const char *string,
> struct krb5_pk_identity *id)
> {
> @@ -2045,23 +2061,62 @@
> "PKINIT: openssl engine init %s failed: %s",
> ctx.engine_name,
> ERR_error_string(ERR_get_error(), NULL));
> + ENGINE_free(e);
> goto out;
> }
> - ENGINE_free(e);
>
> ret = eval_pairs(context, e, ctx.engine_name, "post",
> ctx.post_cmds, ctx.num_post);
> if (ret)
> goto out;
>
> + /*
> + * If the engine supports a LOAD_CERT_CTRL function,
> + * lets try it. Mods for the OpenSC have been submitted
> + * to support this function. Eventially this should
> + * be a ENGINE_load_cert function
> + * if it failes, see if the CERT= is a file
> + */
> + {
> + struct {
> + const char * cert_id;
> + X509 * cert;
> + } parms;
> +
> + parms.cert_id = ctx.cert_file;
> + parms.cert = NULL;
> + ENGINE_ctrl_cmd(e, "LOAD_CERT_CTRL", 0, &parms, NULL, 1);
> + if (parms.cert) {
> + id->cert = sk_X509_new_null();
> + sk_X509_insert(id->cert, parms.cert, 0);
> + }
> + }
> + if (!id->cert) {
> ret = load_openssl_cert(context, ctx.cert_file, &id->cert);
> if (ret)
> goto out;
> + }
>
> - id->private_key = ENGINE_load_private_key(e,
> + {
> + UI_METHOD * krb5_ui_method = NULL;
> + struct krb5_ui_data ui_data;
> +
> + krb5_ui_method = UI_create_method("Krb5 ui method");
> + UI_method_set_reader(krb5_ui_method, krb5_ui_method_read_string);
> +
> + ui_data.context = context;
> + ui_data.prompter = prompter;
> + if (prompter == NULL)
> + ui_data.prompter = krb5_prompter_posix;
> + ui_data.prompter_data = prompter_data;
> +
> + id->private_key = ENGINE_load_private_key(e,
> ctx.key_id,
> - NULL,
> - NULL);
> + krb5_ui_method,
> + (void*) &ui_data);
> + UI_destroy_method(krb5_ui_method);
> + }
> +
> if (id->private_key == NULL) {
> krb5_set_error_string(context,
> "PKINIT: failed to load private key: %s",
> @@ -2093,8 +2148,10 @@
> free(user_conf);
> if (file_conf)
> free(file_conf);
> - if (e)
> - ENGINE_free(e);
> + if (e) {
> + ENGINE_finish(e); /* make sure all shared libs are unloaded */
> + ENGINE_free(e);
> + }
>
> return ret;
> }
> @@ -2105,6 +2162,7 @@
> const char *user_id,
> const char *x509_anchors,
> krb5_prompter_fct prompter,
> + void *prompter_data,
> char *password)
> {
> STACK_OF(X509) *trusted_certs = NULL;
> @@ -2117,6 +2175,7 @@
> krb5_error_code (*load_pair)(krb5_context,
> char *,
> krb5_prompter_fct prompter,
> + void * prompter_data,
> const char *,
> struct krb5_pk_identity *) = NULL;
>
> @@ -2166,7 +2225,7 @@
> ERR_load_crypto_strings();
>
>
> - ret = (*load_pair)(context, password, prompter, user_id, id);
> + ret = (*load_pair)(context, password, prompter, prompter_data, user_id, id);
> if (ret)
> goto out;
>
> @@ -2275,6 +2334,7 @@
> ctx = opt->private->pk_init_ctx;
> if (ctx->dh)
> DH_free(ctx->dh);
> + ctx->dh = NULL;
> if (ctx->id) {
> if (ctx->id->cert)
> sk_X509_pop_free(ctx->id->cert, X509_free);
> @@ -2282,9 +2342,13 @@
> sk_X509_pop_free(ctx->id->trusted_certs, X509_free);
> if (ctx->id->private_key)
> EVP_PKEY_free(ctx->id->private_key);
> - if (ctx->id->engine)
> + if (ctx->id->engine) {
> + ENGINE_finish(ctx->id->engine); /* unload shared libs etc */
> ENGINE_free(ctx->id->engine);
> + ctx->id->engine = NULL;
> + }
> free(ctx->id);
> + ctx->id = NULL;
> }
> opt->private->pk_init_ctx = NULL;
> #endif
> @@ -2298,6 +2362,7 @@
> const char *x509_anchors,
> int flags,
> krb5_prompter_fct prompter,
> + void *prompter_data,
> char *password)
> {
> #ifdef PKINIT
> @@ -2320,6 +2385,7 @@
> user_id,
> x509_anchors,
> prompter,
> + prompter_data,
> password);
> if (ret) {
> free(opt->private->pk_init_ctx);
> @@ -2375,3 +2441,54 @@
> return EINVAL;
> #endif
> }
> +
> +static int
> +krb5_ui_method_read_string(UI *ui, UI_STRING *uis)
> +{
> + /* UI ex_data has the callback_data as passed to Engina */
> + /* This is far from being complete, we will only process */
> + /* one prompt */
> +
> + char buffer[BUFSIZ];
> + krb5_error_code ret;
> + krb5_prompt prompt;
> + krb5_data password_data;
> + struct krb5_ui_data *ui_data;
> +
> + ui_data = (struct krb5_ui_data *)UI_get_app_data(ui);
> +
> + switch (UI_get_string_type(uis)) {
> + case UIT_INFO:
> + case UIT_ERROR:
> + /* looks like the RedHat pam_prompter might handle
> + * INFO and ERROR, Will see what happens */
> + case UIT_VERIFY:
> + case UIT_PROMPT:
> + password_data.data = buffer;
> + password_data.length = UI_get_result_maxsize(uis);
> + prompt.prompt = UI_get0_output_string(uis);
> + prompt.hidden = !(UI_get_input_flags(uis) & UI_INPUT_FLAG_ECHO);
> + prompt.reply = &password_data;
> + prompt.type = KRB5_PROMPT_TYPE_PASSWORD;
> +
> + ret = (*ui_data->prompter)(ui_data->context,
> + ui_data->prompter_data, NULL, NULL, 1, &prompt);
> + if (ret == 0) {
> + UI_set_result(ui, uis, password_data.data);
> + /* the RedHat pam_krb5 pam_prompter does a strdup */
> + /* but others may copy into buffer */
> + if (password_data.data && password_data.data != &buffer)
> + free (password_data.data);
> + memset (buffer, 0, sizeof(buffer));
> + return 1;
> + }
> + break;
> + case UIT_NONE:
> + case UIT_BOOLEAN:
> + /*DEE for now do not handle */
> + break;
> +
> + }
> + return 0;
> +}
> +
> --- ./lib/krb5/,krb5-protos.h Mon Apr 25 19:17:09 2005
> +++ ./lib/krb5/krb5-protos.h Thu Apr 28 11:24:34 2005
> @@ -1830,6 +1830,7 @@
> const char */*x509_anchors*/,
> int /*flags*/,
> krb5_prompter_fct /*prompter*/,
> + void */*data*/,
> char */*password*/);
>
> void KRB5_LIB_FUNCTION
> --- ./lib/roken/,getifaddrs.c Tue Apr 12 06:28:46 2005
> +++ ./lib/roken/getifaddrs.c Wed Apr 27 16:35:32 2005
> @@ -670,6 +670,7 @@
> case IFLA_QDISC:
> break;
> default:
> + break;
> }
> break;
> case RTM_NEWADDR:
> @@ -710,6 +711,7 @@
> case IFA_CACHEINFO:
> break;
> default:
> + break;
> }
> }
> }
> --- ./lib/com_err/,com_err.c Sun Apr 24 14:42:39 2005
> +++ ./lib/com_err/com_err.c Thu Apr 28 14:42:41 2005
> @@ -56,7 +56,7 @@
> p = strerror(code);
> }
> if (p != NULL && *p != '\0') {
> - strlcpy(msg, p, sizeof(msg));
> + strncpy(msg, p, sizeof(msg));
> } else
> snprintf(msg, sizeof(msg), "Unknown error %ld", code);
> return msg;
PGP signature