[RFC/PATCH] cifs: add server-provided principal name in upcall

[RFC/PATCH] cifs: add server-provided principal name in upcall

[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

[RFC/PATCH] cifs.upcall: use kernel.provided principal name if available

[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

Re: [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available

[Jeff Layton]

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>

Re: [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available

[Jeff Layton]

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>

Re: [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available

[Andrew Bartlett]

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

Re: [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available

[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

Re: [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available

[Andrew Bartlett]

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

Re: [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available

[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

Re: [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available

[simo]

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>

Re: [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available

[Andrew Bartlett]

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

Re: [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available

[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

Re: [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available

[simo]

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>

Re: [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available

[Andrew Bartlett]

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

Re: [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available

[Jeff Layton]

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>

Re: [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available

[Jeff Layton]

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>

Re: [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available

[Martin Wilck]

> 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

Re: [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available

[Jeff Layton]

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>

Re: [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available

[simo]

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>

Re: [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available

[Andrew Bartlett]

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

Re: [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available

[Martin Wilck]

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

Re: [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available

[Jeff Layton]

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>

Re: [RFC/PATCH] cifs.upcall: use kernel.provided principal name if available

[Steve French]

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

Re: [RFC/PATCH] cifs: add server-provided principal name in upcall

[Jeff Layton]

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>