From 37fb9b90d4c479863796cf3020ddf68906893397 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Thu, 25 Aug 2016 12:59:46 -0400 Subject: [PATCH] Another set of cifs.upcall cleanup patches Signed-off-by: Jeff Layton --- 0001-aclocal-fix-typo-in-idmap.m4.patch | 2 +- ...-krb5-routines-to-get-default-ccname.patch | 2 +- ...-the-krb5_context-a-static-global-va.patch | 2 +- ...cifs.upcall-remove-KRB5_TC_OPENCLOSE.patch | 42 +++ ...-make-get_tgt_time-take-a-ccache-arg.patch | 77 ++++++ ...p-passing-around-ccache-name-strings.patch | 248 ++++++++++++++++++ cifs-utils.spec | 12 +- 7 files changed, 380 insertions(+), 5 deletions(-) create mode 100644 0004-cifs.upcall-remove-KRB5_TC_OPENCLOSE.patch create mode 100644 0005-cifs.upcall-make-get_tgt_time-take-a-ccache-arg.patch create mode 100644 0006-cifs.upcall-stop-passing-around-ccache-name-strings.patch diff --git a/0001-aclocal-fix-typo-in-idmap.m4.patch b/0001-aclocal-fix-typo-in-idmap.m4.patch index 9f32bc3..053a22a 100644 --- a/0001-aclocal-fix-typo-in-idmap.m4.patch +++ b/0001-aclocal-fix-typo-in-idmap.m4.patch @@ -1,7 +1,7 @@ From bbbf7133aec555c5d27ee3163d6045ecfc4673d9 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Tue, 12 Jul 2016 16:53:25 -0400 -Subject: [cifs-utils PATCH 1/3] aclocal: fix typo in idmap.m4 +Subject: [cifs-utils PATCH 1/6] aclocal: fix typo in idmap.m4 We really don't want to do the same check twice. diff --git a/0002-cifs.upcall-use-krb5-routines-to-get-default-ccname.patch b/0002-cifs.upcall-use-krb5-routines-to-get-default-ccname.patch index a4dc81f..bf2eeb3 100644 --- a/0002-cifs.upcall-use-krb5-routines-to-get-default-ccname.patch +++ b/0002-cifs.upcall-use-krb5-routines-to-get-default-ccname.patch @@ -1,7 +1,7 @@ From 9be6e885c3bd63aa6ae9e6351e1b33a4b15d9183 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Sun, 21 Aug 2016 09:42:59 -0400 -Subject: [cifs-utils PATCH 2/3] cifs.upcall: use krb5 routines to get default +Subject: [cifs-utils PATCH 2/6] cifs.upcall: use krb5 routines to get default ccname Currently we end up groveling around in /tmp, trying to guess what the diff --git a/0003-cifs.upcall-make-the-krb5_context-a-static-global-va.patch b/0003-cifs.upcall-make-the-krb5_context-a-static-global-va.patch index b8086c8..5d2d795 100644 --- a/0003-cifs.upcall-make-the-krb5_context-a-static-global-va.patch +++ b/0003-cifs.upcall-make-the-krb5_context-a-static-global-va.patch @@ -1,7 +1,7 @@ From a3743af0c579cee61b816080de978ae7a7663b05 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Mon, 22 Aug 2016 07:34:21 -0400 -Subject: [cifs-utils PATCH 3/3] cifs.upcall: make the krb5_context a static +Subject: [cifs-utils PATCH 3/6] cifs.upcall: make the krb5_context a static global variable There's no need to keep initing a new context for every function. Just diff --git a/0004-cifs.upcall-remove-KRB5_TC_OPENCLOSE.patch b/0004-cifs.upcall-remove-KRB5_TC_OPENCLOSE.patch new file mode 100644 index 0000000..ecbf252 --- /dev/null +++ b/0004-cifs.upcall-remove-KRB5_TC_OPENCLOSE.patch @@ -0,0 +1,42 @@ +From 3db6b3a814a2908b230fcfbdb82846775e56dd93 Mon Sep 17 00:00:00 2001 +From: Jeff Layton +Date: Wed, 24 Aug 2016 11:39:06 -0400 +Subject: [cifs-utils PATCH 4/6] cifs.upcall: remove KRB5_TC_OPENCLOSE + +The header file says that this is deprecated, and all of the info I've +seen about it mentioned that it was for performance more than +correctness. It dates back to the original code dump from Igor, so I +think we're safe to just drop it at this point. + +Signed-off-by: Jeff Layton +--- + cifs.upcall.c | 6 ------ + 1 file changed, 6 deletions(-) + +diff --git a/cifs.upcall.c b/cifs.upcall.c +index 8448d00f6061..a25833592440 100644 +--- a/cifs.upcall.c ++++ b/cifs.upcall.c +@@ -157,9 +157,6 @@ err_endseq: + err_ccstart: + krb5_free_principal(context, principal); + err_princ: +-#if defined(KRB5_TC_OPENCLOSE) +- krb5_cc_set_flags(context, ccache, KRB5_TC_OPENCLOSE); +-#endif + krb5_cc_close(context, ccache); + err_cache: + return credtime; +@@ -388,9 +385,6 @@ out_free_creds: + out_free_principal: + krb5_free_principal(context, in_creds.client); + out_free_ccache: +-#if defined(KRB5_TC_OPENCLOSE) +- krb5_cc_set_flags(context, ccache, KRB5_TC_OPENCLOSE); +-#endif + krb5_cc_close(context, ccache); + return ret; + } +-- +2.7.4 + diff --git a/0005-cifs.upcall-make-get_tgt_time-take-a-ccache-arg.patch b/0005-cifs.upcall-make-get_tgt_time-take-a-ccache-arg.patch new file mode 100644 index 0000000..3fe8735 --- /dev/null +++ b/0005-cifs.upcall-make-get_tgt_time-take-a-ccache-arg.patch @@ -0,0 +1,77 @@ +From 39dbb7b47bea9d6d7cf93ddd53cda501c3898bd6 Mon Sep 17 00:00:00 2001 +From: Jeff Layton +Date: Wed, 24 Aug 2016 11:41:53 -0400 +Subject: [cifs-utils PATCH 5/6] cifs.upcall: make get_tgt_time take a ccache + arg + +...instead of dealing with the ccname. Push resolution of the cache +into the caller. + +Signed-off-by: Jeff Layton +--- + cifs.upcall.c | 21 ++++++++++----------- + 1 file changed, 10 insertions(+), 11 deletions(-) + +diff --git a/cifs.upcall.c b/cifs.upcall.c +index a25833592440..a20576654a95 100644 +--- a/cifs.upcall.c ++++ b/cifs.upcall.c +@@ -102,20 +102,14 @@ krb5_auth_con_getsendsubkey(krb5_context context, + #endif + + /* does the ccache have a valid TGT? */ +-static time_t get_tgt_time(const char *ccname) ++static time_t get_tgt_time(krb5_ccache ccache) + { +- krb5_ccache ccache; + krb5_cc_cursor cur; + krb5_creds creds; + krb5_principal principal; + time_t credtime = 0; + char *realm = NULL; + +- if (krb5_cc_resolve(context, ccname, &ccache)) { +- syslog(LOG_DEBUG, "%s: unable to resolve krb5 cache", __func__); +- goto err_cache; +- } +- + if (krb5_cc_set_flags(context, ccache, 0)) { + syslog(LOG_DEBUG, "%s: unable to set flags", __func__); + goto err_cache; +@@ -156,8 +150,6 @@ err_endseq: + krb5_cc_end_seq_get(context, ccache, &cur); + err_ccstart: + krb5_free_principal(context, principal); +-err_princ: +- krb5_cc_close(context, ccache); + err_cache: + return credtime; + } +@@ -167,15 +159,22 @@ get_default_cc(void) + { + const char *ccname; + char *rcc = NULL; ++ krb5_ccache ccache; + + ccname = krb5_cc_default_name(context); + if (!ccname) { +- syslog(LOG_DEBUG, "krb5_cc_default returned NULL."); ++ syslog(LOG_DEBUG, "%s: krb5_cc_default returned NULL.", __func__); + return NULL; + } + +- if (get_tgt_time(ccname)) ++ if (krb5_cc_resolve(context, ccname, &ccache)) { ++ syslog(LOG_DEBUG, "%s: unable to resolve krb5 cache", __func__); ++ return NULL; ++ } ++ ++ if (get_tgt_time(ccache)) + rcc = strdup(ccname); ++ krb5_cc_close(context, ccache); + return rcc; + } + +-- +2.7.4 + diff --git a/0006-cifs.upcall-stop-passing-around-ccache-name-strings.patch b/0006-cifs.upcall-stop-passing-around-ccache-name-strings.patch new file mode 100644 index 0000000..73a0c9b --- /dev/null +++ b/0006-cifs.upcall-stop-passing-around-ccache-name-strings.patch @@ -0,0 +1,248 @@ +From 7852becab01989634aacc1fb8ff9581a11a7cdcf Mon Sep 17 00:00:00 2001 +From: Jeff Layton +Date: Wed, 24 Aug 2016 12:56:54 -0400 +Subject: [cifs-utils PATCH 6/6] cifs.upcall: stop passing around ccache name + strings + +Instead, get a ccache handle and pass that around. That way we can keep +the cache open until the program is complete as well. + +Signed-off-by: Jeff Layton +--- + cifs.upcall.c | 98 +++++++++++++++++++++++++---------------------------------- + 1 file changed, 41 insertions(+), 57 deletions(-) + +diff --git a/cifs.upcall.c b/cifs.upcall.c +index a20576654a95..8f146c92b4a5 100644 +--- a/cifs.upcall.c ++++ b/cifs.upcall.c +@@ -117,7 +117,7 @@ static time_t get_tgt_time(krb5_ccache ccache) + + if (krb5_cc_get_principal(context, ccache, &principal)) { + syslog(LOG_DEBUG, "%s: unable to get principal", __func__); +- goto err_princ; ++ goto err_cache; + } + + if (krb5_cc_start_seq_get(context, ccache, &cur)) { +@@ -154,32 +154,27 @@ err_cache: + return credtime; + } + +-static char * ++static krb5_ccache + get_default_cc(void) + { +- const char *ccname; +- char *rcc = NULL; +- krb5_ccache ccache; ++ krb5_error_code ret; ++ krb5_ccache cc; + +- ccname = krb5_cc_default_name(context); +- if (!ccname) { +- syslog(LOG_DEBUG, "%s: krb5_cc_default returned NULL.", __func__); ++ ret = krb5_cc_default(context, &cc); ++ if (ret) { ++ syslog(LOG_DEBUG, "%s: krb5_cc_default returned %d", __func__, ret); + return NULL; + } + +- if (krb5_cc_resolve(context, ccname, &ccache)) { +- syslog(LOG_DEBUG, "%s: unable to resolve krb5 cache", __func__); +- return NULL; ++ if (!get_tgt_time(cc)) { ++ krb5_cc_close(context, cc); ++ cc = NULL; + } +- +- if (get_tgt_time(ccache)) +- rcc = strdup(ccname); +- krb5_cc_close(context, ccache); +- return rcc; ++ return cc; + } + + +-static char * ++static krb5_ccache + init_cc_from_keytab(const char *keytab_name, const char *user) + { + krb5_error_code ret; +@@ -187,7 +182,6 @@ init_cc_from_keytab(const char *keytab_name, const char *user) + krb5_keytab keytab = NULL; + krb5_principal me = NULL; + krb5_ccache cc = NULL; +- char *ccname = NULL; + + memset((char *) &my_creds, 0, sizeof(my_creds)); + +@@ -229,61 +223,46 @@ init_cc_from_keytab(const char *keytab_name, const char *user) + } + + ret = krb5_cc_store_cred(context, cc, &my_creds); +- if (ret) ++ if (ret) { + syslog(LOG_DEBUG, "krb5_cc_store_cred: %d", (int)ret); +- +- ccname = strdup(krb5_cc_default_name(context)); +- if (ccname == NULL) +- syslog(LOG_ERR, "Unable to allocate memory"); +-icfk_cleanup: ++ goto icfk_cleanup; ++ } ++out: + my_creds.client = (krb5_principal)0; + krb5_free_cred_contents(context, &my_creds); + + if (me) + krb5_free_principal(context, me); +- if (cc) +- krb5_cc_close(context, cc); + if (keytab) + krb5_kt_close(context, keytab); +- return ccname; ++ return cc; ++icfk_cleanup: ++ if (cc) { ++ krb5_cc_close(context, cc); ++ cc = NULL; ++ } ++ goto out; + } + + static int +-cifs_krb5_get_req(const char *host, const char *ccname, ++cifs_krb5_get_req(const char *host, krb5_ccache ccache, + DATA_BLOB * mechtoken, DATA_BLOB * sess_key) + { + krb5_error_code ret; + krb5_keyblock *tokb; +- krb5_ccache ccache; + krb5_creds in_creds, *out_creds; + krb5_data apreq_pkt, in_data; + krb5_auth_context auth_context = NULL; + #if defined(HAVE_KRB5_AUTH_CON_SETADDRS) && defined(HAVE_KRB5_AUTH_CON_SET_REQ_CKSUMTYPE) + static const uint8_t gss_cksum[24] = { 0x10, 0x00, /* ... */}; + #endif +- if (ccname) { +- ret = krb5_cc_resolve(context, ccname, &ccache); +- if (ret) { +- syslog(LOG_DEBUG, "%s: unable to resolve %s to ccache\n", +- __func__, ccname); +- return ret; +- } +- } else { +- ret = krb5_cc_default(context, &ccache); +- if (ret) { +- syslog(LOG_DEBUG, "%s: krb5_cc_default: %d", +- __func__, (int)ret); +- return ret; +- } +- } +- + memset(&in_creds, 0, sizeof(in_creds)); + + ret = krb5_cc_get_principal(context, ccache, &in_creds.client); + if (ret) { + syslog(LOG_DEBUG, "%s: unable to get client principal name", + __func__); +- goto out_free_ccache; ++ return ret; + } + + ret = krb5_sname_to_principal(context, host, "cifs", KRB5_NT_UNKNOWN, +@@ -383,8 +362,6 @@ out_free_creds: + krb5_free_creds(context, out_creds); + out_free_principal: + krb5_free_principal(context, in_creds.client); +-out_free_ccache: +- krb5_cc_close(context, ccache); + return ret; + } + +@@ -410,7 +387,7 @@ out_free_ccache: + */ + static int + handle_krb5_mech(const char *oid, const char *host, DATA_BLOB * secblob, +- DATA_BLOB * sess_key, const char *ccname) ++ DATA_BLOB * sess_key, krb5_ccache ccache) + { + int retval; + DATA_BLOB tkt, tkt_wrapped; +@@ -418,7 +395,7 @@ handle_krb5_mech(const char *oid, const char *host, DATA_BLOB * secblob, + syslog(LOG_DEBUG, "%s: getting service ticket for %s", __func__, host); + + /* get a kerberos ticket for the service and extract the session key */ +- retval = cifs_krb5_get_req(host, ccname, &tkt, sess_key); ++ retval = cifs_krb5_get_req(host, ccache, &tkt, sess_key); + if (retval) { + syslog(LOG_DEBUG, "%s: failed to obtain service ticket (%d)", + __func__, retval); +@@ -709,12 +686,13 @@ int main(const int argc, char *const argv[]) + unsigned int have; + long rc = 1; + int c, try_dns = 0, legacy_uid = 0; +- char *buf, *ccname = NULL; ++ char *buf; + char hostbuf[NI_MAXHOST], *host; + struct decoded_args arg; + const char *oid; + uid_t uid; + char *keytab_name = NULL; ++ krb5_ccache ccache = NULL; + + hostbuf[0] = '\0'; + memset(&arg, 0, sizeof(arg)); +@@ -828,10 +806,15 @@ int main(const int argc, char *const argv[]) + goto out; + } + +- ccname = get_default_cc(); ++ ccache = get_default_cc(); + /* Couldn't find credcache? Try to use keytab */ +- if (ccname == NULL && arg.username != NULL) +- ccname = init_cc_from_keytab(keytab_name, arg.username); ++ if (ccache == NULL && arg.username != NULL) ++ ccache = init_cc_from_keytab(keytab_name, arg.username); ++ ++ if (ccache == NULL) { ++ rc = 1; ++ goto out; ++ } + + host = arg.hostname; + +@@ -859,7 +842,7 @@ int main(const int argc, char *const argv[]) + + retry_new_hostname: + lowercase_string(host); +- rc = handle_krb5_mech(oid, host, &secblob, &sess_key, ccname); ++ rc = handle_krb5_mech(oid, host, &secblob, &sess_key, ccache); + if (!rc) + break; + +@@ -904,7 +887,7 @@ retry_new_hostname: + break; + } + +- rc = handle_krb5_mech(oid, fqdn, &secblob, &sess_key, ccname); ++ rc = handle_krb5_mech(oid, fqdn, &secblob, &sess_key, ccache); + if (!rc) + break; + } +@@ -968,9 +951,10 @@ out: + } + data_blob_free(&secblob); + data_blob_free(&sess_key); ++ if (ccache) ++ krb5_cc_close(context, ccache); + if (context) + krb5_free_context(context); +- SAFE_FREE(ccname); + SAFE_FREE(arg.hostname); + SAFE_FREE(arg.ip); + SAFE_FREE(arg.username); +-- +2.7.4 + diff --git a/cifs-utils.spec b/cifs-utils.spec index d41be70..50a82cf 100644 --- a/cifs-utils.spec +++ b/cifs-utils.spec @@ -3,7 +3,7 @@ Name: cifs-utils Version: 6.5 -Release: 2%{pre_release}%{?dist} +Release: 3%{pre_release}%{?dist} Summary: Utilities for mounting and managing CIFS mounts Group: System Environment/Daemons @@ -21,7 +21,9 @@ Source0: https://download.samba.org/pub/linux-cifs/cifs-utils/%{name}-%{ Patch1: 0001-aclocal-fix-typo-in-idmap.m4.patch Patch2: 0002-cifs.upcall-use-krb5-routines-to-get-default-ccname.patch Patch3: 0003-cifs.upcall-make-the-krb5_context-a-static-global-va.patch - +Patch4: 0004-cifs.upcall-remove-KRB5_TC_OPENCLOSE.patch +Patch5: 0005-cifs.upcall-make-get_tgt_time-take-a-ccache-arg.patch +Patch6: 0006-cifs.upcall-stop-passing-around-ccache-name-strings.patch %description The SMB/CIFS protocol is a standard file sharing protocol widely deployed @@ -59,6 +61,9 @@ provide these credentials to the kernel automatically at login. %patch1 -p1 %patch2 -p1 %patch3 -p1 +%patch4 -p1 +%patch5 -p1 +%patch6 -p1 %build autoreconf -i @@ -115,6 +120,9 @@ fi %{_mandir}/man8/pam_cifscreds.8.gz %changelog +* Wed Aug 24 2016 Jeff Layton - 6.5-3 +- more cifs.upcall cleanup work + * Wed Aug 24 2016 Jeff Layton - 6.5-2 - clean up and streamline cifs.upcall handling for GSSAPI