krb5 commit: Make empty passwords work via init_creds APIs

Greg Hudson ghudson at MIT.EDU
Mon May 27 13:05:22 EDT 2013


https://github.com/krb5/krb5/commit/f3458ed803ae97b6c6c7c63baeb82b26c4943d4c
commit f3458ed803ae97b6c6c7c63baeb82b26c4943d4c
Author: Greg Hudson <ghudson at mit.edu>
Date:   Thu May 23 15:33:58 2013 -0400

    Make empty passwords work via init_creds APIs
    
    In the gak_data value used by krb5_get_as_key_password, separate the
    already-known password from the storage we might have allocated to put
    it in, so that we no longer use an empty data buffer to determine
    whether we know the password.  This allows empty passwords to work via
    the API.
    
    Remove the kadm5 test which explicitly uses an empty password.
    
    Based on a patch from Stef Walter.
    
    ticket: 7642

 src/lib/kadm5/unit-test/api.current/init.exp |   27 -------
 src/lib/krb5/krb/get_in_tkt.c                |    5 +-
 src/lib/krb5/krb/gic_pwd.c                   |  107 +++++++++++++-------------
 src/lib/krb5/krb/init_creds_ctx.h            |    7 ++-
 src/tests/Makefile.in                        |   11 ++-
 src/tests/deps                               |    2 +
 src/tests/t_general.py                       |    9 ++
 src/tests/t_init_creds.c                     |   88 +++++++++++++++++++++
 8 files changed, 166 insertions(+), 90 deletions(-)

diff --git a/src/lib/kadm5/unit-test/api.current/init.exp b/src/lib/kadm5/unit-test/api.current/init.exp
index b324df8..d9ae3fb 100644
--- a/src/lib/kadm5/unit-test/api.current/init.exp
+++ b/src/lib/kadm5/unit-test/api.current/init.exp
@@ -99,33 +99,6 @@ proc test6 {} {
 }
 if { $RPC } { test6 } 
 
-test "init 7"
-proc test7 {} {
-    global test
-
-    send "kadm5_init admin \"\" \$KADM5_ADMIN_SERVICE null \$KADM5_STRUCT_VERSION \$KADM5_API_VERSION_3 server_handle\n"
-
-    expect {
-	-re "assword\[^\r\n\]*:" { }
-	-re "key:$" { }
-	eof {
-		fail "$test: eof instead of password prompt"
-		api_exit
-		api_start
-		return
-	}
-	timeout {
-	    fail "$test: timeout instead of password prompt"
-	    return
-	}
-    }
-    one_line_succeed_test "admin"
-    if {! [cmd {kadm5_destroy $server_handle}]} {
-	error_and_restart "$test: couldn't close database"
-    }
-}
-if { $RPC } { test7 } 
-
 test "init 8"
 
 proc test8 {} {
diff --git a/src/lib/krb5/krb/get_in_tkt.c b/src/lib/krb5/krb/get_in_tkt.c
index 59614e7..20bc689 100644
--- a/src/lib/krb5/krb/get_in_tkt.c
+++ b/src/lib/krb5/krb/get_in_tkt.c
@@ -493,8 +493,7 @@ krb5_init_creds_free(krb5_context context,
     }
     k5_response_items_free(ctx->rctx.items);
     free(ctx->in_tkt_service);
-    zap(ctx->password.data, ctx->password.length);
-    krb5_free_data_contents(context, &ctx->password);
+    zapfree(ctx->gakpw.storage.data, ctx->gakpw.storage.length);
     krb5_free_error(context, ctx->err_reply);
     krb5_free_pa_data(context, ctx->err_padata);
     krb5_free_cred_contents(context, &ctx->cred);
@@ -788,7 +787,7 @@ krb5_init_creds_init(krb5_context context,
     ctx->prompter = prompter;
     ctx->prompter_data = data;
     ctx->gak_fct = krb5_get_as_key_password;
-    ctx->gak_data = &ctx->password;
+    ctx->gak_data = &ctx->gakpw;
 
     ctx->request_time = 0; /* filled in later */
     ctx->start_time = start_time;
diff --git a/src/lib/krb5/krb/gic_pwd.c b/src/lib/krb5/krb/gic_pwd.c
index 22db2b5..a97823f 100644
--- a/src/lib/krb5/krb/gic_pwd.c
+++ b/src/lib/krb5/krb/gic_pwd.c
@@ -17,22 +17,19 @@ krb5_get_as_key_password(krb5_context context,
                          void *gak_data,
                          k5_response_items *ritems)
 {
-    krb5_data *password;
+    struct gak_password *gp = gak_data;
     krb5_error_code ret;
     krb5_data defsalt;
     char *clientstr;
-    char promptstr[1024];
+    char promptstr[1024], pwbuf[1024];
+    krb5_data pw;
     krb5_prompt prompt;
     krb5_prompt_type prompt_type;
     const char *rpass;
 
-    password = (krb5_data *) gak_data;
-    assert(password->length > 0);
-
     /* If we need to get the AS key via the responder, ask for it. */
     if (as_key == NULL) {
-        /* However, if we already have a password, don't ask. */
-        if (password->data[0] != '\0')
+        if (gp->password != NULL)
             return 0;
 
         return k5_response_items_ask_question(ritems,
@@ -55,17 +52,20 @@ krb5_get_as_key_password(krb5_context context,
         }
     }
 
-    if (password->data[0] == '\0') {
+    if (gp->password == NULL) {
         /* Check the responder for the password. */
         rpass = k5_response_items_get_answer(ritems,
                                              KRB5_RESPONDER_QUESTION_PASSWORD);
         if (rpass != NULL) {
-            strlcpy(password->data, rpass, password->length);
-            password->length = strlen(password->data);
+            ret = alloc_data(&gp->storage, strlen(rpass));
+            if (ret)
+                return ret;
+            memcpy(gp->storage.data, rpass, strlen(rpass));
+            gp->password = &gp->storage;
         }
     }
 
-    if (password->data[0] == '\0') {
+    if (gp->password == NULL) {
         if (prompter == NULL)
             return(EIO);
 
@@ -76,9 +76,10 @@ krb5_get_as_key_password(krb5_context context,
                  clientstr);
         free(clientstr);
 
+        pw = make_data(pwbuf, sizeof(pwbuf));
         prompt.prompt = promptstr;
         prompt.hidden = 1;
-        prompt.reply = password;
+        prompt.reply = &pw;
         prompt_type = KRB5_PROMPT_TYPE_PASSWORD;
 
         /* PROMPTER_INVOCATION */
@@ -87,6 +88,12 @@ krb5_get_as_key_password(krb5_context context,
         k5_set_prompt_types(context, 0);
         if (ret)
             return(ret);
+
+        ret = krb5int_copy_data_contents(context, &pw, &gp->storage);
+        zap(pw.data, pw.length);
+        if (ret)
+            return ret;
+        gp->password = &gp->storage;
     }
 
     if (salt == NULL) {
@@ -98,7 +105,7 @@ krb5_get_as_key_password(krb5_context context,
         defsalt.length = 0;
     }
 
-    ret = krb5_c_string_to_key_with_params(context, etype, password, salt,
+    ret = krb5_c_string_to_key_with_params(context, etype, gp->password, salt,
                                            params->data?params:NULL, as_key);
 
     if (defsalt.length)
@@ -118,16 +125,11 @@ krb5_init_creds_set_password(krb5_context context,
     if (s == NULL)
         return ENOMEM;
 
-    if (ctx->password.data != NULL) {
-        zap(ctx->password.data, ctx->password.length);
-        krb5_free_data_contents(context, &ctx->password);
-    }
-
-    ctx->password.data = s;
-    ctx->password.length = strlen(s);
+    zapfree(ctx->gakpw.storage.data, ctx->gakpw.storage.length);
+    ctx->gakpw.storage = string2data(s);
+    ctx->gakpw.password = &ctx->gakpw.storage;
     ctx->gak_fct = krb5_get_as_key_password;
-    ctx->gak_data = &ctx->password;
-
+    ctx->gak_data = &ctx->gakpw;
     return 0;
 }
 
@@ -257,6 +259,7 @@ krb5_get_init_creds_password(krb5_context context,
     int tries;
     krb5_creds chpw_creds;
     krb5_get_init_creds_opt *chpw_opts = NULL;
+    struct gak_password gakpw;
     krb5_data pw0, pw1;
     char banner[1024], pw0array[1024], pw1array[1024];
     krb5_prompt prompt[2];
@@ -267,29 +270,18 @@ krb5_get_init_creds_password(krb5_context context,
     use_master = 0;
     as_reply = NULL;
     memset(&chpw_creds, 0, sizeof(chpw_creds));
+    memset(&gakpw, 0, sizeof(gakpw));
 
-    pw0.data = pw0array;
-
-    if (password && password[0]) {
-        if (strlcpy(pw0.data, password, sizeof(pw0array)) >= sizeof(pw0array)) {
-            ret = EINVAL;
-            goto cleanup;
-        }
-        pw0.length = strlen(password);
-    } else {
-        pw0.data[0] = '\0';
-        pw0.length = sizeof(pw0array);
+    if (password != NULL) {
+        pw0 = string2data((char *)password);
+        gakpw.password = &pw0;
     }
 
-    pw1.data = pw1array;
-    pw1.data[0] = '\0';
-    pw1.length = sizeof(pw1array);
-
     /* first try: get the requested tkt from any kdc */
 
     ret = k5_get_init_creds(context, creds, client, prompter, data, start_time,
                             in_tkt_service, options, krb5_get_as_key_password,
-                            (void *) &pw0, &use_master, &as_reply);
+                            &gakpw, &use_master, &as_reply);
 
     /* check for success */
 
@@ -318,8 +310,8 @@ krb5_get_init_creds_password(krb5_context context,
         }
         ret = k5_get_init_creds(context, creds, client, prompter, data,
                                 start_time, in_tkt_service, options,
-                                krb5_get_as_key_password, (void *) &pw0,
-                                &use_master, &as_reply);
+                                krb5_get_as_key_password, &gakpw, &use_master,
+                                &as_reply);
 
         if (ret == 0)
             goto cleanup;
@@ -365,16 +357,22 @@ krb5_get_init_creds_password(krb5_context context,
 
     ret = k5_get_init_creds(context, &chpw_creds, client, prompter, data,
                             start_time, "kadmin/changepw", chpw_opts,
-                            krb5_get_as_key_password, (void *) &pw0,
-                            &use_master, NULL);
+                            krb5_get_as_key_password, &gakpw, &use_master,
+                            NULL);
     if (ret)
         goto cleanup;
 
+    pw0.data = pw0array;
+    pw0.data[0] = '\0';
+    pw0.length = sizeof(pw0array);
     prompt[0].prompt = _("Enter new password");
     prompt[0].hidden = 1;
     prompt[0].reply = &pw0;
     prompt_types[0] = KRB5_PROMPT_TYPE_NEW_PASSWORD;
 
+    pw1.data = pw1array;
+    pw1.data[0] = '\0';
+    pw1.length = sizeof(pw1array);
     prompt[1].prompt = _("Enter it again");
     prompt[1].hidden = 1;
     prompt[1].reply = &pw1;
@@ -460,10 +458,11 @@ krb5_get_init_creds_password(krb5_context context,
        is final.  */
 
     TRACE_GIC_PWD_CHANGED(context);
+    gakpw.password = &pw0;
     ret = k5_get_init_creds(context, creds, client, prompter, data,
                             start_time, in_tkt_service, options,
-                            krb5_get_as_key_password, (void *) &pw0,
-                            &use_master, &as_reply);
+                            krb5_get_as_key_password, &gakpw, &use_master,
+                            &as_reply);
     if (ret)
         goto cleanup;
 
@@ -474,6 +473,7 @@ cleanup:
 
     if (chpw_opts)
         krb5_get_init_creds_opt_free(context, chpw_opts);
+    zapfree(gakpw.storage.data, gakpw.storage.length);
     memset(pw0array, 0, sizeof(pw0array));
     memset(pw1array, 0, sizeof(pw1array));
     krb5_free_cred_contents(context, &chpw_creds);
@@ -512,21 +512,17 @@ krb5_get_in_tkt_with_password(krb5_context context, krb5_flags options,
                               krb5_creds *creds, krb5_kdc_rep **ret_as_reply)
 {
     krb5_error_code retval;
-    krb5_data pw0;
-    char pw0array[1024];
+    struct gak_password gakpw;
+    krb5_data pw;
     char * server;
     krb5_principal server_princ, client_princ;
     int use_master = 0;
     krb5_get_init_creds_opt *opts = NULL;
 
-    pw0.data = pw0array;
-    if (password && password[0]) {
-        if (strlcpy(pw0.data, password, sizeof(pw0array)) >= sizeof(pw0array))
-            return EINVAL;
-        pw0.length = strlen(password);
-    } else {
-        pw0.data[0] = '\0';
-        pw0.length = sizeof(pw0array);
+    memset(&gakpw, 0, sizeof(gakpw));
+    if (password != NULL) {
+        pw = string2data((char *)password);
+        gakpw.password = &pw;
     }
     retval = k5_populate_gic_opt(context, &opts, options, addrs, ktypes,
                                  pre_auth_types, creds);
@@ -541,10 +537,11 @@ krb5_get_in_tkt_with_password(krb5_context context, krb5_flags options,
     client_princ = creds->client;
     retval = k5_get_init_creds(context, creds, creds->client,
                                krb5_prompter_posix, NULL, 0, server, opts,
-                               krb5_get_as_key_password, &pw0, &use_master,
+                               krb5_get_as_key_password, &gakpw, &use_master,
                                ret_as_reply);
     krb5_free_unparsed_name( context, server);
     krb5_get_init_creds_opt_free(context, opts);
+    zapfree(gakpw.storage.data, gakpw.storage.length);
     if (retval) {
         return (retval);
     }
diff --git a/src/lib/krb5/krb/init_creds_ctx.h b/src/lib/krb5/krb/init_creds_ctx.h
index d886c7a..4dbb0e9 100644
--- a/src/lib/krb5/krb/init_creds_ctx.h
+++ b/src/lib/krb5/krb/init_creds_ctx.h
@@ -10,6 +10,11 @@ struct krb5_responder_context_st {
     k5_response_items *items;
 };
 
+struct gak_password {
+    krb5_data storage;
+    const krb5_data *password;
+};
+
 struct _krb5_init_creds_context {
     krb5_gic_opt_ext *opte;
     char *in_tkt_service;
@@ -23,7 +28,7 @@ struct _krb5_init_creds_context {
     krb5_deltat renew_life;
     krb5_boolean complete;
     unsigned int loopcount;
-    krb5_data password;
+    struct gak_password gakpw;
     krb5_error *err_reply;
     krb5_pa_data **err_padata;
     krb5_creds cred;
diff --git a/src/tests/Makefile.in b/src/tests/Makefile.in
index 2358c89..91f312e 100644
--- a/src/tests/Makefile.in
+++ b/src/tests/Makefile.in
@@ -5,8 +5,8 @@ SUBDIRS = resolve asn.1 create hammer verify gssapi dejagnu shlib \
 
 RUN_SETUP = @KRB5_RUN_ENV@ KRB5_KDC_PROFILE=kdc.conf KRB5_CONFIG=krb5.conf
 
-OBJS= gcred.o hist.o kdbtest.o t_localauth.o
-EXTRADEPSRCS= gcred.c hist.c kdbtest.c t_localauth.c
+OBJS= gcred.o hist.o kdbtest.o t_init_creds.o t_localauth.o
+EXTRADEPSRCS= gcred.c hist.c kdbtest.c t_init_creds.c t_localauth.c
 
 TEST_DB = ./testdb
 TEST_REALM = FOO.TEST.REALM
@@ -28,6 +28,9 @@ kdbtest: kdbtest.o $(KDB5_DEPLIBS) $(KADMSRV_DEPLIBS) $(KRB5_BASE_DEPLIBS)
 	$(CC_LINK) -o $@ kdbtest.o $(KDB5_LIBS) $(KADMSRV_LIBS) \
 		$(KRB5_BASE_LIBS)
 
+t_init_creds: t_init_creds.o $(KRB5_BASE_DEPLIBS)
+	$(CC_LINK) -o $@ t_init_creds.o $(KRB5_BASE_LIBS)
+
 t_localauth: t_localauth.o $(KRB5_BASE_DEPLIBS)
 	$(CC_LINK) -o $@ t_localauth.o $(KRB5_BASE_LIBS)
 
@@ -73,7 +76,7 @@ kdb_check: kdc.conf krb5.conf
 	$(RUN_SETUP) $(VALGRIND) ../kadmin/dbutil/kdb5_util $(KADMIN_OPTS) destroy -f
 	$(RM) $(TEST_DB)* stash_file
 
-check-pytests:: gcred hist kdbtest t_localauth
+check-pytests:: gcred hist kdbtest t_init_creds t_localauth
 	$(RUNPYTEST) $(srcdir)/t_general.py $(PYTESTFLAGS)
 	$(RUNPYTEST) $(srcdir)/t_dump.py $(PYTESTFLAGS)
 	$(RUNPYTEST) $(srcdir)/t_iprop.py $(PYTESTFLAGS)
@@ -101,5 +104,5 @@ check-pytests:: gcred hist kdbtest t_localauth
 	$(RUNPYTEST) $(srcdir)/t_cve-2013-1416.py $(PYTESTFLAGS)
 
 clean::
-	$(RM) gcred hist kdbtest krb5.conf kdc.conf t_localauth
+	$(RM) gcred hist kdbtest krb5.conf kdc.conf t_init_creds t_localauth
 	$(RM) -rf kdc_realm/sandbox ldap
diff --git a/src/tests/deps b/src/tests/deps
index e21779e..b50d0df 100644
--- a/src/tests/deps
+++ b/src/tests/deps
@@ -40,5 +40,7 @@ $(OUTPRE)kdbtest.$(OBJEXT): $(BUILDTOP)/include/gssapi/gssapi.h \
   $(top_srcdir)/include/gssrpc/svc.h $(top_srcdir)/include/gssrpc/svc_auth.h \
   $(top_srcdir)/include/gssrpc/xdr.h $(top_srcdir)/include/kdb.h \
   $(top_srcdir)/include/krb5.h kdbtest.c
+$(OUTPRE)t_init_creds.$(OBJEXT): $(BUILDTOP)/include/krb5/krb5.h \
+  $(COM_ERR_DEPS) $(top_srcdir)/include/krb5.h t_init_creds.c
 $(OUTPRE)t_localauth.$(OBJEXT): $(BUILDTOP)/include/krb5/krb5.h \
   $(COM_ERR_DEPS) $(top_srcdir)/include/krb5.h t_localauth.c
diff --git a/src/tests/t_general.py b/src/tests/t_general.py
index bb7a543..98e77a2 100755
--- a/src/tests/t_general.py
+++ b/src/tests/t_general.py
@@ -22,6 +22,15 @@ for realm in multipass_realms(create_host=False):
     # Test kinit against kdb keytab
     realm.run([kinit, "-k", "-t", "KDB:", realm.user_princ])
 
+# Test that we can get initial creds with an empty password via the
+# API.  We have to disable the "empty" pwqual module to create a
+# principal with an empty password.  (Regression test for #7642.)
+conf={'plugins': {'pwqual': {'disable': 'empty'}}}
+realm = K5Realm(create_user=False, create_host=False, krb5_conf=conf)
+realm.run_kadminl('addprinc -pw "" user')
+realm.run(['./t_init_creds', 'user', ''])
+realm.stop()
+
 realm = K5Realm(create_host=False)
 
 # Spot-check KRB5_TRACE output
diff --git a/src/tests/t_init_creds.c b/src/tests/t_init_creds.c
new file mode 100644
index 0000000..6be8340
--- /dev/null
+++ b/src/tests/t_init_creds.c
@@ -0,0 +1,88 @@
+/* -*- mode: c; c-basic-offset: 4; indent-tabs-mode: nil -*- */
+/* tests/t_init_creds.c - test harness for getting initial creds */
+/*
+ * Copyright (C) 2013 by the Massachusetts Institute of Technology.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * * Redistributions in binary form must reproduce the above copyright
+ *   notice, this list of conditions and the following disclaimer in
+ *   the documentation and/or other materials provided with the
+ *   distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
+ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
+ * OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+/*
+ * This program exercises the init_creds APIs in ways kinit doesn't.  Right now
+ * it is very simplistic, but it can be extended as needed.
+ */
+
+#include <krb5.h>
+#include <stdio.h>
+
+static krb5_context ctx;
+
+static void
+check(krb5_error_code code)
+{
+    const char *errmsg;
+
+    if (code) {
+        errmsg = krb5_get_error_message(ctx, code);
+        fprintf(stderr, "%s\n", errmsg);
+        krb5_free_error_message(ctx, errmsg);
+        exit(1);
+    }
+}
+
+int
+main(int argc, char **argv)
+{
+    const char *princstr, *password;
+    krb5_principal client;
+    krb5_init_creds_context icc;
+    krb5_creds creds;
+
+    if (argc != 3) {
+        fprintf(stderr, "Usage: t_init_creds princname password\n");
+        exit(1);
+    }
+    princstr = argv[1];
+    password = argv[2];
+
+    check(krb5_init_context(&ctx));
+    check(krb5_parse_name(ctx, princstr, &client));
+
+    /* Try once with the traditional interface. */
+    check(krb5_get_init_creds_password(ctx, &creds, client, password, NULL,
+                                       NULL, 0, NULL, NULL));
+    krb5_free_cred_contents(ctx, &creds);
+
+    /* Try again with the step interface. */
+    check(krb5_init_creds_init(ctx, client, NULL, NULL, 0, NULL, &icc));
+    check(krb5_init_creds_set_password(ctx, icc, password));
+    check(krb5_init_creds_get(ctx, icc));
+    krb5_init_creds_free(ctx, icc);
+
+    krb5_free_principal(ctx, client);
+    krb5_free_context(ctx);
+    return 0;
+}


More information about the cvs-krb5 mailing list