krb5 commit: Fix kprop and kpropd realm handling

Greg Hudson ghudson at mit.edu
Mon Jun 13 12:46:28 EDT 2016


https://github.com/krb5/krb5/commit/0e7b4c901bf475f61b74ca56363089906e324272
commit 0e7b4c901bf475f61b74ca56363089906e324272
Author: Matt Rogers <mrogers at redhat.com>
Date:   Thu Feb 25 10:38:07 2016 -0500

    Fix kprop and kpropd realm handling
    
    Add the sn2princ_with_realm() helper function (currently duplicated in
    kprop.c and kpropd.c) to simplify principal realm substitution.  Use
    sn2princ_with_realm() in kprop.c and kpropd.c in place of
    krb5_sname_to_principal(), with the default realm if -r is not provided.
    If a realm is given to kpropd, set it as the default realm on the
    kpropd_context, allowing a later call of ulog_replay() to open the
    correct database.
    
    Remove referral realm code in kprop.c and kpropd.c.  Pass the realm
    (default or provided) to the kdb5_util and kprop commands called by
    kadmind.
    
    ticket: 8277

 src/kadmin/server/ipropd_svc.c |   15 +++---
 src/slave/kprop.c              |   80 +++++++++++++++++++++-------------
 src/slave/kpropd.c             |   93 ++++++++++++++++++++++-----------------
 3 files changed, 109 insertions(+), 79 deletions(-)

diff --git a/src/kadmin/server/ipropd_svc.c b/src/kadmin/server/ipropd_svc.c
index 7c7f850..336e5de 100644
--- a/src/kadmin/server/ipropd_svc.c
+++ b/src/kadmin/server/ipropd_svc.c
@@ -335,8 +335,8 @@ ipropx_resync(uint32_t vers, struct svc_req *rqstp)
      * and timestamp are in the ulog (then the slaves can get the
      * subsequent updates very iprop).
      */
-    if (asprintf(&ubuf, "%s dump -i%d -c %s",
-		 kdb5_util, vers, dump_file) < 0) {
+    if (asprintf(&ubuf, "%s -r %s dump -i%d -c %s", kdb5_util,
+		 handle->params.realm, vers, dump_file) < 0) {
 	krb5_klog_syslog(LOG_ERR,
 			 _("%s: cannot construct kdb5 util dump string too long; out of memory"),
 			 whoami);
@@ -390,14 +390,15 @@ ipropx_resync(uint32_t vers, struct svc_req *rqstp)
 	    _exit(1);
 	}
 
-	DPRINT("%s: exec `kprop -f %s %s' ...\n",
-		whoami, dump_file, clhost);
+	DPRINT("%s: exec `kprop -r %s -f %s %s' ...\n",
+	       handle->params.realm, whoami, dump_file, clhost);
 	/* XXX Yuck!  */
 	if (getenv("KPROP_PORT")) {
-	    pret = execl(kprop, "kprop", "-f", dump_file, "-P",
-			 getenv("KPROP_PORT"), clhost, NULL);
+	    pret = execl(kprop, "kprop", "-r", handle->params.realm, "-f",
+			 dump_file, "-P", getenv("KPROP_PORT"), clhost, NULL);
 	} else {
-	    pret = execl(kprop, "kprop", "-f", dump_file, clhost, NULL);
+	    pret = execl(kprop, "kprop", "-r", handle->params.realm, "-f",
+			 dump_file, clhost, NULL);
 	}
 	perror(whoami);
 	krb5_klog_syslog(LOG_ERR,
diff --git a/src/slave/kprop.c b/src/slave/kprop.c
index 696e89c..955db50 100644
--- a/src/slave/kprop.c
+++ b/src/slave/kprop.c
@@ -52,6 +52,7 @@ static int debug = 0;
 static char *srvtab = NULL;
 static char *slave_host;
 static char *realm = NULL;
+static char *def_realm = NULL;
 static char *file = KPROP_DEFAULT_FILE;
 
 /* The Kerberos principal we'll be sending as, initialized in get_tickets. */
@@ -63,7 +64,7 @@ static krb5_address *receiver_addr;
 static const char *port = KPROP_SERVICE;
 static char *dbpathname;
 
-static void parse_args(int argc, char **argv);
+static void parse_args(krb5_context context, int argc, char **argv);
 static void get_tickets(krb5_context context);
 static void usage(void);
 static void open_connection(krb5_context context, char *host, int *fd_out);
@@ -101,7 +102,7 @@ main(int argc, char **argv)
         com_err(argv[0], retval, _("while initializing krb5"));
         exit(1);
     }
-    parse_args(argc, argv);
+    parse_args(context, argc, argv);
     get_tickets(context);
 
     database_fd = open_database(context, file, &database_size);
@@ -113,13 +114,15 @@ main(int argc, char **argv)
     printf(_("Database propagation to %s: SUCCEEDED\n"), slave_host);
     krb5_free_cred_contents(context, my_creds);
     close_database(context, database_fd);
+    krb5_free_default_realm(context, def_realm);
     exit(0);
 }
 
 static void
-parse_args(int argc, char **argv)
+parse_args(krb5_context context, int argc, char **argv)
 {
     char *word, ch;
+    krb5_error_code ret;
 
     progname = *argv++;
     while (--argc && (word = *argv++) != NULL) {
@@ -168,51 +171,66 @@ parse_args(int argc, char **argv)
     }
     if (slave_host == NULL)
         usage();
+
+    if (realm == NULL) {
+        ret = krb5_get_default_realm(context, &def_realm);
+        if (ret) {
+            com_err(progname, errno, _("while getting default realm"));
+            exit(1);
+        }
+        realm = def_realm;
+    }
+}
+
+/* Runs krb5_sname_to_principal with a substitute realm
+ * Duplicated in kpropd.c, sharing TBD */
+static krb5_error_code
+sn2princ_with_realm(krb5_context context, const char *hostname,
+                    const char *sname, krb5_int32 type, const char *rrealm,
+                    krb5_principal *princ_out)
+{
+    krb5_error_code ret;
+    krb5_principal princ = NULL;
+
+    *princ_out = NULL;
+
+    if (rrealm == NULL)
+        return EINVAL;
+
+    ret = krb5_sname_to_principal(context, hostname, sname, type, &princ);
+    if (ret)
+        return ret;
+
+    ret = krb5_set_principal_realm(context, princ, rrealm);
+    if (ret) {
+        krb5_free_principal(context, princ);
+        return ret;
+    }
+
+    *princ_out = princ;
+    return 0;
 }
 
 static void
 get_tickets(krb5_context context)
 {
-    char *def_realm, *server;
+    char *server;
     krb5_error_code retval;
     krb5_keytab keytab = NULL;
     krb5_principal server_princ = NULL;
 
     /* Figure out what tickets we'll be using to send. */
-    retval = krb5_sname_to_principal(context, NULL, NULL, KRB5_NT_SRV_HST,
-                                     &my_principal);
+    retval = sn2princ_with_realm(context, NULL, NULL, KRB5_NT_SRV_HST, realm,
+                                 &my_principal);
     if (retval) {
         com_err(progname, errno, _("while setting client principal name"));
         exit(1);
     }
-    if (realm != NULL) {
-        retval = krb5_set_principal_realm(context, my_principal, realm);
-        if (retval) {
-            com_err(progname, errno,
-                    _("while setting client principal realm"));
-            exit(1);
-        }
-    } else if (krb5_is_referral_realm(krb5_princ_realm(context,
-                                                       my_principal))) {
-        /* We're going to use this as a client principal, so it can't have the
-         * referral realm.  Use the default realm instead. */
-        retval = krb5_get_default_realm(context, &def_realm);
-        if (retval) {
-            com_err(progname, errno, _("while getting default realm"));
-            exit(1);
-        }
-        retval = krb5_set_principal_realm(context, my_principal, def_realm);
-        if (retval) {
-            com_err(progname, errno,
-                    _("while setting client principal realm"));
-            exit(1);
-        }
-    }
 
     /* Construct the principal name for the slave host. */
     memset(&creds, 0, sizeof(creds));
-    retval = krb5_sname_to_principal(context, slave_host, KPROP_SERVICE_NAME,
-                                     KRB5_NT_SRV_HST, &server_princ);
+    retval = sn2princ_with_realm(context, slave_host, KPROP_SERVICE_NAME,
+                                 KRB5_NT_SRV_HST, realm, &server_princ);
     if (retval) {
         com_err(progname, errno, _("while setting server principal name"));
         exit(1);
diff --git a/src/slave/kpropd.c b/src/slave/kpropd.c
index 1383156..1b60126 100644
--- a/src/slave/kpropd.c
+++ b/src/slave/kpropd.c
@@ -90,7 +90,6 @@ extern int daemon(int, int);
 
 #define SYSLOG_CLASS LOG_DAEMON
 
-char *def_realm = NULL;
 int runonce = 0;
 
 /*
@@ -128,6 +127,7 @@ static krb5_principal client;   /* This is who we're talking to */
 static krb5_context kpropd_context;
 static krb5_auth_context auth_context;
 static char *realm = NULL;      /* Our realm */
+static char *def_realm = NULL;  /* Ref pointer for default realm */
 static char *file = KPROPD_DEFAULT_FILE;
 static char *temp_file_name;
 static char *kdb5_util = KPROPD_DEFAULT_KDB5_UTIL;
@@ -601,6 +601,34 @@ full_resync(CLIENT *clnt)
     return (status == RPC_SUCCESS) ? &clnt_res : NULL;
 }
 
+/* Runs krb5_sname_to_principal with a substitute realm.
+ * Duplicated in kprop.c, sharing TBD */
+static krb5_error_code
+sn2princ_with_realm(krb5_context context, const char *hostname,
+                    const char *sname, krb5_int32 type, const char *rrealm,
+                    krb5_principal *princ_out)
+{
+    krb5_error_code ret;
+    krb5_principal princ = NULL;
+
+    *princ_out = NULL;
+
+    if (rrealm == NULL)
+        return EINVAL;
+
+    ret = krb5_sname_to_principal(context, hostname, sname, type, &princ);
+    if (ret)
+        return ret;
+
+    ret = krb5_set_principal_realm(context, princ, rrealm);
+    if (ret) {
+        krb5_free_principal(context, princ);
+        return ret;
+    }
+
+    *princ_out = princ;
+    return 0;
+}
 /*
  * Beg for incrementals from the KDC.
  *
@@ -631,53 +659,26 @@ do_iprop()
     if (pollin == 0)
         pollin = 10;
 
-    /* Grab the realm info and check if iprop is enabled. */
-    if (def_realm == NULL) {
-        retval = krb5_get_default_realm(kpropd_context, &def_realm);
-        if (retval) {
-            com_err(progname, retval, _("Unable to get default realm"));
-            return retval;
-        }
-    }
-
-    params.mask |= KADM5_CONFIG_REALM;
-    params.realm = def_realm;
-
     if (master_svc_princstr == NULL) {
-        retval = kadm5_get_kiprop_host_srv_name(kpropd_context, def_realm,
+        retval = kadm5_get_kiprop_host_srv_name(kpropd_context, realm,
                                                 &master_svc_princstr);
         if (retval) {
             com_err(progname, retval,
                     _("%s: unable to get kiprop host based "
                       "service name for realm %s\n"),
-                    progname, def_realm);
+                    progname, realm);
             return retval;
         }
     }
 
-    retval = krb5_sname_to_principal(kpropd_context, NULL, KIPROP_SVC_NAME,
-                                     KRB5_NT_SRV_HST, &iprop_svc_principal);
+    retval = sn2princ_with_realm(kpropd_context, NULL, KIPROP_SVC_NAME,
+                                 KRB5_NT_SRV_HST, realm, &iprop_svc_principal);
     if (retval) {
         com_err(progname, retval,
                 _("while trying to construct host service principal"));
         return retval;
     }
 
-    /* XXX referrals? */
-    if (krb5_is_referral_realm(krb5_princ_realm(kpropd_context,
-                                                iprop_svc_principal))) {
-        krb5_data *r = krb5_princ_realm(kpropd_context,
-                                        iprop_svc_principal);
-        assert(def_realm != NULL);
-        r->length = strlen(def_realm);
-        r->data = strdup(def_realm);
-        if (r->data == NULL) {
-            com_err(progname, retval,
-                    _("while determining local service principal name"));
-            return retval;
-        }
-        /* XXX Memory leak: Old r->data value.  */
-    }
     retval = krb5_unparse_name(kpropd_context, iprop_svc_principal,
                                &iprop_svc_princstr);
     if (retval) {
@@ -1157,21 +1158,29 @@ parse_args(char **argv)
     if (!debug)
         set_com_err_hook(kpropd_com_err_proc);
 
+    if (realm == NULL) {
+        retval = krb5_get_default_realm(kpropd_context, &def_realm);
+        if (retval) {
+            com_err(progname, retval, _("Unable to get default realm"));
+            exit(1);
+        }
+        realm = def_realm;
+    } else {
+        retval = krb5_set_default_realm(kpropd_context, realm);
+        if (retval) {
+            com_err(progname, retval, _("Unable to set default realm"));
+            exit(1);
+        }
+    }
+
     /* Construct service name from local hostname. */
-    retval = krb5_sname_to_principal(kpropd_context, NULL, KPROP_SERVICE_NAME,
-                                     KRB5_NT_SRV_HST, &server);
+    retval = sn2princ_with_realm(kpropd_context, NULL, KPROP_SERVICE_NAME,
+                                 KRB5_NT_SRV_HST, realm, &server);
     if (retval) {
         com_err(progname, retval,
                 _("while trying to construct my service name"));
         exit(1);
     }
-    if (realm != NULL) {
-        retval = krb5_set_principal_realm(kpropd_context, server, realm);
-        if (retval) {
-            com_err(progname, errno, _("while constructing my service realm"));
-            exit(1);
-        }
-    }
 
     /* Construct the name of the temporary file. */
     if (asprintf(&temp_file_name, "%s.temp", file) < 0) {
@@ -1180,6 +1189,8 @@ parse_args(char **argv)
         exit(1);
     }
 
+    params.realm = realm;
+    params.mask |= KADM5_CONFIG_REALM;
     retval = kadm5_get_config_params(kpropd_context, 1, &params, &params);
     if (retval) {
         com_err(progname, retval, _("while initializing"));


More information about the cvs-krb5 mailing list