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 inconsistently-applied test for
a null minor_status.  Also remove minor_status null checks from
generic_gss_release_oid() and generic_gss_str_to_oid(), but add output
initializations and pointer checks to the API functions in g_oid_ops.c
in a similar manner to other GSSAPI functions.  Remove
gssint_copy_oid_set() and replace its one call with a call to
generic_gss_copy_oid_set().

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 because 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
9ebae7cb434b9b177c0af85c67a6d6267f46bc68.

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

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

Reported by Valery Fedorenko.

diff --git a/src/clients/ksu/heuristic.c b/src/clients/ksu/heuristic.c
index 6ed94eb887cd7893322f3087a469a2e5a0f96ea0..fcc1ddf8a76e7ee2298f47462eb8208c9507f8b2 100644 (file)
--- a/src/clients/ksu/heuristic.c
+++ b/src/clients/ksu/heuristic.c
@@ -227,8 +227,11 @@ get_authorized_princ_names(const char *luser, char *cmd, char ***princ_list)
         }
     }
     if (!k5users_flag){
-        if ((users_fp = fopen(k5users_path, "r")) == NULL)
+        users_fp = fopen(k5users_path, "r");
+        if (users_fp == 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);

diff --git a/src/clients/ksu/main.c b/src/clients/ksu/main.c
index debad94861a121d58dff07fdc1fccd7c5ba61ca2..ca3981ea753f95c5cfb5ea1e4f954bbc83b21d74 100644 (file)
--- a/src/clients/ksu/main.c
+++ b/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 */

diff --git a/src/kadmin/server/ovsec_kadmd.c b/src/kadmin/server/ovsec_kadmd.c
index 5450bae80b8c178d56cabf4ce3ebd54e72694f97..6e080e197d5e76eeab7463da00b90f4d69f69c8c 100644 (file)
--- a/src/kadmin/server/ovsec_kadmd.c
+++ b/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);

diff --git a/src/kdc/main.c b/src/kdc/main.c
index 3698a4b0da813f305753e985c81150b4f5dd332c..bef8b3ee2c7ad88febcbca398af7036aa0bc6ab3 100644 (file)
--- a/src/kdc/main.c
+++ b/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

diff --git a/src/kprop/kpropd.c b/src/kprop/kpropd.c
index 64afd3946bbcc5e5227ccfde7c0a90ef0824ddf4..be48062ff573891488b83dd8b421fc728bbd896d 100644 (file)
--- a/src/kprop/kpropd.c
+++ b/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);

diff --git a/src/lib/crypto/krb/make_checksum.c b/src/lib/crypto/krb/make_checksum.c
index 398c84a8dbb1acca1bf2a51c6c2ab2057c7c9f1e..3c57e4173b7f887823857bb02d936ca4cb1df5c1 100644 (file)
--- a/src/lib/crypto/krb/make_checksum.c
+++ b/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)

diff --git a/src/lib/crypto/krb/make_checksum_iov.c b/src/lib/crypto/krb/make_checksum_iov.c
index 84e98b141b9974d0eeb248f900cafc8c32e31ed9..c9e9da87f57e293ec6caf5f51a57d3e987d8c4ed 100644 (file)
--- a/src/lib/crypto/krb/make_checksum_iov.c
+++ b/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)

diff --git a/src/lib/crypto/krb/verify_checksum_iov.c b/src/lib/crypto/krb/verify_checksum_iov.c
index 47a25a93b4e8f2aec7b063e90f3e992aabcef385..532e45cd9df764ff85374e5417958c0c57b9b7a7 100644 (file)
--- a/src/lib/crypto/krb/verify_checksum_iov.c
+++ b/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)

diff --git a/src/lib/gssapi/generic/oid_ops.c b/src/lib/gssapi/generic/oid_ops.c
index 253d64694dd4b423077ac350c54e01a5612ff3a0..0d65a95fcf0bdcd890af076e0a7844d8612b6f87 100644 (file)
--- a/src/lib/gssapi/generic/oid_ops.c
+++ b/src/lib/gssapi/generic/oid_ops.c
@@ -68,8 +68,7 @@
 OM_uint32
 generic_gss_release_oid(OM_uint32 *minor_status, gss_OID *oid)
 {
-    if (minor_status)
-        *minor_status = 0;
+    *minor_status = 0;
 
     if (oid == NULL || *oid == GSS_C_NO_OID)
         return(GSS_S_COMPLETE);
@@ -245,8 +244,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;
@@ -353,8 +351,7 @@ generic_gss_str_to_oid(OM_uint32 *minor_status,
     int brace = 0;
     gss_OID oid;
 
-    if (minor_status != NULL)
-        *minor_status = 0;
+    *minor_status = 0;
 
     if (oid_out != NULL)
         *oid_out = GSS_C_NO_OID;

diff --git a/src/lib/gssapi/mechglue/g_oid_ops.c b/src/lib/gssapi/mechglue/g_oid_ops.c
index f29fb3b33e01053c556c2c15f5ae52f1c322192e..fa87d80956594abb2075859f47fa63928f13e373 100644 (file)
--- a/src/lib/gssapi/mechglue/g_oid_ops.c
+++ b/src/lib/gssapi/mechglue/g_oid_ops.c
@@ -36,6 +36,13 @@ OM_uint32 KRB5_CALLCONV
 gss_create_empty_oid_set(OM_uint32 *minor_status, gss_OID_set *oid_set)
 {
     OM_uint32 status;
+
+    if (minor_status != NULL)
+       *minor_status = 0;
+    if (oid_set != NULL)
+       *oid_set = GSS_C_NO_OID_SET;
+    if (minor_status == NULL || oid_set == NULL)
+       return GSS_S_CALL_INACCESSIBLE_WRITE;
     status = generic_gss_create_empty_oid_set(minor_status, oid_set);
     if (status != GSS_S_COMPLETE)
        map_errcode(minor_status);
@@ -47,6 +54,14 @@ gss_add_oid_set_member(OM_uint32 *minor_status, gss_OID member_oid,
                       gss_OID_set *oid_set)
 {
     OM_uint32 status;
+
+    if (minor_status != NULL)
+       *minor_status = 0;
+    if (minor_status == NULL || oid_set == NULL)
+       return GSS_S_CALL_INACCESSIBLE_WRITE;
+    if (member_oid == GSS_C_NO_OID || member_oid->length == 0 ||
+       member_oid->elements == NULL)
+       return GSS_S_CALL_INACCESSIBLE_READ;
     status = generic_gss_add_oid_set_member(minor_status, member_oid, oid_set);
     if (status != GSS_S_COMPLETE)
        map_errcode(minor_status);
@@ -57,13 +72,33 @@ OM_uint32 KRB5_CALLCONV
 gss_test_oid_set_member(OM_uint32 *minor_status, gss_OID member,
                        gss_OID_set set, int *present)
 {
+    if (minor_status != NULL)
+       *minor_status = 0;
+    if (present != NULL)
+       *present = 0;
+    if (minor_status == NULL || present == NULL)
+       return GSS_S_CALL_INACCESSIBLE_WRITE;
+    if (member == GSS_C_NO_OID || set == GSS_C_NO_OID_SET)
+       return GSS_S_CALL_INACCESSIBLE_READ;
     return generic_gss_test_oid_set_member(minor_status, member, set, present);
 }
 
 OM_uint32 KRB5_CALLCONV
 gss_oid_to_str(OM_uint32 *minor_status, gss_OID oid, gss_buffer_t oid_str)
 {
-    OM_uint32 status = generic_gss_oid_to_str(minor_status, oid, oid_str);
+    OM_uint32 status;
+
+    if (minor_status != NULL)
+       *minor_status = 0;
+    if (oid_str != GSS_C_NO_BUFFER) {
+       oid_str->length = 0;
+       oid_str->value = NULL;
+    }
+    if (minor_status == NULL || oid_str == GSS_C_NO_BUFFER)
+       return GSS_S_CALL_INACCESSIBLE_WRITE;
+    if (oid == GSS_C_NO_OID || oid->length == 0 || oid->elements == NULL)
+       return GSS_S_CALL_INACCESSIBLE_READ;
+    status = generic_gss_oid_to_str(minor_status, oid, oid_str);
     if (status != GSS_S_COMPLETE)
        map_errcode(minor_status);
     return status;
@@ -72,21 +107,22 @@ gss_oid_to_str(OM_uint32 *minor_status, gss_OID oid, gss_buffer_t oid_str)
 OM_uint32 KRB5_CALLCONV
 gss_str_to_oid(OM_uint32 *minor_status, gss_buffer_t oid_str, gss_OID *oid)
 {
-    OM_uint32 status = generic_gss_str_to_oid(minor_status, oid_str, oid);
+    OM_uint32 status;
+
+    if (minor_status != NULL)
+       *minor_status = 0;
+    if (oid != NULL)
+       *oid = GSS_C_NO_OID;
+    if (minor_status == NULL || oid == NULL)
+       return GSS_S_CALL_INACCESSIBLE_WRITE;
+    if (GSS_EMPTY_BUFFER(oid_str))
+       return GSS_S_CALL_INACCESSIBLE_READ;
+    status = generic_gss_str_to_oid(minor_status, oid_str, oid);
     if (status != GSS_S_COMPLETE)
        map_errcode(minor_status);
     return status;
 }
 
-OM_uint32
-gssint_copy_oid_set(
-    OM_uint32 *minor_status,
-    const gss_OID_set_desc * const oidset,
-    gss_OID_set *new_oidset)
-{
-    return generic_gss_copy_oid_set(minor_status, oidset, new_oidset);
-}
-
 int KRB5_CALLCONV
 gss_oid_equal(
     gss_const_OID first_oid,

diff --git a/src/lib/gssapi/mechglue/mglueP.h b/src/lib/gssapi/mechglue/mglueP.h
index edd759cb0b01cae6a9b3d17e5ba854300c309597..2fdd4a9ba599dbef46154bf9acb0b66556e455d5 100644 (file)
--- a/src/lib/gssapi/mechglue/mglueP.h
+++ b/src/lib/gssapi/mechglue/mglueP.h
@@ -799,12 +799,6 @@ OM_uint32 gssint_create_union_context(
        gss_union_ctx_id_t *    /* ctx_out */
 );
 
-OM_uint32 gssint_copy_oid_set(
-       OM_uint32 *,                    /* minor_status */
-       const gss_OID_set_desc * const, /* oid set */
-       gss_OID_set *                   /* new oid set */
-);
-
 gss_OID gss_find_mechanism_from_name_type (gss_OID); /* name_type */
 
 OM_uint32 gss_add_mech_name_type

diff --git a/src/lib/gssapi/spnego/spnego_mech.c b/src/lib/gssapi/spnego/spnego_mech.c
index 5923c880b8debe45449b0eecafa71c5bfa913db1..43ba63ab2a7b0954c1b8753350bbc7059d6a1ebd 100644 (file)
--- a/src/lib/gssapi/spnego/spnego_mech.c
+++ b/src/lib/gssapi/spnego/spnego_mech.c
@@ -379,7 +379,7 @@ spnego_gss_acquire_cred_from(OM_uint32 *minor_status,
                                     &amechs, time_rec);
 
        if (actual_mechs && amechs != GSS_C_NULL_OID_SET) {
-               (void) gssint_copy_oid_set(&tmpmin, amechs, actual_mechs);
+               (void) generic_gss_copy_oid_set(&tmpmin, amechs, actual_mechs);
        }
        (void) gss_release_oid_set(&tmpmin, &amechs);
 

diff --git a/src/lib/krb5/krb/ser_actx.c b/src/lib/krb5/krb/ser_actx.c
index 6de35a146ac893b03b6474e4bfed127ee1cfcf91..ed8e25596b5d9f5607c3e337756698b92eb8ff08 100644 (file)
--- a/src/lib/krb5/krb/ser_actx.c
+++ b/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);
             }
 

diff --git a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
index 4ae2c00ad5ecc0bc7c98f88ae9ea84762155bd67..14370ae34be9ff9df02cbd52291d5ff67e770d1a 100644 (file)
--- a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
+++ b/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. */

diff --git a/src/util/profile/prof_tree.c b/src/util/profile/prof_tree.c
index 3e2aaa1cf611560877a6fa359e7e27259aa17c52..b02675741d73c27d83af8c81acd91a2d30695224 100644 (file)
--- a/src/util/profile/prof_tree.c
+++ b/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