The branch, master has been updated
       via  da99e3a724b493ba47a06d0704b891819ad16647 (commit)
       via  3544e685ade5b331e473c8680d42a748d9389125 (commit)
      from  1a97bd915dfe90b40ec03617af3d8d25483af9c9 (commit)

http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit da99e3a724b493ba47a06d0704b891819ad16647
Author: Jeff Layton <jlay...@redhat.com>
Date:   Wed Aug 26 06:26:02 2009 -0400

    cifs.upcall: make using ip address conditional on new option
    
    Igor Mammedov pointed out that reverse resolving an IP address to get
    the hostname portion of a principal could open a possible attack
    vector. If an attacker were to gain control of DNS, then he could
    redirect the mount to a server of his choosing, and fix the reverse
    resolution to point to a hostname of his choosing (one where he has
    the key for the corresponding cifs/ or host/ principal).
    
    That said, we often trust DNS for other reasons and it can be useful
    to do so. Make the code that allows trusting DNS to be enabled by
    adding --trust-dns to the cifs.upcall invocation.
    
    Signed-off-by: Jeff Layton <jlay...@redhat.com>

commit 3544e685ade5b331e473c8680d42a748d9389125
Author: Jeff Layton <jlay...@redhat.com>
Date:   Wed Aug 26 06:15:42 2009 -0400

    cifs.upcall: switch to getopt_long
    
    ...to allow long option names.
    
    Signed-off-by: Jeff Layton <jlay...@redhat.com>

-----------------------------------------------------------------------

Summary of changes:
 client/cifs.upcall.c                  |   68 ++++++++++++++++++++++-----------
 docs-xml/manpages-3/cifs.upcall.8.xml |   15 +++++--
 2 files changed, 56 insertions(+), 27 deletions(-)


Changeset truncated at 500 lines:

diff --git a/client/cifs.upcall.c b/client/cifs.upcall.c
index c89df9c..1645322 100644
--- a/client/cifs.upcall.c
+++ b/client/cifs.upcall.c
@@ -27,6 +27,7 @@ create dns_resolver * * /usr/local/sbin/cifs.upcall %k
 
 #include "includes.h"
 #include <keyutils.h>
+#include <getopt.h>
 
 #include "cifs_spnego.h"
 
@@ -153,9 +154,9 @@ handle_krb5_mech(const char *oid, const char *principal, 
DATA_BLOB *secblob,
 #define DKD_HAVE_IP            0x8
 #define DKD_HAVE_UID           0x10
 #define DKD_HAVE_PID           0x20
-#define DKD_MUSTHAVE_SET (DKD_HAVE_IP|DKD_HAVE_VERSION|DKD_HAVE_SEC)
+#define DKD_MUSTHAVE_SET (DKD_HAVE_HOSTNAME|DKD_HAVE_VERSION|DKD_HAVE_SEC)
 
-static struct decoded_args {
+struct decoded_args {
        int             ver;
        char            *hostname;
        char            *ip;
@@ -353,10 +354,16 @@ ip_to_fqdn(const char *addrstr, char *host, size_t 
hostlen)
 static void
 usage(void)
 {
-       syslog(LOG_INFO, "Usage: %s [-v] key_serial", prog);
-       fprintf(stderr, "Usage: %s [-v] key_serial\n", prog);
+       syslog(LOG_INFO, "Usage: %s [-t] [-v] key_serial", prog);
+       fprintf(stderr, "Usage: %s [-t] [-v] key_serial\n", prog);
 }
 
+const struct option long_options[] = {
+       { "trust-dns",  0, NULL, 't' },
+       { "version",    0, NULL, 'v' },
+       { NULL,         0, NULL, 0 }
+};
+
 int main(const int argc, char *const argv[])
 {
        struct cifs_spnego_msg *keydata = NULL;
@@ -366,19 +373,24 @@ int main(const int argc, char *const argv[])
        size_t datalen;
        unsigned int have;
        long rc = 1;
-       int c;
-       char *buf, *princ, *ccname = NULL;
-       char hostbuf[NI_MAXHOST];
+       int c, try_dns = 0;
+       char *buf, *princ = NULL, *ccname = NULL;
+       char hostbuf[NI_MAXHOST], *host;
        struct decoded_args arg = { };
        const char *oid;
 
+       hostbuf[0] = '\0';
+
        openlog(prog, 0, LOG_DAEMON);
 
-       while ((c = getopt(argc, argv, "cv")) != -1) {
+       while ((c = getopt_long(argc, argv, "ctv", long_options, NULL)) != -1) {
                switch (c) {
                case 'c':
                        /* legacy option -- skip it */
                        break;
+               case 't':
+                       try_dns++;
+                       break;
                case 'v':
                        printf("version: %s\n", CIFSSPNEGO_VERSION);
                        goto out;
@@ -446,21 +458,18 @@ int main(const int argc, char *const argv[])
        if (have & DKD_HAVE_PID)
                ccname = get_krb5_ccname(arg.pid);
 
-       if (have & DKD_HAVE_IP) {
-               rc = ip_to_fqdn(arg.ip, hostbuf, sizeof(hostbuf));
-               if (rc)
-                       goto out;
-       }
+       host = arg.hostname;
 
        // do mech specific authorization
        switch (arg.sec) {
        case MS_KRB5:
        case KRB5:
+retry_new_hostname:
                /* for "cifs/" service name + terminating 0 */
-               datalen = strnlen(hostbuf, sizeof(hostbuf)) + 5 + 1;
+               datalen = strlen(host) + 5 + 1;
                princ = SMB_XMALLOC_ARRAY(char, datalen);
                if (!princ) {
-                       rc = 1;
+                       rc = -ENOMEM;
                        break;
                }
 
@@ -474,21 +483,35 @@ int main(const int argc, char *const argv[])
                 * getting a host/ principal if that doesn't work.
                 */
                strlcpy(princ, "cifs/", datalen);
-               strlcpy(princ + 5, hostbuf, datalen - 5);
+               strlcpy(princ + 5, host, datalen - 5);
                rc = handle_krb5_mech(oid, princ, &secblob, &sess_key, ccname);
-               if (rc) {
-                       memcpy(princ, "host/", 5);
-                       rc = handle_krb5_mech(oid, princ, &secblob, &sess_key,
-                                               ccname);
-               }
+               if (!rc)
+                       break;
+
+               memcpy(princ, "host/", 5);
+               rc = handle_krb5_mech(oid, princ, &secblob, &sess_key, ccname);
+               if (!rc)
+                       break;
+
+               if (!try_dns || !(have & DKD_HAVE_IP))
+                       break;
+
+               rc = ip_to_fqdn(arg.ip, hostbuf, sizeof(hostbuf));
+               if (rc)
+                       break;
+
                SAFE_FREE(princ);
-               break;
+               try_dns = 0;
+               host = hostbuf;
+               goto retry_new_hostname;
        default:
                syslog(LOG_ERR, "sectype: %d is not implemented", arg.sec);
                rc = 1;
                break;
        }
 
+       SAFE_FREE(princ);
+
        if (rc)
                goto out;
 
@@ -530,6 +553,7 @@ out:
        data_blob_free(&sess_key);
        SAFE_FREE(ccname);
        SAFE_FREE(arg.hostname);
+       SAFE_FREE(arg.ip);
        SAFE_FREE(keydata);
        return rc;
 }
diff --git a/docs-xml/manpages-3/cifs.upcall.8.xml 
b/docs-xml/manpages-3/cifs.upcall.8.xml
index 427bb44..05a5e0a 100644
--- a/docs-xml/manpages-3/cifs.upcall.8.xml
+++ b/docs-xml/manpages-3/cifs.upcall.8.xml
@@ -19,8 +19,8 @@
 <refsynopsisdiv>
         <cmdsynopsis>
                 <command>cifs.upcall</command>
-                <arg choice="opt">-c</arg>
-                <arg choice="opt">-v</arg>
+                <arg choice="opt">--trust-dns|-t</arg>
+                <arg choice="opt">--version|-v</arg>
                 <arg choice="req">keyid</arg>
         </cmdsynopsis>
 </refsynopsisdiv>
@@ -51,9 +51,14 @@ to be run that way.</para>
                <listitem><para>This option is deprecated and is currently 
ignored.
                </para></listitem>
                </varlistentry>
-
                <varlistentry>
-               <term>-v</term>
+               <term>--trust-dns|-t</term>
+               <listitem><para>With krb5 upcalls, the name used as the host 
portion of the service principal defaults to the hostname portion of the UNC. 
This option allows the upcall program to reverse resolve the network address of 
the server in order to get the hostname.</para>
+               <para>This is less secure than not trusting DNS. When using 
this option, it's possible that an attacker could get control of DNS and trick 
the client into mounting a different server altogether. It's preferable to 
instead add server principals to the KDC for every possible hostname, but this 
option exists for cases where that isn't possible. The default is to not trust 
reverse hostname lookups in this fashion.
+               </para></listitem>
+               </varlistentry>
+               <varlistentry>
+               <term>--version|-v</term>
                <listitem><para>Print version number and exit.
                </para></listitem>
                </varlistentry>
@@ -85,7 +90,7 @@ to be run that way.</para>
        <para>To make this program useful for CIFS, you'll need to set up 
entries for them in request-key.conf<manvolnum>5</manvolnum>. Here's an example 
of an entry for each key type:</para>
 <programlisting>
 #OPERATION  TYPE           D C PROGRAM ARG1 ARG2...
-#=========  =============  = = ==========================================
+#=========  =============  = = ================================
 create      cifs.spnego    * * /usr/local/sbin/cifs.upcall %k
 create      dns_resolver   * * /usr/local/sbin/cifs.upcall %k
 </programlisting>


-- 
Samba Shared Repository

Reply via email to