[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Use of PKINIT from PAM
- To: Love Hörnquist Åstrand <lha@kth.se>
- Subject: Re: Use of PKINIT from PAM
- From: "Douglas E. Engert" <deengert@anl.gov>
- Date: Sat, 30 Apr 2005 20:42:28 -0500
- Cc: heimdal-discuss@sics.se
- In-Reply-To: <am4qdo2q52.fsf@nutcracker.it.su.se>
- References: <42715DC5.8090509@anl.gov> <am4qdo2q52.fsf@nutcracker.it.su.se>
- Sender: owner-heimdal-discuss@sics.se
- User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.2) Gecko/20040803
Thanks, I will download a new version on Monday.
Love Hörnquist Åstrand wrote:
> 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);
Yes,sounds safer. This was not clear, as the pam_krb5 would do a strdup,
and replace the password_data.data but some other routine might expect the
password_data.data to point at a buffer.
>
>
> 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.
So far the PAM interface is primative. If the password from PAM is
blank, it tries PKINIT, and a seperate prompt is issued for the pin.
But there might be a better way, like test if these is a smartcard
present in the reader then use it, if not assume use passwords.
Right now that functionality is missing.
If this functionality was added would it be a seperate call, or
part of krb5_get_init_creds_opt_pkint, or krb5_get_init_creds?
,
>
> 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;
--
Douglas E. Engert <DEEngert@anl.gov>
Argonne National Laboratory
9700 South Cass Avenue
Argonne, Illinois 60439
(630) 252-5444