* [RFC/PATCH] cifs: add server-provided principal name in upcall @ 2011-09-06 15:21 Martin Wilck [not found] ` <1315322512-10652-1-git-send-email-martin.wilck-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org> 0 siblings, 1 reply; 23+ messages in thread From: Martin Wilck @ 2011-09-06 15:21 UTC (permalink / raw) To: linux-cifs-u79uwXL29TY76Z2rM5mHXA Cc: sfrench-eUNUBHrolfbYtjvyW6yDsg, Martin Wilck The current cifs implementation discards a principal name found in the CIFS server's SecBlob. This patch adds the principal name into the data used for the request_key call. Combined with a separate cifs-utils patch, this enables cifs mounts using kerberos on servers that use different principal names than the default cifs/<hostname> or host/<hostname> which are tried by cifs.upcall. Signed-off-by: Martin Wilck <martin.wilck-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org> --- fs/cifs/asn1.c | 28 ++++++++++++++++++++++++---- fs/cifs/cifs_spnego.c | 10 ++++++++++ fs/cifs/cifs_spnego.h | 2 +- fs/cifs/cifsglob.h | 1 + fs/cifs/connect.c | 3 +++ 5 files changed, 39 insertions(+), 5 deletions(-) diff --git a/fs/cifs/asn1.c b/fs/cifs/asn1.c index cfd1ce3..5ab23f2 100644 --- a/fs/cifs/asn1.c +++ b/fs/cifs/asn1.c @@ -369,7 +369,7 @@ static unsigned char asn1_ulong_decode(struct asn1_ctx *ctx, *integer |= ch; } return 1; -} +} */ static unsigned char asn1_octets_decode(struct asn1_ctx *ctx, @@ -395,7 +395,7 @@ asn1_octets_decode(struct asn1_ctx *ctx, (*len)++; } return 1; -} */ +} static unsigned char asn1_subid_decode(struct asn1_ctx *ctx, unsigned long *subid) @@ -496,9 +496,9 @@ decode_negTokenInit(unsigned char *security_blob, int length, { struct asn1_ctx ctx; unsigned char *end; - unsigned char *sequence_end; + unsigned char *sequence_end, *principal; unsigned long *oid = NULL; - unsigned int cls, con, tag, oidlen, rc; + unsigned int cls, con, tag, oidlen, rc, princlen; /* cifs_dump_mem(" Received SecBlob ", security_blob, length); */ @@ -661,6 +661,26 @@ decode_negTokenInit(unsigned char *security_blob, int length, } cFYI(1, "Need to call asn1_octets_decode() function for %s", ctx.pointer); /* is this UTF-8 or ASCII? */ + if (asn1_octets_decode(&ctx, end, &principal, &princlen) == 0) { + cFYI(1, "Error decoding principal name exit10"); + return 0; + } else if (princlen == 0) { + cFYI(1, "Empty principal name"); + } else if (!strcmp(principal, "not_defined_in_RFC4178@please_ignore")) { + cFYI(1, "Ignoring principal name"); + } else { + server->principal = kmalloc(princlen+1, GFP_ATOMIC); + if (server->principal != NULL) { + memcpy(server->principal, principal, princlen); + server->principal[princlen] = '\0'; + cFYI(1, "Got principal: %s", server->principal); + } else { + kfree(principal); + cFYI(1, "Error allocating memory exit 11"); + return 0; + } + } + kfree(principal); decode_negtoken_exit: return 1; } diff --git a/fs/cifs/cifs_spnego.c b/fs/cifs/cifs_spnego.c index 2272fd5..356a8a6 100644 --- a/fs/cifs/cifs_spnego.c +++ b/fs/cifs/cifs_spnego.c @@ -93,6 +93,9 @@ struct key_type cifs_spnego_key_type = { /* strlen of ";pid=0x" */ #define PID_KEY_LEN 7 +/* strlen of ";pri=" */ +#define PRINC_KEY_LEN 5 + /* get a key struct with a SPNEGO security blob, suitable for session setup */ struct key * cifs_get_spnego_key(struct cifs_ses *sesInfo) @@ -109,6 +112,8 @@ cifs_get_spnego_key(struct cifs_ses *sesInfo) host=hostname sec=mechanism uid=0xFF user=username */ desc_len = MAX_VER_STR_LEN + HOST_KEY_LEN + strlen(hostname) + + (server->principal ? + PRINC_KEY_LEN + strlen(server->principal) : 0) + IP_KEY_LEN + INET6_ADDRSTRLEN + MAX_MECH_STR_LEN + UID_KEY_LEN + (sizeof(uid_t) * 2) + @@ -128,6 +133,11 @@ cifs_get_spnego_key(struct cifs_ses *sesInfo) hostname); dp = description + strlen(description); + if (server->principal) { + sprintf(dp, "pri=%s;", server->principal); + dp = description + strlen(description); + } + /* add the server address */ if (server->dstaddr.ss_family == AF_INET) sprintf(dp, "ip4=%pI4", &sa->sin_addr); diff --git a/fs/cifs/cifs_spnego.h b/fs/cifs/cifs_spnego.h index 31bef9e..bae1877 100644 --- a/fs/cifs/cifs_spnego.h +++ b/fs/cifs/cifs_spnego.h @@ -23,7 +23,7 @@ #ifndef _CIFS_SPNEGO_H #define _CIFS_SPNEGO_H -#define CIFS_SPNEGO_UPCALL_VERSION 2 +#define CIFS_SPNEGO_UPCALL_VERSION 3 /* * The version field should always be set to CIFS_SPNEGO_UPCALL_VERSION. diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h index 95dad9d..86de4fb 100644 --- a/fs/cifs/cifsglob.h +++ b/fs/cifs/cifsglob.h @@ -238,6 +238,7 @@ struct TCP_Server_Info { char server_RFC1001_name[RFC1001_NAME_LEN_WITH_NULL]; enum statusEnum tcpStatus; /* what we think the status is */ char *hostname; /* hostname portion of UNC string */ + char *principal; struct socket *ssocket; struct sockaddr_storage dstaddr; struct sockaddr_storage srcaddr; /* locally bind to this IP */ diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index f4af4cc..6fdca9f 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -618,6 +618,8 @@ static void clean_demultiplex_info(struct TCP_Server_Info *server) } kfree(server->hostname); + if (server->principal != NULL) + kfree(server->principal); kfree(server); length = atomic_dec_return(&tcpSesAllocCount); @@ -1780,6 +1782,7 @@ cifs_get_tcp_session(struct smb_vol *volume_info) rc = PTR_ERR(tcp_ses->hostname); goto out_err_crypto_release; } + tcp_ses->principal = NULL; tcp_ses->noblocksnd = volume_info->noblocksnd; tcp_ses->noautotune = volume_info->noautotune; -- 1.7.6 ^ permalink raw reply related [flat|nested] 23+ messages in thread
[parent not found: <1315322512-10652-1-git-send-email-martin.wilck-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org>]
* [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available [not found] ` <1315322512-10652-1-git-send-email-martin.wilck-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org> @ 2011-09-06 15:26 ` Martin Wilck [not found] ` <1315322794-10725-1-git-send-email-martin.wilck-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org> 2011-09-06 16:16 ` [RFC/PATCH] cifs: add server-provided principal name in upcall Jeff Layton 1 sibling, 1 reply; 23+ messages in thread From: Martin Wilck @ 2011-09-06 15:26 UTC (permalink / raw) To: linux-cifs-u79uwXL29TY76Z2rM5mHXA Cc: jlayton-eUNUBHrolfbYtjvyW6yDsg, sfrench-eUNUBHrolfbYtjvyW6yDsg, Martin Wilck This patch complements the kernel patch "cifs: add server-provided principal name in upcall". When cifs.upcall is started with -p, the kernel-provided principal name from the initial negotiation with the server will be used to obtain a ticket. If this fails, "cifs/<hostname>" and "host/hostname" fallbacks will be tried as fallback. This will enable kerberos-based CIFS mounts on more servers, and make mount.cifs work more similar to smbclient. Note that the "-p" option must be added in the cifs.upcall line in /etc/request-key.conf to activate this. Signed-off-by: Martin Wilck <martin.wilck-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org> --- cifs.upcall.8.in | 6 ++++++ cifs.upcall.c | 38 +++++++++++++++++++++++++++++++------- cifs_spnego.h | 2 +- 3 files changed, 38 insertions(+), 8 deletions(-) diff --git a/cifs.upcall.8.in b/cifs.upcall.8.in index 03842b7..7d90d1e 100644 --- a/cifs.upcall.8.in +++ b/cifs.upcall.8.in @@ -52,6 +52,12 @@ Traditionally, the kernel has sent only a single uid= parameter to the upcall fo Newer kernels send a creduid= option as well, which contains what uid it thinks actually owns the credentials that it\'s looking for\&. At mount time, this is generally set to the real uid of the user doing the mount. For multisession mounts, it's set to the fsuid of the mount user. Set this option if you want cifs.upcall to use the older uid= parameter instead of the creduid= parameter\&. .RE .PP +\-\-principal\-from\-kernel|\-p +.RS 4 +When this option is used, the principal name obtained by the kernel from the CIFS server is used rather than a principal name derived from the host name. If this fails, the host name is used as fallback. +This makes it possible to obtain credentials for CIFS servers using nonstandard principal names, and makes cifs mounts behave more similar to smbclient, which also uses the server-provided principal name. +.RE +.PP \-\-version|\-v .RS 4 Print version number and exit\&. diff --git a/cifs.upcall.c b/cifs.upcall.c index de92092..2adac1d 100644 --- a/cifs.upcall.c +++ b/cifs.upcall.c @@ -518,11 +518,13 @@ handle_krb5_mech(const char *oid, const char *principal, DATA_BLOB * secblob, #define DKD_HAVE_PID 0x20 #define DKD_HAVE_CREDUID 0x40 #define DKD_HAVE_USERNAME 0x80 +#define DKD_HAVE_PRINCIPAL 0x100 #define DKD_MUSTHAVE_SET (DKD_HAVE_HOSTNAME|DKD_HAVE_VERSION|DKD_HAVE_SEC) struct decoded_args { int ver; char *hostname; + char *principal; char *ip; char *username; uid_t uid; @@ -557,6 +559,21 @@ decode_key_description(const char *desc, struct decoded_args *arg) } retval |= DKD_HAVE_HOSTNAME; syslog(LOG_DEBUG, "host=%s", arg->hostname); + } else if (!strncmp(tkn, "pri=", 4)) { + if (pos == NULL) + len = strlen(tkn); + else + len = pos - tkn; + + len -= 4; + SAFE_FREE(arg->principal); + arg->principal = strndup(tkn + 4, len); + if (arg->principal == NULL) { + syslog(LOG_ERR, "Unable to allocate memory"); + return 1; + } + retval |= DKD_HAVE_PRINCIPAL; + syslog(LOG_DEBUG, "pri=%s", arg->principal); } else if (!strncmp(tkn, "ip4=", 4) || !strncmp(tkn, "ip6=", 4)) { if (pos == NULL) len = strlen(tkn); @@ -755,6 +772,7 @@ static void usage(void) const struct option long_options[] = { {"trust-dns", 0, NULL, 't'}, {"legacy-uid", 0, NULL, 'l'}, + {"principal-from-kernel", 0, NULL, 'p'}, {"version", 0, NULL, 'v'}, {NULL, 0, NULL, 0} }; @@ -768,7 +786,7 @@ int main(const int argc, char *const argv[]) size_t datalen; unsigned int have; long rc = 1; - int c, try_dns = 0, legacy_uid = 0; + int c, try_dns = 0, legacy_uid = 0, use_principal = 0; char *buf, *princ = NULL, *ccname = NULL; char hostbuf[NI_MAXHOST], *host; struct decoded_args arg; @@ -781,7 +799,7 @@ int main(const int argc, char *const argv[]) openlog(prog, 0, LOG_DAEMON); - while ((c = getopt_long(argc, argv, "cltv", long_options, NULL)) != -1) { + while ((c = getopt_long(argc, argv, "cltpv", long_options, NULL)) != -1) { switch (c) { case 'c': /* legacy option -- skip it */ @@ -792,6 +810,9 @@ int main(const int argc, char *const argv[]) case 'l': legacy_uid++; break; + case 'p': + use_principal++; + break; case 'v': printf("version: %s\n", VERSION); goto out; @@ -873,9 +894,16 @@ int main(const int argc, char *const argv[]) host = arg.hostname; // do mech specific authorization + oid = OID_KERBEROS5; switch (arg.sec) { case MS_KRB5: + oid = OID_KERBEROS5_OLD; case KRB5: + if (use_principal && have & DKD_HAVE_PRINCIPAL) { + rc = handle_krb5_mech(oid, arg.principal, &secblob, &sess_key, ccname); + if (!rc) + break; + } retry_new_hostname: /* for "cifs/" service name + terminating 0 */ datalen = strlen(host) + 5 + 1; @@ -885,11 +913,6 @@ retry_new_hostname: break; } - if (arg.sec == MS_KRB5) - oid = OID_KERBEROS5_OLD; - else - oid = OID_KERBEROS5; - /* * try getting a cifs/ principal first and then fall back to * getting a host/ principal if that doesn't work. @@ -965,6 +988,7 @@ out: data_blob_free(&sess_key); SAFE_FREE(ccname); SAFE_FREE(arg.hostname); + SAFE_FREE(arg.principal); SAFE_FREE(arg.ip); SAFE_FREE(arg.username); SAFE_FREE(keydata); diff --git a/cifs_spnego.h b/cifs_spnego.h index f8753a7..d43f965 100644 --- a/cifs_spnego.h +++ b/cifs_spnego.h @@ -23,7 +23,7 @@ #ifndef _CIFS_SPNEGO_H #define _CIFS_SPNEGO_H -#define CIFS_SPNEGO_UPCALL_VERSION 2 +#define CIFS_SPNEGO_UPCALL_VERSION 3 /* * The version field should always be set to CIFS_SPNEGO_UPCALL_VERSION. -- 1.7.6 ^ permalink raw reply related [flat|nested] 23+ messages in thread
[parent not found: <1315322794-10725-1-git-send-email-martin.wilck-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org>]
* Re: [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available [not found] ` <1315322794-10725-1-git-send-email-martin.wilck-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org> @ 2011-09-06 16:10 ` Jeff Layton [not found] ` <4E673D6F.90606@ts.fujitsu.com> 2011-09-07 22:18 ` Steve French 1 sibling, 1 reply; 23+ messages in thread From: Jeff Layton @ 2011-09-06 16:10 UTC (permalink / raw) To: Martin Wilck Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA, sfrench-eUNUBHrolfbYtjvyW6yDsg On Tue, 6 Sep 2011 17:26:34 +0200 Martin Wilck <martin.wilck-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org> wrote: > This patch complements the kernel patch > "cifs: add server-provided principal name in upcall". When cifs.upcall > is started with -p, the kernel-provided principal name from the initial > negotiation with the server will be used to obtain a ticket. If this fails, > "cifs/<hostname>" and "host/hostname" fallbacks will be tried as fallback. > > This will enable kerberos-based CIFS mounts on more servers, and make > mount.cifs work more similar to smbclient. > > Note that the "-p" option must be added in the cifs.upcall line in > /etc/request-key.conf to activate this. > > Signed-off-by: Martin Wilck <martin.wilck-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org> > --- > cifs.upcall.8.in | 6 ++++++ > cifs.upcall.c | 38 +++++++++++++++++++++++++++++++------- > cifs_spnego.h | 2 +- > 3 files changed, 38 insertions(+), 8 deletions(-) > > diff --git a/cifs.upcall.8.in b/cifs.upcall.8.in > index 03842b7..7d90d1e 100644 > --- a/cifs.upcall.8.in > +++ b/cifs.upcall.8.in > @@ -52,6 +52,12 @@ Traditionally, the kernel has sent only a single uid= parameter to the upcall fo > Newer kernels send a creduid= option as well, which contains what uid it thinks actually owns the credentials that it\'s looking for\&. At mount time, this is generally set to the real uid of the user doing the mount. For multisession mounts, it's set to the fsuid of the mount user. Set this option if you want cifs.upcall to use the older uid= parameter instead of the creduid= parameter\&. > .RE > .PP > +\-\-principal\-from\-kernel|\-p > +.RS 4 > +When this option is used, the principal name obtained by the kernel from the CIFS server is used rather than a principal name derived from the host name. If this fails, the host name is used as fallback. > +This makes it possible to obtain credentials for CIFS servers using nonstandard principal names, and makes cifs mounts behave more similar to smbclient, which also uses the server-provided principal name. > +.RE > +.PP I'm not opposed to adding this with appropriate warnings about the danger involved. Trusting the SPN provided in the NEGOTIATE response waters down much of the security that Kerberos provides. Granted, cifs doesn't currently do mutual auth, but if it did, using this would make it pretty useless. It would probably be a good idea to clearly warn that an attacker can use this in order to trick the client into mounting a server of his choosing (providing he can redirect the traffic to that server too). > \-\-version|\-v > .RS 4 > Print version number and exit\&. > diff --git a/cifs.upcall.c b/cifs.upcall.c > index de92092..2adac1d 100644 > --- a/cifs.upcall.c > +++ b/cifs.upcall.c > @@ -518,11 +518,13 @@ handle_krb5_mech(const char *oid, const char *principal, DATA_BLOB * secblob, > #define DKD_HAVE_PID 0x20 > #define DKD_HAVE_CREDUID 0x40 > #define DKD_HAVE_USERNAME 0x80 > +#define DKD_HAVE_PRINCIPAL 0x100 > #define DKD_MUSTHAVE_SET (DKD_HAVE_HOSTNAME|DKD_HAVE_VERSION|DKD_HAVE_SEC) > > struct decoded_args { > int ver; > char *hostname; > + char *principal; > char *ip; > char *username; > uid_t uid; > @@ -557,6 +559,21 @@ decode_key_description(const char *desc, struct decoded_args *arg) > } > retval |= DKD_HAVE_HOSTNAME; > syslog(LOG_DEBUG, "host=%s", arg->hostname); > + } else if (!strncmp(tkn, "pri=", 4)) { > + if (pos == NULL) > + len = strlen(tkn); > + else > + len = pos - tkn; > + > + len -= 4; > + SAFE_FREE(arg->principal); > + arg->principal = strndup(tkn + 4, len); > + if (arg->principal == NULL) { > + syslog(LOG_ERR, "Unable to allocate memory"); > + return 1; > + } > + retval |= DKD_HAVE_PRINCIPAL; > + syslog(LOG_DEBUG, "pri=%s", arg->principal); > } else if (!strncmp(tkn, "ip4=", 4) || !strncmp(tkn, "ip6=", 4)) { > if (pos == NULL) > len = strlen(tkn); > @@ -755,6 +772,7 @@ static void usage(void) > const struct option long_options[] = { > {"trust-dns", 0, NULL, 't'}, > {"legacy-uid", 0, NULL, 'l'}, > + {"principal-from-kernel", 0, NULL, 'p'}, > {"version", 0, NULL, 'v'}, > {NULL, 0, NULL, 0} > }; > @@ -768,7 +786,7 @@ int main(const int argc, char *const argv[]) > size_t datalen; > unsigned int have; > long rc = 1; > - int c, try_dns = 0, legacy_uid = 0; > + int c, try_dns = 0, legacy_uid = 0, use_principal = 0; > char *buf, *princ = NULL, *ccname = NULL; > char hostbuf[NI_MAXHOST], *host; > struct decoded_args arg; > @@ -781,7 +799,7 @@ int main(const int argc, char *const argv[]) > > openlog(prog, 0, LOG_DAEMON); > > - while ((c = getopt_long(argc, argv, "cltv", long_options, NULL)) != -1) { > + while ((c = getopt_long(argc, argv, "cltpv", long_options, NULL)) != -1) { > switch (c) { > case 'c': > /* legacy option -- skip it */ > @@ -792,6 +810,9 @@ int main(const int argc, char *const argv[]) > case 'l': > legacy_uid++; > break; > + case 'p': > + use_principal++; > + break; > case 'v': > printf("version: %s\n", VERSION); > goto out; > @@ -873,9 +894,16 @@ int main(const int argc, char *const argv[]) > host = arg.hostname; > > // do mech specific authorization > + oid = OID_KERBEROS5; > switch (arg.sec) { > case MS_KRB5: > + oid = OID_KERBEROS5_OLD; > case KRB5: > + if (use_principal && have & DKD_HAVE_PRINCIPAL) { > + rc = handle_krb5_mech(oid, arg.principal, &secblob, &sess_key, ccname); > + if (!rc) > + break; > + } > retry_new_hostname: > /* for "cifs/" service name + terminating 0 */ > datalen = strlen(host) + 5 + 1; > @@ -885,11 +913,6 @@ retry_new_hostname: > break; > } > > - if (arg.sec == MS_KRB5) > - oid = OID_KERBEROS5_OLD; > - else > - oid = OID_KERBEROS5; > - This delta and the change above it to move that into the switch would be more appropriate in a separate patch. > /* > * try getting a cifs/ principal first and then fall back to > * getting a host/ principal if that doesn't work. > @@ -965,6 +988,7 @@ out: > data_blob_free(&sess_key); > SAFE_FREE(ccname); > SAFE_FREE(arg.hostname); > + SAFE_FREE(arg.principal); > SAFE_FREE(arg.ip); > SAFE_FREE(arg.username); > SAFE_FREE(keydata); > diff --git a/cifs_spnego.h b/cifs_spnego.h > index f8753a7..d43f965 100644 > --- a/cifs_spnego.h > +++ b/cifs_spnego.h > @@ -23,7 +23,7 @@ > #ifndef _CIFS_SPNEGO_H > #define _CIFS_SPNEGO_H > > -#define CIFS_SPNEGO_UPCALL_VERSION 2 > +#define CIFS_SPNEGO_UPCALL_VERSION 3 I don't think we need to rev the upcall version for this. Adding a new field doesn't require us to change that. > > /* > * The version field should always be set to CIFS_SPNEGO_UPCALL_VERSION. -- Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <4E673D6F.90606@ts.fujitsu.com>]
* Re: [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available [not found] ` <4E673D6F.90606@ts.fujitsu.com> @ 2011-09-07 13:03 ` Jeff Layton 2011-09-07 21:42 ` Andrew Bartlett 0 siblings, 1 reply; 23+ messages in thread From: Jeff Layton @ 2011-09-07 13:03 UTC (permalink / raw) To: Martin Wilck; +Cc: linux-cifs, samba-technical, Martin Wilck On Wed, 07 Sep 2011 11:46:23 +0200 Martin Wilck <martin.wilck@ts.fujitsu.com> wrote: > Hi Jeff, > > thanks for reviewing this. > > > I'm not opposed to adding this with appropriate warnings about the > > danger involved. > > > > Trusting the SPN provided in the NEGOTIATE response waters down much of > > the security that Kerberos provides. Granted, cifs doesn't currently do > > mutual auth, but if it did, using this would make it pretty useless. > > Please help me understand - is this functionality any different from > smbclient's? If yes, what do I need to change? If no, smbclient users > will suffer from the same security risk (I see that a mounted file > system is a higher risk than a process like smbclient). > > Is there any way to do this more securely? > > > It would probably be a good idea to clearly warn that an attacker can > > use this in order to trick the client into mounting a server of his > > choosing (providing he can redirect the traffic to that server too). > > I'm will happily add a warning if you tell me where you'd like to have > it - in the man page, or in the kernel logs, or in the cifs.upcall log? > > Regards > Martin > (re-cc'ing linux-cifs and cc'ing samba-technical) We've discussed this on the list many times before, but the most comprehensive discussion is here. I recommend reading over that as it explains the problems in detail: http://lists.samba.org/archive/linux-cifs-client/2008-August/003348.html Really, the best answer is not to rely on this. Windows clients never have, and recent windows servers don't even populate the field. smbclient does, but support for that was added long ago. It should probably be removed as Andrew suggested in the above thread, or perhaps made conditional on a new smb.conf option that defaults to being off. -- Jeff Layton <jlayton@samba.org> ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available 2011-09-07 13:03 ` Jeff Layton @ 2011-09-07 21:42 ` Andrew Bartlett 2011-09-08 7:23 ` Martin Wilck 0 siblings, 1 reply; 23+ messages in thread From: Andrew Bartlett @ 2011-09-07 21:42 UTC (permalink / raw) To: Jeff Layton; +Cc: Martin Wilck, linux-cifs, samba-technical, Martin Wilck On Wed, 2011-09-07 at 09:03 -0400, Jeff Layton wrote: > On Wed, 07 Sep 2011 11:46:23 +0200 > Martin Wilck <martin.wilck@ts.fujitsu.com> wrote: > > > Hi Jeff, > > > > thanks for reviewing this. > > > > > I'm not opposed to adding this with appropriate warnings about the > > > danger involved. > > > > > > Trusting the SPN provided in the NEGOTIATE response waters down much of > > > the security that Kerberos provides. Granted, cifs doesn't currently do > > > mutual auth, but if it did, using this would make it pretty useless. > > > > Please help me understand - is this functionality any different from > > smbclient's? If yes, what do I need to change? If no, smbclient users > > will suffer from the same security risk (I see that a mounted file > > system is a higher risk than a process like smbclient). > > > > Is there any way to do this more securely? > > > > > It would probably be a good idea to clearly warn that an attacker can > > > use this in order to trick the client into mounting a server of his > > > choosing (providing he can redirect the traffic to that server too). > > > > I'm will happily add a warning if you tell me where you'd like to have > > it - in the man page, or in the kernel logs, or in the cifs.upcall log? > > > > Regards > > Martin > > > > (re-cc'ing linux-cifs and cc'ing samba-technical) > > We've discussed this on the list many times before, but the most > comprehensive discussion is here. I recommend reading over that as it > explains the problems in detail: > > http://lists.samba.org/archive/linux-cifs-client/2008-August/003348.html > > Really, the best answer is not to rely on this. Windows clients never > have, and recent windows servers don't even populate the field. > > smbclient does, but support for that was added long ago. It should > probably be removed as Andrew suggested in the above thread, or > perhaps made conditional on a new smb.conf option that defaults to > being off. Samba 3.5 (latest release) and Samba 3.6 now default to the server-provided SPN being untrusted and unused. Samba 3.6 also does not send it by default. No new software should start using this principal, and now all modern servers will not send it. Please do not introduce this security hole into new software. The only reason that Samba continues to have optional support for this at all is in deference to possible legacy users. Andrew Bartlett -- Andrew Bartlett http://samba.org/~abartlet/ Authentication Developer, Samba Team http://samba.org ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available 2011-09-07 21:42 ` Andrew Bartlett @ 2011-09-08 7:23 ` Martin Wilck [not found] ` <4E686D69.9090503-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org> 0 siblings, 1 reply; 23+ messages in thread From: Martin Wilck @ 2011-09-08 7:23 UTC (permalink / raw) To: Andrew Bartlett Cc: Jeff Layton, linux-cifs-u79uwXL29TY76Z2rM5mHXA, samba-technical-w/Ol4Ecudpl8XjKLYN78aQ, Martin Wilck Hi Andrew, > Samba 3.5 (latest release) and Samba 3.6 now default to the > server-provided SPN being untrusted and unused. Samba 3.6 also does not > send it by default. No new software should start using this principal, > and now all modern servers will not send it. Please let me repeat my dumb question: Once a Windows user has logged in to the AD Domain, she'll be able to access all shares in the domain (which her account is allowed to use) without having to retype the domain password. Am I wrong assuming this works through Kerberos somehow? On Linux clients, users need to retype passwords all the time. That makes Linux machines 2nd class network members, at least psychologically for their users. The reason (in my environment) is that the samba tools are unable to figure out the correct SPN. I understand your argument that trying to do that via the server-provided name is insecure. That means there must be some other way, as Windows clients are able to find the SPN. What is it? Regards Martin -- Dr. Martin Wilck PRIMERGY System Software Engineer x86 Server Engineering FUJITSU Fujitsu Technology Solutions GmbH Heinz-Nixdorf-Ring 1 33106 Paderborn, Germany Phone: ++49 5251 525 2796 Fax: ++49 5251 525 2820 Email: martin.wilck-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org Internet: http://ts.fujitsu.com Company Details: http://ts.fujitsu.com/imprint ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <4E686D69.9090503-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org>]
* Re: [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available [not found] ` <4E686D69.9090503-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org> @ 2011-09-08 7:39 ` Andrew Bartlett 2011-09-08 12:53 ` Martin Wilck 0 siblings, 1 reply; 23+ messages in thread From: Andrew Bartlett @ 2011-09-08 7:39 UTC (permalink / raw) To: Martin Wilck Cc: Jeff Layton, linux-cifs-u79uwXL29TY76Z2rM5mHXA, samba-technical-w/Ol4Ecudpl8XjKLYN78aQ, Martin Wilck On Thu, 2011-09-08 at 09:23 +0200, Martin Wilck wrote: > Hi Andrew, > > > Samba 3.5 (latest release) and Samba 3.6 now default to the > > server-provided SPN being untrusted and unused. Samba 3.6 also does not > > send it by default. No new software should start using this principal, > > and now all modern servers will not send it. > > Please let me repeat my dumb question: Once a Windows user has logged in > to the AD Domain, she'll be able to access all shares in the domain > (which her account is allowed to use) without having to retype the > domain password. Am I wrong assuming this works through Kerberos somehow? > > On Linux clients, users need to retype passwords all the time. That > makes Linux machines 2nd class network members, at least psychologically > for their users. The reason (in my environment) is that the samba tools > are unable to figure out the correct SPN. I understand your argument > that trying to do that via the server-provided name is insecure. That > means there must be some other way, as Windows clients are able to find > the SPN. What is it? The name of the server is the right name, ie the name in the UNC path or URL. Do your linux users mount by IP address perhaps? Andrew Bartlett -- Andrew Bartlett http://samba.org/~abartlet/ Authentication Developer, Samba Team http://samba.org ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available 2011-09-08 7:39 ` Andrew Bartlett @ 2011-09-08 12:53 ` Martin Wilck [not found] ` <4E68BACD.2020403-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org> 0 siblings, 1 reply; 23+ messages in thread From: Martin Wilck @ 2011-09-08 12:53 UTC (permalink / raw) To: Andrew Bartlett Cc: Jeff Layton, linux-cifs-u79uwXL29TY76Z2rM5mHXA, samba-technical-w/Ol4Ecudpl8XjKLYN78aQ, Martin Wilck On 09/08/2011 09:39 AM, Andrew Bartlett wrote: > The name of the server is the right name, ie the name in the UNC path > or URL. Hmm, it may be that just a few servers are affected here. we have one important server which appears to have a weird DNS setup: dig +short -t A server.domain.net 172.100.10.25 dig +short -t CNAME server.domain.net (NORESPONSE) dig +short -t PTR -x 172.100.10.25 c10203.domain.net dig +short -t A c10203.domain.net (NORESPONSE) While this is quite obviously broken, Windows clients in the domain have no problems accessing this server, while Linux clients do. The only temporary workaround I found for this is to add c10203 to /etc/hosts locally and mount the share as "//c10203/share" (//c10203.domain.net/share does *not* work). This is samba 3.5.8, kernel 2.6.38.6-27.fc15 on Fedora 15. > Do your linux users mount by IP address perhaps? No, see above. Martin -- Dr. Martin Wilck PRIMERGY System Software Engineer x86 Server Engineering FUJITSU Fujitsu Technology Solutions GmbH Heinz-Nixdorf-Ring 1 33106 Paderborn, Germany Phone: ++49 5251 525 2796 Fax: ++49 5251 525 2820 Email: martin.wilck-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org Internet: http://ts.fujitsu.com Company Details: http://ts.fujitsu.com/imprint ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <4E68BACD.2020403-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org>]
* Re: [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available [not found] ` <4E68BACD.2020403-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org> @ 2011-09-08 12:59 ` simo 2011-09-08 13:01 ` Andrew Bartlett 1 sibling, 0 replies; 23+ messages in thread From: simo @ 2011-09-08 12:59 UTC (permalink / raw) To: Martin Wilck Cc: Andrew Bartlett, Jeff Layton, linux-cifs-u79uwXL29TY76Z2rM5mHXA, samba-technical-w/Ol4Ecudpl8XjKLYN78aQ, Martin Wilck On Thu, 2011-09-08 at 14:53 +0200, Martin Wilck wrote: > On 09/08/2011 09:39 AM, Andrew Bartlett wrote: > > > The name of the server is the right name, ie the name in the UNC path > > or URL. > > Hmm, it may be that just a few servers are affected here. we have one > important server which appears to have a weird DNS setup: > > dig +short -t A server.domain.net > 172.100.10.25 > dig +short -t CNAME server.domain.net > (NORESPONSE) > dig +short -t PTR -x 172.100.10.25 > c10203.domain.net > dig +short -t A c10203.domain.net > (NORESPONSE) > > While this is quite obviously broken, Windows clients in the domain have > no problems accessing this server, while Linux clients do. The only > temporary workaround I found for this is to add c10203 to /etc/hosts > locally and mount the share as "//c10203/share" > (//c10203.domain.net/share does *not* work). > > This is samba 3.5.8, kernel 2.6.38.6-27.fc15 on Fedora 15. Try setting rdns = false in krb5.conf It should work, as we recently fixed a few bugs in that support, but I do not recall if they have already all been pushed to f15 stable. Simo. -- Simo Sorce Samba Team GPL Compliance Officer <simo-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> Principal Software Engineer at Red Hat, Inc. <simo-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available [not found] ` <4E68BACD.2020403-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org> 2011-09-08 12:59 ` simo @ 2011-09-08 13:01 ` Andrew Bartlett 2011-09-08 13:13 ` Martin Wilck 1 sibling, 1 reply; 23+ messages in thread From: Andrew Bartlett @ 2011-09-08 13:01 UTC (permalink / raw) To: Martin Wilck Cc: Jeff Layton, linux-cifs-u79uwXL29TY76Z2rM5mHXA, samba-technical-w/Ol4Ecudpl8XjKLYN78aQ, Martin Wilck On Thu, 2011-09-08 at 14:53 +0200, Martin Wilck wrote: > On 09/08/2011 09:39 AM, Andrew Bartlett wrote: > > > The name of the server is the right name, ie the name in the UNC path > > or URL. > > Hmm, it may be that just a few servers are affected here. we have one > important server which appears to have a weird DNS setup: > > dig +short -t A server.domain.net > 172.100.10.25 > dig +short -t CNAME server.domain.net > (NORESPONSE) > dig +short -t PTR -x 172.100.10.25 > c10203.domain.net > dig +short -t A c10203.domain.net > (NORESPONSE) > > While this is quite obviously broken, Windows clients in the domain have > no problems accessing this server, while Linux clients do. The only > temporary workaround I found for this is to add c10203 to /etc/hosts > locally and mount the share as "//c10203/share" > (//c10203.domain.net/share does *not* work). > > This is samba 3.5.8, kernel 2.6.38.6-27.fc15 on Fedora 15. Try [libdefaults] rdns = false in your krb5.conf (The default value here isn't suitable for use in an AD environment). Andrew Bartlett -- Andrew Bartlett http://samba.org/~abartlet/ Authentication Developer, Samba Team http://samba.org ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available 2011-09-08 13:01 ` Andrew Bartlett @ 2011-09-08 13:13 ` Martin Wilck [not found] ` <4E68BF73.2090707-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org> 0 siblings, 1 reply; 23+ messages in thread From: Martin Wilck @ 2011-09-08 13:13 UTC (permalink / raw) To: Andrew Bartlett Cc: Jeff Layton, linux-cifs-u79uwXL29TY76Z2rM5mHXA, samba-technical-w/Ol4Ecudpl8XjKLYN78aQ, Martin Wilck On 09/08/2011 03:01 PM, Andrew Bartlett wrote: > Try > [libdefaults] > rdns = false > > in your krb5.conf Doesn't work, sorry. Actually, it doesn't seem to make any difference in my setup. In my scenario, cifs.upcall would be able to infer the correct SPN with the following algorithm: - get the IP address using DNS - get the "real" server FQDN using RDNS - use "cifs/<hostname portion of the "real" FQDN>" as SPN Thus RDNS might indeed be beneficial here (but "rdns = true" makes no difference, either). OTOH, from the security point of view, this algorithm might not be more secure than the server-provided SPN, because the attack scenario assumes that DNS and/or general network packet transmission is already hijacked. The question remains: what are the windows clients doing to overcome this situation? Martin > (The default value here isn't suitable for use in an AD environment). > > Andrew Bartlett -- Dr. Martin Wilck PRIMERGY System Software Engineer x86 Server Engineering FUJITSU Fujitsu Technology Solutions GmbH Heinz-Nixdorf-Ring 1 33106 Paderborn, Germany Phone: ++49 5251 525 2796 Fax: ++49 5251 525 2820 Email: martin.wilck-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org Internet: http://ts.fujitsu.com Company Details: http://ts.fujitsu.com/imprint ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <4E68BF73.2090707-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org>]
* Re: [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available [not found] ` <4E68BF73.2090707-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org> @ 2011-09-08 13:23 ` simo 2011-09-08 13:23 ` Andrew Bartlett 2011-09-08 13:31 ` Jeff Layton 2 siblings, 0 replies; 23+ messages in thread From: simo @ 2011-09-08 13:23 UTC (permalink / raw) To: Martin Wilck Cc: Andrew Bartlett, Jeff Layton, linux-cifs-u79uwXL29TY76Z2rM5mHXA, samba-technical-w/Ol4Ecudpl8XjKLYN78aQ, Martin Wilck On Thu, 2011-09-08 at 15:13 +0200, Martin Wilck wrote: > Doesn't work, sorry. Actually, it doesn't seem to make any difference > in > my setup. In my scenario, cifs.upcall would be able to infer the > correct > SPN with the following algorithm: > > - get the IP address using DNS > - get the "real" server FQDN using RDNS > - use "cifs/<hostname portion of the "real" FQDN>" as SPN If cifs does an explicit PTR resolution than that is probably a bug and should be fixed. > Thus RDNS might indeed be beneficial here (but "rdns = true" makes no > difference, either). > > OTOH, from the security point of view, this algorithm might not be > more > secure than the server-provided SPN, because the attack scenario > assumes > that DNS and/or general network packet transmission is already > hijacked. DNS being hijacked is not an issue. The issue is thinking you are connecting to server A and instead ending up with connecting to B If DNS is hijacked so the A points to B's ip address you still ask for a ticket for A and that's what you get from the KDC. When then ou try to auth to B with A's ticket, all breaks down. Instead if you trust what B says its name is, then you would get a ticket for B and auth will continue properly. So a user that wanted to contact server A ends up contacting servr B and may not notice. Now reply A with topsecret.agency.gov and B with public.agency.gov and you understand why that is an issue. > The question remains: what are the windows clients doing to overcome > this situation? They delegate a lot of name resolution to the KDC, in particular they let the KDC do the name canonicalization instead of doing it on their own. Simo. -- Simo Sorce Samba Team GPL Compliance Officer <simo-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> Principal Software Engineer at Red Hat, Inc. <simo-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available [not found] ` <4E68BF73.2090707-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org> 2011-09-08 13:23 ` simo @ 2011-09-08 13:23 ` Andrew Bartlett 2011-09-08 14:54 ` Jeff Layton [not found] ` <4E68EEAE.2090102@ts.fujitsu.com> 2011-09-08 13:31 ` Jeff Layton 2 siblings, 2 replies; 23+ messages in thread From: Andrew Bartlett @ 2011-09-08 13:23 UTC (permalink / raw) To: Martin Wilck Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA, samba-technical-w/Ol4Ecudpl8XjKLYN78aQ, Martin Wilck On Thu, 2011-09-08 at 15:13 +0200, Martin Wilck wrote: > On 09/08/2011 03:01 PM, Andrew Bartlett wrote: > > > Try > > [libdefaults] > > rdns = false > > > > in your krb5.conf > > Doesn't work, sorry. Actually, it doesn't seem to make any difference in > my setup. In my scenario, cifs.upcall would be able to infer the correct > SPN with the following algorithm: > > - get the IP address using DNS > - get the "real" server FQDN using RDNS > - use "cifs/<hostname portion of the "real" FQDN>" as SPN > > Thus RDNS might indeed be beneficial here (but "rdns = true" makes no > difference, either). > > OTOH, from the security point of view, this algorithm might not be more > secure than the server-provided SPN, because the attack scenario assumes > that DNS and/or general network packet transmission is already hijacked. > > The question remains: what are the windows clients doing to overcome > this situation? They use only the name, as typed. Windows never uses reverse DNS, as it is rare on Windows networks. The AD KDC answers to short, long and alias names for a server, removing the need for the client to 'guess' what the right name it. The SPN should simply be cifs/<name as originally specified>. Andrew Bartlett -- Andrew Bartlett http://samba.org/~abartlet/ Authentication Developer, Samba Team http://samba.org ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available 2011-09-08 13:23 ` Andrew Bartlett @ 2011-09-08 14:54 ` Jeff Layton [not found] ` <4E68EEAE.2090102@ts.fujitsu.com> 1 sibling, 0 replies; 23+ messages in thread From: Jeff Layton @ 2011-09-08 14:54 UTC (permalink / raw) To: Andrew Bartlett Cc: Martin Wilck, linux-cifs-u79uwXL29TY76Z2rM5mHXA, samba-technical-w/Ol4Ecudpl8XjKLYN78aQ, Martin Wilck On Thu, 08 Sep 2011 23:23:05 +1000 Andrew Bartlett <abartlet-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote: > On Thu, 2011-09-08 at 15:13 +0200, Martin Wilck wrote: > > On 09/08/2011 03:01 PM, Andrew Bartlett wrote: > > > > > Try > > > [libdefaults] > > > rdns = false > > > > > > in your krb5.conf > > > > Doesn't work, sorry. Actually, it doesn't seem to make any difference in > > my setup. In my scenario, cifs.upcall would be able to infer the correct > > SPN with the following algorithm: > > > > - get the IP address using DNS > > - get the "real" server FQDN using RDNS > > - use "cifs/<hostname portion of the "real" FQDN>" as SPN > > > > Thus RDNS might indeed be beneficial here (but "rdns = true" makes no > > difference, either). > > > > OTOH, from the security point of view, this algorithm might not be more > > secure than the server-provided SPN, because the attack scenario assumes > > that DNS and/or general network packet transmission is already hijacked. > > > > The question remains: what are the windows clients doing to overcome > > this situation? > > They use only the name, as typed. Windows never uses reverse DNS, as it > is rare on Windows networks. > > The AD KDC answers to short, long and alias names for a server, removing > the need for the client to 'guess' what the right name it. The SPN > should simply be cifs/<name as originally specified>. > That should be what cifs.ko does as well... The hostname portion of the UNC is passed to the kernel, which in turn passes it to the keys upcall. That's then prefixed with "cifs/" and we attempt to get a service ticket for that principal. If that fails, we try again with a prefix of "host/". Perhaps there's a better way to construct the SPN from that info? I'm open to suggestions here, but we'll need to take care not to break existing setups. -- Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <4E68EEAE.2090102@ts.fujitsu.com>]
[parent not found: <4E68EEAE.2090102-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org>]
* Re: [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available [not found] ` <4E68EEAE.2090102-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org> @ 2011-09-09 13:37 ` Jeff Layton 2011-09-12 9:01 ` Martin Wilck 0 siblings, 1 reply; 23+ messages in thread From: Jeff Layton @ 2011-09-09 13:37 UTC (permalink / raw) To: Martin Wilck Cc: Andrew Bartlett, samba-technical-w/Ol4Ecudpl8XjKLYN78aQ, Martin Wilck, linux-cifs-u79uwXL29TY76Z2rM5mHXA On Thu, 08 Sep 2011 18:34:54 +0200 Martin Wilck <martin.wilck-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org> wrote: > (I needed to remove linux-cifs from cc because of DNS troubles again - > sorry) > > On 09/08/2011 03:23 PM, Andrew Bartlett wrote: > > >> The question remains: what are the windows clients doing to overcome > >> this situation? > > > > They use only the name, as typed. Windows never uses reverse DNS, as it > > is rare on Windows networks. > > It turns out that with the mentioned server, Kerberos doesn't work with > Windows clients, either. Instead I can see (in wireshark) the Win > clients falling back to NTLM authentication (or using it right away). > That explains a lot. Sorry for the noise. (I'm currently looking for a > switch in XP to force it to use Kerberos for testing purposes, but so > far I found none). > Ok, good to know...thanks. For now, we'll plan to table these patches. For the record, I'm not 100% opposed to adding something like this as a workaround. What would probably be better would be a way for someone to specify the SPN in the mount options. The kernel could then pass that to the upcall and we wouldn't need to trust this string from the server. Admins would of course need to know what SPN to put in there however. Something like: -o spn=cifs/otherhostname.example.com ...for example. -- Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available 2011-09-09 13:37 ` Jeff Layton @ 2011-09-12 9:01 ` Martin Wilck [not found] ` <4E6DCA86.8020707-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org> 0 siblings, 1 reply; 23+ messages in thread From: Martin Wilck @ 2011-09-12 9:01 UTC (permalink / raw) To: Jeff Layton; +Cc: linux-cifs, samba-technical, Martin Wilck, Andrew Bartlett > For the record, I'm not 100% opposed to adding something like this as a > workaround. What would probably be better would be a way for someone to > specify the SPN in the mount options. The kernel could then pass that > to the upcall and we wouldn't need to trust this string from the > server. Admins would of course need to know what SPN to put in there > however. Something like: > > -o spn=cifs/otherhostname.example.com Sounds good. In our AD environment, an admin can do ldapsearch "(cn=$COMPUTERNAME)" serviceprincipalname to get the supported principal name(s). Martin -- Dr. Martin Wilck PRIMERGY System Software Engineer x86 Server Engineering FUJITSU Fujitsu Technology Solutions GmbH Heinz-Nixdorf-Ring 1 33106 Paderborn, Germany Phone: ++49 5251 525 2796 Fax: ++49 5251 525 2820 Email: martin.wilck@ts.fujitsu.com Internet: http://ts.fujitsu.com Company Details: http://ts.fujitsu.com/imprint ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <4E6DCA86.8020707-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org>]
* Re: [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available [not found] ` <4E6DCA86.8020707-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org> @ 2011-09-12 13:41 ` Jeff Layton [not found] ` <20110912094114.4e7f2b8e-4QP7MXygkU+dMjc06nkz3ljfA9RmPOcC@public.gmane.org> 2011-09-12 23:23 ` Andrew Bartlett 1 sibling, 1 reply; 23+ messages in thread From: Jeff Layton @ 2011-09-12 13:41 UTC (permalink / raw) To: Martin Wilck Cc: Andrew Bartlett, samba-technical-w/Ol4Ecudpl8XjKLYN78aQ, Martin Wilck, linux-cifs-u79uwXL29TY76Z2rM5mHXA On Mon, 12 Sep 2011 11:01:58 +0200 Martin Wilck <martin.wilck-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org> wrote: > > For the record, I'm not 100% opposed to adding something like this as a > > workaround. What would probably be better would be a way for someone to > > specify the SPN in the mount options. The kernel could then pass that > > to the upcall and we wouldn't need to trust this string from the > > server. Admins would of course need to know what SPN to put in there > > however. Something like: > > > > -o spn=cifs/otherhostname.example.com > > Sounds good. In our AD environment, an admin can do > > ldapsearch "(cn=$COMPUTERNAME)" serviceprincipalname > > to get the supported principal name(s). > If that's the standard mechanism that windows machines use to determine this, we could consider doing something similar in cifs.upcall. Maybe add a new command-line option that tells it to query a particular LDAP server with krb5 auth to determine this? -- Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <20110912094114.4e7f2b8e-4QP7MXygkU+dMjc06nkz3ljfA9RmPOcC@public.gmane.org>]
* Re: [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available [not found] ` <20110912094114.4e7f2b8e-4QP7MXygkU+dMjc06nkz3ljfA9RmPOcC@public.gmane.org> @ 2011-09-12 14:00 ` simo 0 siblings, 0 replies; 23+ messages in thread From: simo @ 2011-09-12 14:00 UTC (permalink / raw) To: Jeff Layton Cc: Martin Wilck, linux-cifs-u79uwXL29TY76Z2rM5mHXA, samba-technical-w/Ol4Ecudpl8XjKLYN78aQ, Martin Wilck, Andrew Bartlett On Mon, 2011-09-12 at 09:41 -0400, Jeff Layton wrote: > On Mon, 12 Sep 2011 11:01:58 +0200 > Martin Wilck <martin.wilck-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org> wrote: > > > > For the record, I'm not 100% opposed to adding something like this as a > > > workaround. What would probably be better would be a way for someone to > > > specify the SPN in the mount options. The kernel could then pass that > > > to the upcall and we wouldn't need to trust this string from the > > > server. Admins would of course need to know what SPN to put in there > > > however. Something like: > > > > > > -o spn=cifs/otherhostname.example.com > > > > Sounds good. In our AD environment, an admin can do > > > > ldapsearch "(cn=$COMPUTERNAME)" serviceprincipalname > > > > to get the supported principal name(s). > > > > If that's the standard mechanism that windows machines use to determine > this, we could consider doing something similar in cifs.upcall. Maybe > add a new command-line option that tells it to query a particular LDAP > server with krb5 auth to determine this? No Windows clients do not rely on that. It's a mixture of dscracknames RPC and the KDC doing canonicalization on its own IIRC. Simo. -- Simo Sorce Samba Team GPL Compliance Officer <simo-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> Principal Software Engineer at Red Hat, Inc. <simo-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available [not found] ` <4E6DCA86.8020707-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org> 2011-09-12 13:41 ` Jeff Layton @ 2011-09-12 23:23 ` Andrew Bartlett 2011-09-13 11:01 ` Martin Wilck 1 sibling, 1 reply; 23+ messages in thread From: Andrew Bartlett @ 2011-09-12 23:23 UTC (permalink / raw) To: Martin Wilck Cc: Jeff Layton, samba-technical-w/Ol4Ecudpl8XjKLYN78aQ, Martin Wilck, linux-cifs-u79uwXL29TY76Z2rM5mHXA On Mon, 2011-09-12 at 11:01 +0200, Martin Wilck wrote: > > For the record, I'm not 100% opposed to adding something like this as a > > workaround. What would probably be better would be a way for someone to > > specify the SPN in the mount options. The kernel could then pass that > > to the upcall and we wouldn't need to trust this string from the > > server. Admins would of course need to know what SPN to put in there > > however. Something like: > > > > -o spn=cifs/otherhostname.example.com > > Sounds good. In our AD environment, an admin can do > > ldapsearch "(cn=$COMPUTERNAME)" serviceprincipalname > > to get the supported principal name(s). If they know the computer name, why don't they connect to it as $COMPUTERNAME? That's how this is meant to work - the DNS or netbios name the user resolves for the connection to is either the cn, dnsHostname or in the servicePrincipalNames of the record. If your users are connecting to names not in that list, why not just add them to the servicePrincipalNames list? We really should not be adding more and more hacks around this area, they will only bite us later. Andrew Bartlett -- Andrew Bartlett http://samba.org/~abartlet/ Authentication Developer, Samba Team http://samba.org ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available 2011-09-12 23:23 ` Andrew Bartlett @ 2011-09-13 11:01 ` Martin Wilck 0 siblings, 0 replies; 23+ messages in thread From: Martin Wilck @ 2011-09-13 11:01 UTC (permalink / raw) To: Andrew Bartlett Cc: Jeff Layton, samba-technical-w/Ol4Ecudpl8XjKLYN78aQ, Martin Wilck, linux-cifs-u79uwXL29TY76Z2rM5mHXA On 09/13/2011 01:23 AM, Andrew Bartlett wrote: > If they know the computer name, why don't they connect to it as > $COMPUTERNAME? That's how this is meant to work - the DNS or netbios > name the user resolves for the connection to is either the cn, > dnsHostname or in the servicePrincipalNames of the record. As I said earlier, that's what the Win clients do, and when it fails, they fall back to NTLM which won't bother with SPNs. The user never gets to know the difference. > If your users are connecting to names not in that list, why not just add > them to the servicePrincipalNames list? We really should not be adding > more and more hacks around this area, they will only bite us later. I have requested that from our sysadmin. When I first discovered that Win clients could connect to the service in question while the Linux cifs client couldn't, I suspected a problem with the cifs client (especially because smbclient was able to connect with kerberos, too). I do understand now that this conclusion was wrong. Regards Martin -- Dr. Martin Wilck PRIMERGY System Software Engineer x86 Server Engineering FUJITSU Fujitsu Technology Solutions GmbH Heinz-Nixdorf-Ring 1 33106 Paderborn, Germany Phone: ++49 5251 525 2796 Fax: ++49 5251 525 2820 Email: martin.wilck-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org Internet: http://ts.fujitsu.com Company Details: http://ts.fujitsu.com/imprint ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available [not found] ` <4E68BF73.2090707-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org> 2011-09-08 13:23 ` simo 2011-09-08 13:23 ` Andrew Bartlett @ 2011-09-08 13:31 ` Jeff Layton 2 siblings, 0 replies; 23+ messages in thread From: Jeff Layton @ 2011-09-08 13:31 UTC (permalink / raw) To: Martin Wilck Cc: Andrew Bartlett, linux-cifs-u79uwXL29TY76Z2rM5mHXA, samba-technical-w/Ol4Ecudpl8XjKLYN78aQ, Martin Wilck On Thu, 08 Sep 2011 15:13:23 +0200 Martin Wilck <martin.wilck-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org> wrote: > On 09/08/2011 03:01 PM, Andrew Bartlett wrote: > > > Try > > [libdefaults] > > rdns = false > > > > in your krb5.conf > > Doesn't work, sorry. Actually, it doesn't seem to make any difference in > my setup. In my scenario, cifs.upcall would be able to infer the correct > SPN with the following algorithm: > > - get the IP address using DNS > - get the "real" server FQDN using RDNS > - use "cifs/<hostname portion of the "real" FQDN>" as SPN > Another somewhat unsecure option for you then is to use the --trust-dns option to cifs.upcall, which will do basically what you describe above. Of course, the best solution would be to lobby your server admins to either fix their DNS, or use setspn.exe to set up the necessary principals in the KDC. -- Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available [not found] ` <1315322794-10725-1-git-send-email-martin.wilck-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org> 2011-09-06 16:10 ` Jeff Layton @ 2011-09-07 22:18 ` Steve French 1 sibling, 0 replies; 23+ messages in thread From: Steve French @ 2011-09-07 22:18 UTC (permalink / raw) To: Martin Wilck Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA, jlayton-eUNUBHrolfbYtjvyW6yDsg, Andrew Bartlett I'll defer to Andrew and Jeff on this one. We could articulate a compatibility issue more clearly, but presumably this is not an issue for Windows (or Samba) servers - but if there is a clear need for this for some other server let us know so we can continue the discussion. On Tue, Sep 6, 2011 at 10:26 AM, Martin Wilck <martin.wilck-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org> wrote: > This patch complements the kernel patch > "cifs: add server-provided principal name in upcall". When cifs.upcall > is started with -p, the kernel-provided principal name from the initial > negotiation with the server will be used to obtain a ticket. If this fails, > "cifs/<hostname>" and "host/hostname" fallbacks will be tried as fallback. > > This will enable kerberos-based CIFS mounts on more servers, and make > mount.cifs work more similar to smbclient. > > Note that the "-p" option must be added in the cifs.upcall line in > /etc/request-key.conf to activate this. > > Signed-off-by: Martin Wilck <martin.wilck-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org> > --- > cifs.upcall.8.in | 6 ++++++ > cifs.upcall.c | 38 +++++++++++++++++++++++++++++++------- > cifs_spnego.h | 2 +- > 3 files changed, 38 insertions(+), 8 deletions(-) > > diff --git a/cifs.upcall.8.in b/cifs.upcall.8.in > index 03842b7..7d90d1e 100644 > --- a/cifs.upcall.8.in > +++ b/cifs.upcall.8.in > @@ -52,6 +52,12 @@ Traditionally, the kernel has sent only a single uid= parameter to the upcall fo > Newer kernels send a creduid= option as well, which contains what uid it thinks actually owns the credentials that it\'s looking for\&. At mount time, this is generally set to the real uid of the user doing the mount. For multisession mounts, it's set to the fsuid of the mount user. Set this option if you want cifs.upcall to use the older uid= parameter instead of the creduid= parameter\&. > .RE > .PP > +\-\-principal\-from\-kernel|\-p > +.RS 4 > +When this option is used, the principal name obtained by the kernel from the CIFS server is used rather than a principal name derived from the host name. If this fails, the host name is used as fallback. > +This makes it possible to obtain credentials for CIFS servers using nonstandard principal names, and makes cifs mounts behave more similar to smbclient, which also uses the server-provided principal name. > +.RE > +.PP > \-\-version|\-v > .RS 4 > Print version number and exit\&. > diff --git a/cifs.upcall.c b/cifs.upcall.c > index de92092..2adac1d 100644 > --- a/cifs.upcall.c > +++ b/cifs.upcall.c > @@ -518,11 +518,13 @@ handle_krb5_mech(const char *oid, const char *principal, DATA_BLOB * secblob, > #define DKD_HAVE_PID 0x20 > #define DKD_HAVE_CREDUID 0x40 > #define DKD_HAVE_USERNAME 0x80 > +#define DKD_HAVE_PRINCIPAL 0x100 > #define DKD_MUSTHAVE_SET (DKD_HAVE_HOSTNAME|DKD_HAVE_VERSION|DKD_HAVE_SEC) > > struct decoded_args { > int ver; > char *hostname; > + char *principal; > char *ip; > char *username; > uid_t uid; > @@ -557,6 +559,21 @@ decode_key_description(const char *desc, struct decoded_args *arg) > } > retval |= DKD_HAVE_HOSTNAME; > syslog(LOG_DEBUG, "host=%s", arg->hostname); > + } else if (!strncmp(tkn, "pri=", 4)) { > + if (pos == NULL) > + len = strlen(tkn); > + else > + len = pos - tkn; > + > + len -= 4; > + SAFE_FREE(arg->principal); > + arg->principal = strndup(tkn + 4, len); > + if (arg->principal == NULL) { > + syslog(LOG_ERR, "Unable to allocate memory"); > + return 1; > + } > + retval |= DKD_HAVE_PRINCIPAL; > + syslog(LOG_DEBUG, "pri=%s", arg->principal); > } else if (!strncmp(tkn, "ip4=", 4) || !strncmp(tkn, "ip6=", 4)) { > if (pos == NULL) > len = strlen(tkn); > @@ -755,6 +772,7 @@ static void usage(void) > const struct option long_options[] = { > {"trust-dns", 0, NULL, 't'}, > {"legacy-uid", 0, NULL, 'l'}, > + {"principal-from-kernel", 0, NULL, 'p'}, > {"version", 0, NULL, 'v'}, > {NULL, 0, NULL, 0} > }; > @@ -768,7 +786,7 @@ int main(const int argc, char *const argv[]) > size_t datalen; > unsigned int have; > long rc = 1; > - int c, try_dns = 0, legacy_uid = 0; > + int c, try_dns = 0, legacy_uid = 0, use_principal = 0; > char *buf, *princ = NULL, *ccname = NULL; > char hostbuf[NI_MAXHOST], *host; > struct decoded_args arg; > @@ -781,7 +799,7 @@ int main(const int argc, char *const argv[]) > > openlog(prog, 0, LOG_DAEMON); > > - while ((c = getopt_long(argc, argv, "cltv", long_options, NULL)) != -1) { > + while ((c = getopt_long(argc, argv, "cltpv", long_options, NULL)) != -1) { > switch (c) { > case 'c': > /* legacy option -- skip it */ > @@ -792,6 +810,9 @@ int main(const int argc, char *const argv[]) > case 'l': > legacy_uid++; > break; > + case 'p': > + use_principal++; > + break; > case 'v': > printf("version: %s\n", VERSION); > goto out; > @@ -873,9 +894,16 @@ int main(const int argc, char *const argv[]) > host = arg.hostname; > > // do mech specific authorization > + oid = OID_KERBEROS5; > switch (arg.sec) { > case MS_KRB5: > + oid = OID_KERBEROS5_OLD; > case KRB5: > + if (use_principal && have & DKD_HAVE_PRINCIPAL) { > + rc = handle_krb5_mech(oid, arg.principal, &secblob, &sess_key, ccname); > + if (!rc) > + break; > + } > retry_new_hostname: > /* for "cifs/" service name + terminating 0 */ > datalen = strlen(host) + 5 + 1; > @@ -885,11 +913,6 @@ retry_new_hostname: > break; > } > > - if (arg.sec == MS_KRB5) > - oid = OID_KERBEROS5_OLD; > - else > - oid = OID_KERBEROS5; > - > /* > * try getting a cifs/ principal first and then fall back to > * getting a host/ principal if that doesn't work. > @@ -965,6 +988,7 @@ out: > data_blob_free(&sess_key); > SAFE_FREE(ccname); > SAFE_FREE(arg.hostname); > + SAFE_FREE(arg.principal); > SAFE_FREE(arg.ip); > SAFE_FREE(arg.username); > SAFE_FREE(keydata); > diff --git a/cifs_spnego.h b/cifs_spnego.h > index f8753a7..d43f965 100644 > --- a/cifs_spnego.h > +++ b/cifs_spnego.h > @@ -23,7 +23,7 @@ > #ifndef _CIFS_SPNEGO_H > #define _CIFS_SPNEGO_H > > -#define CIFS_SPNEGO_UPCALL_VERSION 2 > +#define CIFS_SPNEGO_UPCALL_VERSION 3 > > /* > * The version field should always be set to CIFS_SPNEGO_UPCALL_VERSION. > -- > 1.7.6 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-cifs" in > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- Thanks, Steve ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC/PATCH] cifs: add server-provided principal name in upcall [not found] ` <1315322512-10652-1-git-send-email-martin.wilck-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org> 2011-09-06 15:26 ` [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available Martin Wilck @ 2011-09-06 16:16 ` Jeff Layton 1 sibling, 0 replies; 23+ messages in thread From: Jeff Layton @ 2011-09-06 16:16 UTC (permalink / raw) To: Martin Wilck Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA, sfrench-eUNUBHrolfbYtjvyW6yDsg On Tue, 6 Sep 2011 17:21:52 +0200 Martin Wilck <martin.wilck-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org> wrote: > The current cifs implementation discards a principal name found > in the CIFS server's SecBlob. This patch adds the principal name > into the data used for the request_key call. Combined with a separate > cifs-utils patch, this enables cifs mounts using kerberos on servers > that use different principal names than the default cifs/<hostname> > or host/<hostname> which are tried by cifs.upcall. > > Signed-off-by: Martin Wilck <martin.wilck-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org> > --- > fs/cifs/asn1.c | 28 ++++++++++++++++++++++++---- > fs/cifs/cifs_spnego.c | 10 ++++++++++ > fs/cifs/cifs_spnego.h | 2 +- > fs/cifs/cifsglob.h | 1 + > fs/cifs/connect.c | 3 +++ > 5 files changed, 39 insertions(+), 5 deletions(-) > > diff --git a/fs/cifs/asn1.c b/fs/cifs/asn1.c > index cfd1ce3..5ab23f2 100644 > --- a/fs/cifs/asn1.c > +++ b/fs/cifs/asn1.c > @@ -369,7 +369,7 @@ static unsigned char asn1_ulong_decode(struct asn1_ctx *ctx, > *integer |= ch; > } > return 1; > -} > +} */ > > static unsigned char > asn1_octets_decode(struct asn1_ctx *ctx, > @@ -395,7 +395,7 @@ asn1_octets_decode(struct asn1_ctx *ctx, > (*len)++; > } > return 1; > -} */ > +} > > static unsigned char > asn1_subid_decode(struct asn1_ctx *ctx, unsigned long *subid) > @@ -496,9 +496,9 @@ decode_negTokenInit(unsigned char *security_blob, int length, > { > struct asn1_ctx ctx; > unsigned char *end; > - unsigned char *sequence_end; > + unsigned char *sequence_end, *principal; > unsigned long *oid = NULL; > - unsigned int cls, con, tag, oidlen, rc; > + unsigned int cls, con, tag, oidlen, rc, princlen; > > /* cifs_dump_mem(" Received SecBlob ", security_blob, length); */ > > @@ -661,6 +661,26 @@ decode_negTokenInit(unsigned char *security_blob, int length, > } > cFYI(1, "Need to call asn1_octets_decode() function for %s", > ctx.pointer); /* is this UTF-8 or ASCII? */ > + if (asn1_octets_decode(&ctx, end, &principal, &princlen) == 0) { > + cFYI(1, "Error decoding principal name exit10"); > + return 0; > + } else if (princlen == 0) { > + cFYI(1, "Empty principal name"); > + } else if (!strcmp(principal, "not_defined_in_RFC4178@please_ignore")) { > + cFYI(1, "Ignoring principal name"); > + } else { > + server->principal = kmalloc(princlen+1, GFP_ATOMIC); ^^^^^^^^^^ I don't think GFP_ATOMIC is really necessary here. > + if (server->principal != NULL) { > + memcpy(server->principal, principal, princlen); > + server->principal[princlen] = '\0'; > + cFYI(1, "Got principal: %s", server->principal); > + } else { > + kfree(principal); > + cFYI(1, "Error allocating memory exit 11"); > + return 0; > + } > + } > + kfree(principal); > decode_negtoken_exit: > return 1; > } > diff --git a/fs/cifs/cifs_spnego.c b/fs/cifs/cifs_spnego.c > index 2272fd5..356a8a6 100644 > --- a/fs/cifs/cifs_spnego.c > +++ b/fs/cifs/cifs_spnego.c > @@ -93,6 +93,9 @@ struct key_type cifs_spnego_key_type = { > /* strlen of ";pid=0x" */ > #define PID_KEY_LEN 7 > > +/* strlen of ";pri=" */ > +#define PRINC_KEY_LEN 5 > + > /* get a key struct with a SPNEGO security blob, suitable for session setup */ > struct key * > cifs_get_spnego_key(struct cifs_ses *sesInfo) > @@ -109,6 +112,8 @@ cifs_get_spnego_key(struct cifs_ses *sesInfo) > host=hostname sec=mechanism uid=0xFF user=username */ > desc_len = MAX_VER_STR_LEN + > HOST_KEY_LEN + strlen(hostname) + > + (server->principal ? > + PRINC_KEY_LEN + strlen(server->principal) : 0) + > IP_KEY_LEN + INET6_ADDRSTRLEN + > MAX_MECH_STR_LEN + > UID_KEY_LEN + (sizeof(uid_t) * 2) + > @@ -128,6 +133,11 @@ cifs_get_spnego_key(struct cifs_ses *sesInfo) > hostname); > dp = description + strlen(description); > > + if (server->principal) { > + sprintf(dp, "pri=%s;", server->principal); > + dp = description + strlen(description); > + } > + > /* add the server address */ > if (server->dstaddr.ss_family == AF_INET) > sprintf(dp, "ip4=%pI4", &sa->sin_addr); > diff --git a/fs/cifs/cifs_spnego.h b/fs/cifs/cifs_spnego.h > index 31bef9e..bae1877 100644 > --- a/fs/cifs/cifs_spnego.h > +++ b/fs/cifs/cifs_spnego.h > @@ -23,7 +23,7 @@ > #ifndef _CIFS_SPNEGO_H > #define _CIFS_SPNEGO_H > > -#define CIFS_SPNEGO_UPCALL_VERSION 2 > +#define CIFS_SPNEGO_UPCALL_VERSION 3 > ^^^^^^^^ Again, no need to rev the upcall format. > /* > * The version field should always be set to CIFS_SPNEGO_UPCALL_VERSION. > diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h > index 95dad9d..86de4fb 100644 > --- a/fs/cifs/cifsglob.h > +++ b/fs/cifs/cifsglob.h > @@ -238,6 +238,7 @@ struct TCP_Server_Info { > char server_RFC1001_name[RFC1001_NAME_LEN_WITH_NULL]; > enum statusEnum tcpStatus; /* what we think the status is */ > char *hostname; /* hostname portion of UNC string */ > + char *principal; > struct socket *ssocket; > struct sockaddr_storage dstaddr; > struct sockaddr_storage srcaddr; /* locally bind to this IP */ > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c > index f4af4cc..6fdca9f 100644 > --- a/fs/cifs/connect.c > +++ b/fs/cifs/connect.c > @@ -618,6 +618,8 @@ static void clean_demultiplex_info(struct TCP_Server_Info *server) > } > > kfree(server->hostname); > + if (server->principal != NULL) > + kfree(server->principal); It's safe to kfree a NULL pointer. > kfree(server); > > length = atomic_dec_return(&tcpSesAllocCount); > @@ -1780,6 +1782,7 @@ cifs_get_tcp_session(struct smb_vol *volume_info) > rc = PTR_ERR(tcp_ses->hostname); > goto out_err_crypto_release; > } > + tcp_ses->principal = NULL; > > tcp_ses->noblocksnd = volume_info->noblocksnd; > tcp_ses->noautotune = volume_info->noautotune; -- Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2011-09-13 11:01 UTC | newest] Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-09-06 15:21 [RFC/PATCH] cifs: add server-provided principal name in upcall Martin Wilck [not found] ` <1315322512-10652-1-git-send-email-martin.wilck-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org> 2011-09-06 15:26 ` [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available Martin Wilck [not found] ` <1315322794-10725-1-git-send-email-martin.wilck-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org> 2011-09-06 16:10 ` Jeff Layton [not found] ` <4E673D6F.90606@ts.fujitsu.com> 2011-09-07 13:03 ` Jeff Layton 2011-09-07 21:42 ` Andrew Bartlett 2011-09-08 7:23 ` Martin Wilck [not found] ` <4E686D69.9090503-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org> 2011-09-08 7:39 ` Andrew Bartlett 2011-09-08 12:53 ` Martin Wilck [not found] ` <4E68BACD.2020403-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org> 2011-09-08 12:59 ` simo 2011-09-08 13:01 ` Andrew Bartlett 2011-09-08 13:13 ` Martin Wilck [not found] ` <4E68BF73.2090707-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org> 2011-09-08 13:23 ` simo 2011-09-08 13:23 ` Andrew Bartlett 2011-09-08 14:54 ` Jeff Layton [not found] ` <4E68EEAE.2090102@ts.fujitsu.com> [not found] ` <4E68EEAE.2090102-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org> 2011-09-09 13:37 ` Jeff Layton 2011-09-12 9:01 ` Martin Wilck [not found] ` <4E6DCA86.8020707-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org> 2011-09-12 13:41 ` Jeff Layton [not found] ` <20110912094114.4e7f2b8e-4QP7MXygkU+dMjc06nkz3ljfA9RmPOcC@public.gmane.org> 2011-09-12 14:00 ` simo 2011-09-12 23:23 ` Andrew Bartlett 2011-09-13 11:01 ` Martin Wilck 2011-09-08 13:31 ` Jeff Layton 2011-09-07 22:18 ` Steve French 2011-09-06 16:16 ` [RFC/PATCH] cifs: add server-provided principal name in upcall Jeff Layton
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.