* [PATCH v2 1/4] keys: add a "secret" key type
[not found] ` <1325878247-12030-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
@ 2012-01-06 19:30 ` Jeff Layton
[not found] ` <1325878247-12030-2-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2012-01-06 19:30 ` [PATCH v2 2/4] cifs: sanitize username handling Jeff Layton
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Jeff Layton @ 2012-01-06 19:30 UTC (permalink / raw)
To: smfrench-Re5JQEeQqe8AvxtiuMwx3w
Cc: dhowells-H+wXaHxf7aLQT0dZR+AlfA,
linux-cifs-u79uwXL29TY76Z2rM5mHXA,
keyrings-6DNke4IJHB0gsBAKwltoeQ
For CIFS, we want to be able to store NTLM credentials (aka username
and password) in the keyring. We do not, however want to allow users
to fetch those keys back out of the keyring since that would be a
security risk.
Unfortunately, due to the nuances of key permission bits, it's not
possible to do this. We need to grant search permissions so the kernel
can find these keys, but that also implies permissions to read the
payload.
Resolve this by adding a new key_type. This key type is essentially
the same as key_type_user, but does not define a .read op. This
prevents the payload from ever being visible from userspace.
Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
include/keys/user-type.h | 3 ++-
security/keys/internal.h | 1 +
security/keys/key.c | 1 +
security/keys/user_defined.c | 17 +++++++++++++++++
4 files changed, 21 insertions(+), 1 deletions(-)
diff --git a/include/keys/user-type.h b/include/keys/user-type.h
index c37c342..41b5515 100644
--- a/include/keys/user-type.h
+++ b/include/keys/user-type.h
@@ -17,7 +17,7 @@
/*****************************************************************************/
/*
- * the payload for a key of type "user"
+ * the payload for a key of type "user" or "secret"
* - once filled in and attached to a key:
* - the payload struct is invariant may not be changed, only replaced
* - the payload must be read with RCU procedures or with the key semaphore
@@ -33,6 +33,7 @@ struct user_key_payload {
};
extern struct key_type key_type_user;
+extern struct key_type key_type_secret;
extern int user_instantiate(struct key *key, const void *data, size_t datalen);
extern int user_update(struct key *key, const void *data, size_t datalen);
diff --git a/security/keys/internal.h b/security/keys/internal.h
index c7a7cae..2784e07 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -33,6 +33,7 @@
extern struct key_type key_type_dead;
extern struct key_type key_type_user;
+extern struct key_type key_type_secret;
/*****************************************************************************/
/*
diff --git a/security/keys/key.c b/security/keys/key.c
index 4414abd..3d1d79d 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -996,6 +996,7 @@ void __init key_init(void)
list_add_tail(&key_type_keyring.link, &key_types_list);
list_add_tail(&key_type_dead.link, &key_types_list);
list_add_tail(&key_type_user.link, &key_types_list);
+ list_add_tail(&key_type_secret.link, &key_types_list);
/* record the root user tracking */
rb_link_node(&root_key_user.node,
diff --git a/security/keys/user_defined.c b/security/keys/user_defined.c
index 69ff52c..e25782f 100644
--- a/security/keys/user_defined.c
+++ b/security/keys/user_defined.c
@@ -36,6 +36,23 @@ struct key_type key_type_user = {
EXPORT_SYMBOL_GPL(key_type_user);
/*
+ * This key type is essentially the same as key_type_user, but it does
+ * not define a .read op. This is suitable for storing information in
+ * the keyring that you do not want to be readable from userspace. For
+ * instance, passwords or secret encryption keys.
+ */
+struct key_type key_type_secret = {
+ .name = "secret",
+ .instantiate = user_instantiate,
+ .update = user_update,
+ .match = user_match,
+ .revoke = user_revoke,
+ .destroy = user_destroy,
+ .describe = user_describe,
+};
+EXPORT_SYMBOL_GPL(key_type_secret);
+
+/*
* instantiate a user defined key
*/
int user_instantiate(struct key *key, const void *data, size_t datalen)
--
1.7.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v2 2/4] cifs: sanitize username handling
[not found] ` <1325878247-12030-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2012-01-06 19:30 ` [PATCH v2 1/4] keys: add a "secret" key type Jeff Layton
@ 2012-01-06 19:30 ` Jeff Layton
2012-01-06 19:30 ` [PATCH v2 3/4] cifs: fetch credentials out of keyring for non-krb5 auth multiuser mounts Jeff Layton
2012-01-06 19:30 ` [PATCH v2 4/4] cifs: warn about impending deprecation of legacy MultiuserMount code Jeff Layton
3 siblings, 0 replies; 8+ messages in thread
From: Jeff Layton @ 2012-01-06 19:30 UTC (permalink / raw)
To: smfrench-Re5JQEeQqe8AvxtiuMwx3w
Cc: dhowells-H+wXaHxf7aLQT0dZR+AlfA,
linux-cifs-u79uwXL29TY76Z2rM5mHXA,
keyrings-6DNke4IJHB0gsBAKwltoeQ
Currently, it's not very clear whether you're allowed to have a NULL
vol->username or ses->user_name. Some places check for it and some don't.
Make it clear that a NULL pointer is OK in these fields, and ensure that
all the callers check for that.
Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
fs/cifs/cifs_spnego.c | 10 +++++++---
fs/cifs/cifsencrypt.c | 11 ++++++++---
fs/cifs/connect.c | 19 ++++++++++++-------
3 files changed, 27 insertions(+), 13 deletions(-)
diff --git a/fs/cifs/cifs_spnego.c b/fs/cifs/cifs_spnego.c
index 2272fd5..e622863 100644
--- a/fs/cifs/cifs_spnego.c
+++ b/fs/cifs/cifs_spnego.c
@@ -113,9 +113,11 @@ cifs_get_spnego_key(struct cifs_ses *sesInfo)
MAX_MECH_STR_LEN +
UID_KEY_LEN + (sizeof(uid_t) * 2) +
CREDUID_KEY_LEN + (sizeof(uid_t) * 2) +
- USER_KEY_LEN + strlen(sesInfo->user_name) +
PID_KEY_LEN + (sizeof(pid_t) * 2) + 1;
+ if (sesInfo->user_name)
+ desc_len += USER_KEY_LEN + strlen(sesInfo->user_name);
+
spnego_key = ERR_PTR(-ENOMEM);
description = kzalloc(desc_len, GFP_KERNEL);
if (description == NULL)
@@ -152,8 +154,10 @@ cifs_get_spnego_key(struct cifs_ses *sesInfo)
dp = description + strlen(description);
sprintf(dp, ";creduid=0x%x", sesInfo->cred_uid);
- dp = description + strlen(description);
- sprintf(dp, ";user=%s", sesInfo->user_name);
+ if (sesInfo->user_name) {
+ dp = description + strlen(description);
+ sprintf(dp, ";user=%s", sesInfo->user_name);
+ }
dp = description + strlen(description);
sprintf(dp, ";pid=0x%x", current->pid);
diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c
index 5d9b9ac..bce99e6 100644
--- a/fs/cifs/cifsencrypt.c
+++ b/fs/cifs/cifsencrypt.c
@@ -420,15 +420,20 @@ static int calc_ntlmv2_hash(struct cifs_ses *ses, char *ntlmv2_hash,
}
/* convert ses->user_name to unicode and uppercase */
- len = strlen(ses->user_name);
+ len = ses->user_name ? strlen(ses->user_name) : 0;
user = kmalloc(2 + (len * 2), GFP_KERNEL);
if (user == NULL) {
cERROR(1, "calc_ntlmv2_hash: user mem alloc failure\n");
rc = -ENOMEM;
return rc;
}
- len = cifs_strtoUCS((__le16 *)user, ses->user_name, len, nls_cp);
- UniStrupr(user);
+
+ if (len) {
+ len = cifs_strtoUCS((__le16 *)user, ses->user_name, len, nls_cp);
+ UniStrupr(user);
+ } else {
+ memset(user, '\0', 2);
+ }
rc = crypto_shash_update(&ses->server->secmech.sdeschmacmd5->shash,
(char *)user, 2 * len);
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 8dbb6ff..5e96e60 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -1997,10 +1997,16 @@ static int match_session(struct cifs_ses *ses, struct smb_vol *vol)
return 0;
break;
default:
+ /* NULL username means anonymous session */
+ if (ses->user_name == NULL) {
+ if (!vol->nullauth)
+ return 0;
+ break;
+ }
+
/* anything else takes username/password */
- if (ses->user_name == NULL)
- return 0;
- if (strncmp(ses->user_name, vol->username,
+ if (strncmp(ses->user_name,
+ vol->username ? vol->username: "",
MAX_USERNAME_SIZE))
return 0;
if (strlen(vol->username) != 0 &&
@@ -3152,10 +3158,9 @@ cifs_setup_volume_info(struct smb_vol *volume_info, char *mount_data,
return -EINVAL;
if (volume_info->nullauth) {
- cFYI(1, "null user");
- volume_info->username = kzalloc(1, GFP_KERNEL);
- if (volume_info->username == NULL)
- return -ENOMEM;
+ cFYI(1, "Anonymous login");
+ kfree(volume_info->username);
+ volume_info->username = NULL;
} else if (volume_info->username) {
/* BB fixme parse for domain name here */
cFYI(1, "Username: %s", volume_info->username);
--
1.7.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v2 3/4] cifs: fetch credentials out of keyring for non-krb5 auth multiuser mounts
[not found] ` <1325878247-12030-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2012-01-06 19:30 ` [PATCH v2 1/4] keys: add a "secret" key type Jeff Layton
2012-01-06 19:30 ` [PATCH v2 2/4] cifs: sanitize username handling Jeff Layton
@ 2012-01-06 19:30 ` Jeff Layton
2012-01-06 19:30 ` [PATCH v2 4/4] cifs: warn about impending deprecation of legacy MultiuserMount code Jeff Layton
3 siblings, 0 replies; 8+ messages in thread
From: Jeff Layton @ 2012-01-06 19:30 UTC (permalink / raw)
To: smfrench-Re5JQEeQqe8AvxtiuMwx3w
Cc: dhowells-H+wXaHxf7aLQT0dZR+AlfA,
linux-cifs-u79uwXL29TY76Z2rM5mHXA,
keyrings-6DNke4IJHB0gsBAKwltoeQ
Fix up multiuser mounts to set the secType and set the username and
password from the key payload in the vol info for non-krb5 auth types.
Look for a key of type "secret" with a description of
"cifs:a:<server address>" or "cifs:d:<domainname>". If that's found,
then scrape the username and password out of the key payload and use
that to create a new user session.
Finally, don't have the code enforce krb5 auth on multiuser mounts,
but do require a kernel with keys support.
Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
fs/cifs/connect.c | 175 ++++++++++++++++++++++++++++++++++++++++++++++++++---
1 files changed, 165 insertions(+), 10 deletions(-)
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 5e96e60..73535ef 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -38,6 +38,7 @@
#include <asm/processor.h>
#include <linux/inet.h>
#include <linux/module.h>
+#include <keys/user-type.h>
#include <net/ipv6.h>
#include "cifspdu.h"
#include "cifsglob.h"
@@ -1594,11 +1595,14 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
}
}
- if (vol->multiuser && !(vol->secFlg & CIFSSEC_MAY_KRB5)) {
- cERROR(1, "Multiuser mounts currently require krb5 "
- "authentication!");
+#ifndef CONFIG_KEYS
+ /* Muliuser mounts require CONFIG_KEYS support */
+ if (vol->multiuser) {
+ cERROR(1, "Multiuser mounts require kernels with "
+ "CONFIG_KEYS enabled.");
goto cifs_parse_mount_err;
}
+#endif
if (vol->UNCip == NULL)
vol->UNCip = &vol->UNC[2];
@@ -2061,6 +2065,132 @@ cifs_put_smb_ses(struct cifs_ses *ses)
cifs_put_tcp_session(server);
}
+#ifdef CONFIG_KEYS
+
+/* strlen("cifs:a:") + INET6_ADDRSTRLEN + 1 */
+#define CIFSCREDS_DESC_SIZE (7 + INET6_ADDRSTRLEN + 1)
+
+/* Populate username and pw fields from keyring if possible */
+static int
+cifs_set_cifscreds(struct smb_vol *vol, struct cifs_ses *ses)
+{
+ int rc = 0;
+ char *desc, *delim, *payload;
+ ssize_t len;
+ struct key *key;
+ struct TCP_Server_Info *server = ses->server;
+ struct sockaddr_in *sa;
+ struct sockaddr_in6 *sa6;
+ struct user_key_payload *upayload;
+
+ desc = kmalloc(CIFSCREDS_DESC_SIZE, GFP_KERNEL);
+ if (!desc)
+ return -ENOMEM;
+
+ /* try to find an address key first */
+ switch (server->dstaddr.ss_family) {
+ case AF_INET:
+ sa = (struct sockaddr_in *)&server->dstaddr;
+ sprintf(desc, "cifs:a:%pI4", &sa->sin_addr.s_addr);
+ break;
+ case AF_INET6:
+ sa6 = (struct sockaddr_in6 *)&server->dstaddr;
+ sprintf(desc, "cifs:a:%pI6c", &sa6->sin6_addr.s6_addr);
+ break;
+ default:
+ cFYI(1, "Bad ss_family (%hu)", server->dstaddr.ss_family);
+ rc = -EINVAL;
+ goto out_err;
+ }
+
+ cFYI(1, "%s: desc=%s", __func__, desc);
+ key = request_key(&key_type_secret, desc, "");
+ if (IS_ERR(key)) {
+ if (!ses->domainName) {
+ cFYI(1, "domainName is NULL");
+ rc = PTR_ERR(key);
+ goto out_err;
+ }
+
+ /* didn't work, try to find a domain key */
+ sprintf(desc, "cifs:d:%s", ses->domainName);
+ cFYI(1, "%s: desc=%s", __func__, desc);
+ key = request_key(&key_type_secret, desc, "");
+ if (IS_ERR(key)) {
+ rc = PTR_ERR(key);
+ goto out_err;
+ }
+ }
+
+ down_read(&key->sem);
+ upayload = key->payload.data;
+ if (IS_ERR_OR_NULL(upayload)) {
+ rc = PTR_ERR(key);
+ goto out_key_put;
+ }
+
+ /* find first : in payload */
+ payload = (char *)upayload->data;
+ delim = strnchr(payload, upayload->datalen, ':');
+ cFYI(1, "payload=%s", payload);
+ if (!delim) {
+ cFYI(1, "Unable to find ':' in payload (datalen=%d)",
+ upayload->datalen);
+ rc = -EINVAL;
+ goto out_key_put;
+ }
+
+ len = delim - payload;
+ if (len > MAX_USERNAME_SIZE || len <= 0) {
+ cFYI(1, "Bad value from username search (len=%ld)", len);
+ rc = -EINVAL;
+ goto out_key_put;
+ }
+
+ vol->username = kstrndup(payload, len, GFP_KERNEL);
+ if (!vol->username) {
+ cFYI(1, "Unable to allocate %ld bytes for username", len);
+ rc = -ENOMEM;
+ goto out_key_put;
+ }
+ cFYI(1, "%s: username=%s", __func__, vol->username);
+
+ len = key->datalen - (len + 1);
+ if (len > MAX_PASSWORD_SIZE || len <= 0) {
+ cFYI(1, "Bad len for password search (len=%ld)", len);
+ rc = -EINVAL;
+ kfree(vol->username);
+ vol->username = NULL;
+ goto out_key_put;
+ }
+
+ ++delim;
+ vol->password = kstrndup(delim, len, GFP_KERNEL);
+ if (!vol->password) {
+ cFYI(1, "Unable to allocate %ld bytes for password", len);
+ rc = -ENOMEM;
+ kfree(vol->username);
+ vol->username = NULL;
+ goto out_key_put;
+ }
+
+out_key_put:
+ up_read(&key->sem);
+ key_put(key);
+out_err:
+ kfree(desc);
+ cFYI(1, "%s: returning %d", __func__, rc);
+ return rc;
+}
+#else /* ! CONFIG_KEYS */
+static inline int
+cifs_set_cifscreds(struct smb_vol *vol __attribute__((unused)),
+ struct cifs_ses *ses __attribute__((unused)))
+{
+ return -ENOSYS;
+}
+#endif /* CONFIG_KEYS */
+
static bool warned_on_ntlm; /* globals init to false automatically */
static struct cifs_ses *
@@ -3678,16 +3808,38 @@ int cifs_setup_session(unsigned int xid, struct cifs_ses *ses,
return rc;
}
+static int
+cifs_set_vol_auth(struct smb_vol *vol, struct cifs_ses *ses)
+{
+ switch(ses->server->secType) {
+ case Kerberos:
+ vol->secFlg = CIFSSEC_MUST_KRB5;
+ return 0;
+ case NTLMv2:
+ vol->secFlg = CIFSSEC_MUST_NTLMV2;
+ break;
+ case NTLM:
+ vol->secFlg = CIFSSEC_MUST_NTLM;
+ break;
+ case RawNTLMSSP:
+ vol->secFlg = CIFSSEC_MUST_NTLMSSP;
+ break;
+ case LANMAN:
+ vol->secFlg = CIFSSEC_MUST_LANMAN;
+ break;
+ }
+
+ return cifs_set_cifscreds(vol, ses);
+}
+
static struct cifs_tcon *
cifs_construct_tcon(struct cifs_sb_info *cifs_sb, uid_t fsuid)
{
+ int rc;
struct cifs_tcon *master_tcon = cifs_sb_master_tcon(cifs_sb);
struct cifs_ses *ses;
struct cifs_tcon *tcon = NULL;
struct smb_vol *vol_info;
- char username[28]; /* big enough for "krb50x" + hex of ULONG_MAX 6+16 */
- /* We used to have this as MAX_USERNAME which is */
- /* way too big now (256 instead of 32) */
vol_info = kzalloc(sizeof(*vol_info), GFP_KERNEL);
if (vol_info == NULL) {
@@ -3695,8 +3847,6 @@ cifs_construct_tcon(struct cifs_sb_info *cifs_sb, uid_t fsuid)
goto out;
}
- snprintf(username, sizeof(username), "krb50x%x", fsuid);
- vol_info->username = username;
vol_info->local_nls = cifs_sb->local_nls;
vol_info->linux_uid = fsuid;
vol_info->cred_uid = fsuid;
@@ -3706,8 +3856,11 @@ cifs_construct_tcon(struct cifs_sb_info *cifs_sb, uid_t fsuid)
vol_info->local_lease = master_tcon->local_lease;
vol_info->no_linux_ext = !master_tcon->unix_ext;
- /* FIXME: allow for other secFlg settings */
- vol_info->secFlg = CIFSSEC_MUST_KRB5;
+ rc = cifs_set_vol_auth(vol_info, master_tcon->ses);
+ if (rc) {
+ tcon = ERR_PTR(rc);
+ goto out;
+ }
/* get a reference for the same TCP session */
spin_lock(&cifs_tcp_ses_lock);
@@ -3730,6 +3883,8 @@ cifs_construct_tcon(struct cifs_sb_info *cifs_sb, uid_t fsuid)
if (ses->capabilities & CAP_UNIX)
reset_cifs_unix_caps(0, tcon, NULL, vol_info);
out:
+ kfree(vol_info->username);
+ kfree(vol_info->password);
kfree(vol_info);
return tcon;
--
1.7.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v2 4/4] cifs: warn about impending deprecation of legacy MultiuserMount code
[not found] ` <1325878247-12030-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
` (2 preceding siblings ...)
2012-01-06 19:30 ` [PATCH v2 3/4] cifs: fetch credentials out of keyring for non-krb5 auth multiuser mounts Jeff Layton
@ 2012-01-06 19:30 ` Jeff Layton
3 siblings, 0 replies; 8+ messages in thread
From: Jeff Layton @ 2012-01-06 19:30 UTC (permalink / raw)
To: smfrench-Re5JQEeQqe8AvxtiuMwx3w
Cc: dhowells-H+wXaHxf7aLQT0dZR+AlfA,
linux-cifs-u79uwXL29TY76Z2rM5mHXA,
keyrings-6DNke4IJHB0gsBAKwltoeQ
We'll allow a grace period of 2 releases (3.3 and 3.4) and then remove
the legacy code in 3.5.
Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
fs/cifs/cifs_debug.c | 11 ++++++++++-
1 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/fs/cifs/cifs_debug.c b/fs/cifs/cifs_debug.c
index 84e8c07..24b3dfc 100644
--- a/fs/cifs/cifs_debug.c
+++ b/fs/cifs/cifs_debug.c
@@ -676,14 +676,23 @@ static ssize_t cifs_multiuser_mount_proc_write(struct file *file,
{
char c;
int rc;
+ static bool warned;
rc = get_user(c, buffer);
if (rc)
return rc;
if (c == '0' || c == 'n' || c == 'N')
multiuser_mount = 0;
- else if (c == '1' || c == 'y' || c == 'Y')
+ else if (c == '1' || c == 'y' || c == 'Y') {
multiuser_mount = 1;
+ if (!warned) {
+ warned = true;
+ printk(KERN_WARNING "CIFS VFS: The legacy multiuser "
+ "mount code is scheduled to be deprecated in "
+ "3.5. Please switch to using the multiuser "
+ "mount option.");
+ }
+ }
return count;
}
--
1.7.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread