Fix minor logic errors

In k5_externalize_auth_context(), serialize the correct field when
remote_port is set.  This is not a reachable bug because the function
is only accessible via gss_export_sec_context(), and the GSS library
does not set a remote port.

In generic_gss_oid_to_str(), remove an inconsistent tests for a null
minor_status.  (XXX consider removing more checks in this file, and
adding validation to g_oid_ops.c functions.)

In the checksum functions, avoid crashing if the caller passes a null
key and checksum type 0.  An error will be returned instead when
find_cksumtype() can't find the checksum type.
(krb5_k_verify_checksum() already had this check.)

In pkinit_open_session(), remove an unnecessary null check for
ctx->p11_module_name, and add a check for p11name being null due to an
asprintf() failure.

In profile_add_node(), add a check for null ret_node in the duplicate
subsection check.  This is not a reachable bug becaues the function is
currently never called with null ret_node and null value.

In ksu's main(), check for krb5_cc_default_name() returning NULL
(which only happens on allocation failure).  Also clean up some
vestiges left behind by commit
9ebae7c.

In the KDC and kpropd write_pid_file(), avoid briefly leaking the file
handle on write failure.

In get_authorized_princ_names(), close login_fp if we fail to open
k5users_path.

Reported by Valery Fedorenko.

src/clients/ksu/heuristic.c

@@ -227,8 +227,10 @@ get_authorized_princ_names(const char *luser, char *cmd, char ***princ_list)
         }
     }
     if (!k5users_flag){
-        if ((users_fp = fopen(k5users_path, "r")) == NULL)
+        if ((users_fp = fopen(k5users_path, "r")) == NULL) {
+            close_time(1, NULL, k5login_flag,login_fp);
             return 0;
+        }
 
         if ( fowner(users_fp, pwd->pw_uid) == FALSE){
             close_time(k5users_flag,users_fp, k5login_flag,login_fp);

src/clients/ksu/main.c

@@ -101,7 +101,6 @@ main(int argc, char ** argv)
 
     krb5_ccache cc_source = NULL;
     const char * cc_source_tag = NULL;
-    const char * cc_source_tag_tmp = NULL;
     char * cmd = NULL, * exec_cmd = NULL;
     int errflg = 0;
     krb5_boolean auth_val;
@@ -274,23 +273,13 @@ main(int argc, char ** argv)
         case 'c':
             if (cc_source_tag == NULL) {
                 cc_source_tag = xstrdup(optarg);
-                if ( strchr(cc_source_tag, ':')){
-                    cc_source_tag_tmp = strchr(cc_source_tag, ':') + 1;
-
-                    if (!ks_ccache_name_is_initialized(ksu_context,
-                                                       cc_source_tag)) {
-                        com_err(prog_name, errno,
-                                _("while looking for credentials cache %s"),
-                                cc_source_tag_tmp);
-                        exit (1);
-                    }
-                }
-                else {
-                    fprintf(stderr, _("malformed credential cache name %s\n"),
+                if (!ks_ccache_name_is_initialized(ksu_context,
+                                                   cc_source_tag)) {
+                    com_err(prog_name, errno,
+                            _("while looking for credentials cache %s"),
                             cc_source_tag);
-                    errflg++;
+                    exit(1);
                 }
-
             } else {
                 fprintf(stderr, _("Only one -c option allowed\n"));
                 errflg++;
@@ -374,11 +363,10 @@ main(int argc, char ** argv)
 
     if (cc_source_tag == NULL){
         cc_source_tag = krb5_cc_default_name(ksu_context);
-        cc_source_tag_tmp = strchr(cc_source_tag, ':');
-        if (cc_source_tag_tmp == 0)
-            cc_source_tag_tmp = cc_source_tag;
-        else
-            cc_source_tag_tmp++;
+        if (cc_source_tag == NULL) {
+            fprintf(stderr, _("ksu: failed to get default ccache name\n"));
+            exit(1);
+        }
     }
 
     /* get a handle for the cache */

src/kadmin/server/ovsec_kadmd.c

@@ -235,7 +235,7 @@ log_badverf(gss_name_t client_name, gss_name_t server_name,
     OM_uint32 minor;
     gss_buffer_desc client, server;
     gss_OID gss_type;
-    const char *a;
+    const char *a, *cname, *sname;
     rpcproc_t proc;
     unsigned int i;
     const char *procname;
@@ -249,19 +249,11 @@ log_badverf(gss_name_t client_name, gss_name_t server_name,
 
     (void)gss_display_name(&minor, client_name, &client, &gss_type);
     (void)gss_display_name(&minor, server_name, &server, &gss_type);
-    if (client.value == NULL) {
-        client.value = "(null)";
-        clen = sizeof("(null)") - 1;
-    } else {
-        clen = client.length;
-    }
+    cname = (client.value == NULL) ? "(null)" : client.value;
+    clen = (client.value == NULL) ? sizeof("(null)") - 1 : client.length;
     trunc_name(&clen, &cdots);
-    if (server.value == NULL) {
-        server.value = "(null)";
-        slen = sizeof("(null)") - 1;
-    } else {
-        slen = server.length;
-    }
+    sname = (server.value == NULL) ? "(null)" : server.value;
+    slen = (server.value == NULL) ? sizeof("(null)") - 1 : server.length;
     trunc_name(&slen, &sdots);
     a = client_addr(rqst->rq_xprt);
 
@@ -277,14 +269,14 @@ log_badverf(gss_name_t client_name, gss_name_t server_name,
         krb5_klog_syslog(LOG_NOTICE,
                          _("WARNING! Forged/garbled request: %s, claimed "
                            "client = %.*s%s, server = %.*s%s, addr = %s"),
-                         procname, (int)clen, (char *)client.value, cdots,
-                         (int)slen, (char *)server.value, sdots, a);
+                         procname, (int)clen, cname, cdots, (int)slen, sname,
+                         sdots, a);
     } else {
         krb5_klog_syslog(LOG_NOTICE,
                          _("WARNING! Forged/garbled request: %d, claimed "
                            "client = %.*s%s, server = %.*s%s, addr = %s"),
-                         proc, (int)clen, (char *)client.value, cdots,
-                         (int)slen, (char *)server.value, sdots, a);
+                         proc, (int)clen, cname, cdots, (int)slen, sname,
+                         sdots, a);
     }
 
     (void)gss_release_buffer(&minor, &client);

src/kdc/main.c

@@ -842,14 +842,15 @@ write_pid_file(const char *path)
 {
     FILE *file;
     unsigned long pid;
+    int st1, st2;
 
     file = fopen(path, "w");
     if (file == NULL)
         return errno;
-    pid = (unsigned long) getpid();
-    if (fprintf(file, "%ld\n", pid) < 0 || fclose(file) == EOF)
-        return errno;
-    return 0;
+    pid = (unsigned long)getpid();
+    st1 = (fprintf(file, "%ld\n", pid) < 0) ? errno : 0;
+    st2 = (fclose(file) == EOF) ? errno : 0;
+    return st1 ? st1 : st2;
 }
 
 static void

src/kprop/kpropd.c

@@ -181,14 +181,15 @@ write_pid_file(const char *path)
 {
     FILE *fp;
     unsigned long pid;
+    int st1, st2;
 
     fp = fopen(path, "w");
     if (fp == NULL)
         return errno;
     pid = (unsigned long)getpid();
-    if (fprintf(fp, "%ld\n", pid) < 0 || fclose(fp) == EOF)
-        return errno;
-    return 0;
+    st1 = (fprintf(fp, "%ld\n", pid) < 0) ? errno : 0;
+    st2 = (fclose(fp) == EOF) ? errno : 0;
+    return st1 ? st1 : st2;
 }
 
 typedef void (*sig_handler_fn)(int sig);

src/lib/crypto/krb/make_checksum.c

@@ -40,7 +40,7 @@ krb5_k_make_checksum(krb5_context context, krb5_cksumtype cksumtype,
     krb5_octet *trunc;
     krb5_error_code ret;
 
-    if (cksumtype == 0) {
+    if (cksumtype == 0 && key != NULL) {
         ret = krb5int_c_mandatory_cksumtype(context, key->keyblock.enctype,
                                             &cksumtype);
         if (ret != 0)

src/lib/crypto/krb/make_checksum_iov.c

@@ -39,7 +39,7 @@ krb5_k_make_checksum_iov(krb5_context context,
     krb5_crypto_iov *checksum;
     const struct krb5_cksumtypes *ctp;
 
-    if (cksumtype == 0) {
+    if (cksumtype == 0 && key != NULL) {
         ret = krb5int_c_mandatory_cksumtype(context, key->keyblock.enctype,
                                             &cksumtype);
         if (ret != 0)

src/lib/crypto/krb/verify_checksum_iov.c

@@ -40,7 +40,7 @@ krb5_k_verify_checksum_iov(krb5_context context,
     krb5_data computed;
     krb5_crypto_iov *checksum;
 
-    if (checksum_type == 0) {
+    if (checksum_type == 0 && key != NULL) {
         ret = krb5int_c_mandatory_cksumtype(context, key->keyblock.enctype,
                                             &checksum_type);
         if (ret != 0)

src/lib/gssapi/generic/oid_ops.c

@@ -245,8 +245,7 @@ generic_gss_oid_to_str(OM_uint32 *minor_status,
     unsigned char       *cp;
     struct k5buf        buf;
 
-    if (minor_status != NULL)
-        *minor_status = 0;
+    *minor_status = 0;
 
     if (oid_str != GSS_C_NO_BUFFER) {
         oid_str->length = 0;

src/lib/krb5/krb/ser_actx.c

@@ -171,7 +171,7 @@ k5_externalize_auth_context(krb5_auth_context auth_context,
             /* Now handle remote_port, if appropriate */
             if (!kret && auth_context->remote_port) {
                 (void) krb5_ser_pack_int32(TOKEN_RPORT, &bp, &remain);
-                kret = k5_externalize_address(auth_context->remote_addr,
+                kret = k5_externalize_address(auth_context->remote_port,
                                               &bp, &remain);
             }
 

src/plugins/preauth/pkinit/pkinit_crypto_openssl.c

@@ -3602,20 +3602,22 @@ pkinit_open_session(krb5_context context,
 
     /* Login if needed */
     if (tinfo.flags & CKF_LOGIN_REQUIRED) {
-        if (cctx->p11_module_name != NULL) {
-            if (cctx->slotid != PK_NOSLOT) {
-                if (asprintf(&p11name,
-                             "PKCS11:module_name=%s:slotid=%ld:token=%.*s",
-                             cctx->p11_module_name, (long)cctx->slotid,
-                             (int)label_len, tinfo.label) < 0)
-                    p11name = NULL;
-            } else {
-                if (asprintf(&p11name,
-                             "PKCS11:module_name=%s,token=%.*s",
-                             cctx->p11_module_name,
-                             (int)label_len, tinfo.label) < 0)
-                    p11name = NULL;
-            }
+        if (cctx->slotid != PK_NOSLOT) {
+            if (asprintf(&p11name,
+                         "PKCS11:module_name=%s:slotid=%ld:token=%.*s",
+                         cctx->p11_module_name, (long)cctx->slotid,
+                         (int)label_len, tinfo.label) < 0)
+                p11name = NULL;
+        } else {
+            if (asprintf(&p11name,
+                         "PKCS11:module_name=%s,token=%.*s",
+                         cctx->p11_module_name,
+                         (int)label_len, tinfo.label) < 0)
+                p11name = NULL;
+        }
+        if (p11name == NULL) {
+            ret = ENOMEM;
+            goto cleanup;
         }
         if (cctx->defer_id_prompt) {
             /* Supply the identity name to be passed to the responder. */

src/util/profile/prof_tree.c

@@ -219,7 +219,8 @@ errcode_t profile_add_node(struct profile_node *section, const char *name,
         } else if (value == NULL && cmp == 0 &&
                    p->value == NULL && p->deleted != 1) {
             /* Found duplicate subsection, so don't make a new one. */
-            *ret_node = p;
+            if (ret_node)
+                *ret_node = p;
             return 0;
         } else if (check_final && cmp == 0 && p->final) {
             /* This key already exists with the final flag and we were asked