gse_krb5: Avoid a segfault when we can not read the dedicated keytab file

This improved code simply cleans up the memory as soon as possible,
rather than using memcmp().  Otherwise, we segfault if
krb5_kt_start_seq_get fails, as it can set the fd element in the
handle to -1.

Change-Id: Ib4821ef944a7e12cd8a891ae07dbfc0567c65495
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Pair-programmed-with: Garming Sam <garming@catalyst.net.nz>
Signed-off-by: Garming Sam <garming@catalyst.net.nz>
Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
Autobuild-Date(master): Thu Nov 27 07:38:02 CET 2014 on sn-devel-104

diff --git a/source3/librpc/crypto/gse_krb5.c b/source3/librpc/crypto/gse_krb5.c
index 43f545ad4cd221dce4dff99ea6bf83e30aa7cbbc..3597329a613f126abc0241b6cee8eaff5296120a 100644 (file)
--- a/source3/librpc/crypto/gse_krb5.c
+++ b/source3/librpc/crypto/gse_krb5.c
@@ -494,15 +494,12 @@ static krb5_error_code fill_mem_keytab_from_dedicated_keytab(krb5_context krbctx
        krb5_kt_cursor kt_cursor;
        krb5_keytab_entry kt_entry;
 
-       ZERO_STRUCT(kt_entry);
-       ZERO_STRUCT(kt_cursor);
-
        ret = smb_krb5_open_keytab(krbctx, lp_dedicated_keytab_file(),
                                   false, &keytab);
        if (ret) {
                DEBUG(1, (__location__ ": smb_krb5_open_keytab failed (%s)\n",
                          error_message(ret)));
-               goto out;
+               return ret;
        }
 
        /*
@@ -522,43 +519,21 @@ static krb5_error_code fill_mem_keytab_from_dedicated_keytab(krb5_context krbctx
                                   &kt_entry, &kt_cursor) == 0)) {
 
                ret = krb5_kt_add_entry(krbctx, *mkeytab, &kt_entry);
+
+               /* Free the entry we just read. */
+               smb_krb5_kt_free_entry(krbctx, &kt_entry);
+
                if (ret) {
                        DEBUG(1, (__location__ ": smb_krb5_unparse_name "
                                  "failed (%s)\n", error_message(ret)));
-                       goto out;
+                       break;
                }
-
-               /* Free the entry we just read. */
-               smb_krb5_kt_free_entry(krbctx, &kt_entry);
-               ZERO_STRUCT(kt_entry);
        }
        krb5_kt_end_seq_get(krbctx, keytab, &kt_cursor);
 
-       ZERO_STRUCT(kt_cursor);
-
 out:
-
-       {
-               krb5_keytab_entry zero_kt_entry;
-               ZERO_STRUCT(zero_kt_entry);
-               if (memcmp(&zero_kt_entry, &kt_entry,
-                          sizeof(krb5_keytab_entry))) {
-                       smb_krb5_kt_free_entry(krbctx, &kt_entry);
-               }
-       }
-
-       {
-               krb5_kt_cursor zero_csr;
-               ZERO_STRUCT(zero_csr);
-               if ((memcmp(&kt_cursor, &zero_csr,
-                           sizeof(krb5_kt_cursor)) != 0) && keytab) {
-                       krb5_kt_end_seq_get(krbctx, keytab, &kt_cursor);
-               }
-       }
-
-       if (keytab) {
-               krb5_kt_close(krbctx, keytab);
-       }
+       
+       krb5_kt_close(krbctx, keytab);
 
        return ret;
 }

diff --git a/source3/script/tests/test_ntlm_auth_krb5.sh b/source3/script/tests/test_ntlm_auth_krb5.sh
index 5989d01046a0b8bdff871aa675c7901180b50925..773cb575811dbd3db7d7bfe9034e706b8ea0d619 100755 (executable)
--- a/source3/script/tests/test_ntlm_auth_krb5.sh
+++ b/source3/script/tests/test_ntlm_auth_krb5.sh
@@ -27,5 +27,6 @@ export KRB5CCNAME
 
 testit "ntlm_auth with krb5 gss-spnego-client and gss-spnego server" $PYTHON $SRC3DIR/torture/test_ntlm_auth.py $NTLM_AUTH $ADDARGS --target-hostname=$SERVER --target-service=host --client-helper=gss-spnego-client --server-helper=gss-spnego --server-use-winbindd || failed=`expr $failed + 1`
 
+echo YR| testit "ntlm_auth with krb5 and an invalid keytab" $NTLM_AUTH --helper-protocol=gss-spnego --option=security=ads --option=kerberosmethod='dedicatedkeytab' --option=dedicatedkeytabfile=FILE:`pwd`/$CCACHE.keytab-does-not-exist || failed=`expr $failed + 1`
 
 testok $0 $failed