[PATCH v1 0/2] Change CIFS_FULL_KEY_DUMP ioctl to return variable size keys

[PATCH v1 0/2] Change CIFS_FULL_KEY_DUMP ioctl to return variable size keys

[Aurélien Aptel]

From: Aurelien Aptel <aaptel@suse.com>

This patchset changes the CIFS_FULL_KEY_DUMP ioctl to return variable
size keys to userspace while keeping the same ioctl number.

This version of the ioctl should be future proof if we ever add more
cipher types or bigger keys.

This also fixes the build error for ARM related to get_user() being
undefined.

I have tested this for AES-128 and AES-256.

I have a separate patch for the smbinfo utility to make use of this
new ioctl that I will send in a separate thread.

Aurelien Aptel (2):
  cifs: set server->cipher_type to AES-128-CCM for SMB3.0
  cifs: change format of CIFS_FULL_KEY_DUMP ioctl

 fs/cifs/cifs_ioctl.h |  25 ++++++--
 fs/cifs/cifspdu.h    |   3 +-
 fs/cifs/ioctl.c      | 143 +++++++++++++++++++++++++++++++------------
 fs/cifs/smb2pdu.c    |   7 +++
 4 files changed, 133 insertions(+), 45 deletions(-)

-- 
2.31.1

[PATCH v1 1/2] cifs: set server->cipher_type to AES-128-CCM for SMB3.0

[Aurélien Aptel]

From: Aurelien Aptel <aaptel@suse.com>

SMB3.0 doesn't have encryption negotiate context but simply uses
the SMB2_GLOBAL_CAP_ENCRYPTION flag.

When that flag is present in the neg response cifs.ko uses AES-128-CCM
which is the only cipher available in this context.

cipher_type was set to the server cipher only when parsing encryption
negotiate context (SMB3.1.1).

For SMB3.0 it was set to 0. This means cipher_type value can be 0 or 1
for AES-128-CCM.

Fix this by checking for SMB3.0 and encryption capability and setting
cipher_type appropriately.

Signed-off-by: Aurelien Aptel <aaptel@suse.com>
---
 fs/cifs/smb2pdu.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 9f24eb88297a..c205f93e0a10 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -958,6 +958,13 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
 	/* Internal types */
 	server->capabilities |= SMB2_NT_FIND | SMB2_LARGE_FILES;
 
+	/*
+	 * SMB3.0 supports only 1 cipher and doesn't have a encryption neg context
+	 * Set the cipher type manually.
+	 */
+	if (server->dialect == SMB30_PROT_ID && (server->capabilities & SMB2_GLOBAL_CAP_ENCRYPTION))
+		server->cipher_type = SMB2_ENCRYPTION_AES128_CCM;
+
 	security_blob = smb2_get_data_area_len(&blob_offset, &blob_length,
 					       (struct smb2_sync_hdr *)rsp);
 	/*
-- 
2.31.1

[PATCH v1 2/2] cifs: change format of CIFS_FULL_KEY_DUMP ioctl

[Aurélien Aptel]

From: Aurelien Aptel <aaptel@suse.com>

Make CIFS_FULL_KEY_DUMP ioctl able to return variable-length keys.

* userspace needs to pass the struct size along with optional
  session_id and some space at the end to store keys
* if there is enough space kernel returns keys in the extra space and
  sets the length of each key via xyz_key_length fields

This also fixes the build error for get_user() on ARM.

Sample program:

	#include <stdlib.h>
	#include <stdio.h>
	#include <stdint.h>
	#include <sys/fcntl.h>
	#include <sys/ioctl.h>

	struct smb3_full_key_debug_info {
	        uint32_t   in_size;
	        uint64_t   session_id;
	        uint16_t   cipher_type;
	        uint8_t    session_key_length;
	        uint8_t    server_in_key_length;
	        uint8_t    server_out_key_length;
	        uint8_t    data[];
	        /*
	         * return this struct with the keys appended at the end:
	         * uint8_t session_key[session_key_length];
	         * uint8_t server_in_key[server_in_key_length];
	         * uint8_t server_out_key[server_out_key_length];
	         */
	} __attribute__((packed));

	#define CIFS_IOCTL_MAGIC 0xCF
	#define CIFS_DUMP_FULL_KEY _IOWR(CIFS_IOCTL_MAGIC, 10, struct smb3_full_key_debug_info)

	void dump(const void *p, size_t len) {
	        const char *hex = "0123456789ABCDEF";
	        const uint8_t *b = p;
	        for (int i = 0; i < len; i++)
	                printf("%c%c ", hex[(b[i]>>4)&0xf], hex[b[i]&0xf]);
	        putchar('\n');
	}

	int main(int argc, char **argv)
	{
	        struct smb3_full_key_debug_info *keys;
	        uint8_t buf[sizeof(*keys)+1024] = {0};
	        size_t off = 0;
	        int fd, rc;

	        keys = (struct smb3_full_key_debug_info *)&buf;
	        keys->in_size = sizeof(buf);

	        fd = open(argv[1], O_RDONLY);
	        if (fd < 0)
	                perror("open"), exit(1);

	        rc = ioctl(fd, CIFS_DUMP_FULL_KEY, keys);
	        if (rc < 0)
	                perror("ioctl"), exit(1);

	        printf("SessionId      ");
	        dump(&keys->session_id, 8);
	        printf("Cipher         %04x\n", keys->cipher_type);

	        printf("SessionKey     ");
	        dump(keys->data+off, keys->session_key_length);
	        off += keys->session_key_length;

	        printf("ServerIn Key   ");
	        dump(keys->data+off, keys->server_in_key_length);
	        off += keys->server_in_key_length;

	        printf("ServerOut Key  ");
	        dump(keys->data+off, keys->server_out_key_length);

	        return 0;
	}

Usage:

	$ gcc -o dumpkeys dumpkeys.c

Against Windows Server 2020 preview (with AES-256-GCM support):

	# mount.cifs //$ip/test /mnt -o "username=administrator,password=foo,vers=3.0,seal"
	# ./dumpkeys /mnt/somefile
	SessionId      0D 00 00 00 00 0C 00 00
	Cipher         0002
	SessionKey     AB CD CC 0D E4 15 05 0C 6F 3C 92 90 19 F3 0D 25
	ServerIn Key   73 C6 6A C8 6B 08 CF A2 CB 8E A5 7D 10 D1 5B DC
	ServerOut Key  6D 7E 2B A1 71 9D D7 2B 94 7B BA C4 F0 A5 A4 F8
	# umount /mnt

	With 256 bit keys:

	# echo 1 > /sys/module/cifs/parameters/require_gcm_256
	# mount.cifs //$ip/test /mnt -o "username=administrator,password=foo,vers=3.11,seal"
	# ./dumpkeys /mnt/somefile
	SessionId      09 00 00 00 00 0C 00 00
	Cipher         0004
	SessionKey     93 F5 82 3B 2F B7 2A 50 0B B9 BA 26 FB 8C 8B 03
	ServerIn Key   6C 6A 89 B2 CB 7B 78 E8 04 93 37 DA 22 53 47 DF B3 2C 5F 02 26 70 43 DB 8D 33 7B DC 66 D3 75 A9
	ServerOut Key  04 11 AA D7 52 C7 A8 0F ED E3 93 3A 65 FE 03 AD 3F 63 03 01 2B C0 1B D7 D7 E5 52 19 7F CC 46 B4

Signed-off-by: Aurelien Aptel <aaptel@suse.com>
---
 fs/cifs/cifs_ioctl.h |  25 ++++++--
 fs/cifs/cifspdu.h    |   3 +-
 fs/cifs/ioctl.c      | 143 +++++++++++++++++++++++++++++++------------
 3 files changed, 126 insertions(+), 45 deletions(-)

diff --git a/fs/cifs/cifs_ioctl.h b/fs/cifs/cifs_ioctl.h
index 4a97fe12006b..4b3d33b2838d 100644
--- a/fs/cifs/cifs_ioctl.h
+++ b/fs/cifs/cifs_ioctl.h
@@ -72,15 +72,28 @@ struct smb3_key_debug_info {
 } __packed;
 
 /*
- * Dump full key (32 byte encrypt/decrypt keys instead of 16 bytes)
- * is needed if GCM256 (stronger encryption) negotiated
+ * Dump variable-sized keys
  */
 struct smb3_full_key_debug_info {
-	__u64	Suid;
+	/* INPUT: size of userspace buffer */
+	__u32   in_size;
+
+	/*
+	 * INPUT: 0 for current user, otherwise session to dump
+	 * OUTPUT: session id that was dumped
+	 */
+	__u64	session_id;
 	__u16	cipher_type;
-	__u8	auth_key[16]; /* SMB2_NTLMV2_SESSKEY_SIZE */
-	__u8	smb3encryptionkey[32]; /* SMB3_ENC_DEC_KEY_SIZE */
-	__u8	smb3decryptionkey[32]; /* SMB3_ENC_DEC_KEY_SIZE */
+	__u8    session_key_length;
+	__u8    server_in_key_length;
+	__u8    server_out_key_length;
+	__u8    data[];
+	/*
+	 * return this struct with the keys appended at the end:
+	 * __u8 session_key[session_key_length];
+	 * __u8 server_in_key[server_in_key_length];
+	 * __u8 server_out_key[server_out_key_length];
+	*/
 } __packed;
 
 struct smb3_notify {
diff --git a/fs/cifs/cifspdu.h b/fs/cifs/cifspdu.h
index b53a87db282f..554d64fe171e 100644
--- a/fs/cifs/cifspdu.h
+++ b/fs/cifs/cifspdu.h
@@ -148,7 +148,8 @@
 #define SMB3_SIGN_KEY_SIZE (16)
 
 /*
- * Size of the smb3 encryption/decryption keys
+ * Size of the smb3 encryption/decryption key storage.
+ * This size is big enough to store any cipher key types.
  */
 #define SMB3_ENC_DEC_KEY_SIZE (32)
 
diff --git a/fs/cifs/ioctl.c b/fs/cifs/ioctl.c
index 28ec8d7c521a..87b2f1309e6f 100644
--- a/fs/cifs/ioctl.c
+++ b/fs/cifs/ioctl.c
@@ -33,6 +33,7 @@
 #include "cifsfs.h"
 #include "cifs_ioctl.h"
 #include "smb2proto.h"
+#include "smb2glob.h"
 #include <linux/btrfs.h>
 
 static long cifs_ioctl_query_info(unsigned int xid, struct file *filep,
@@ -214,48 +215,112 @@ static int cifs_shutdown(struct super_block *sb, unsigned long arg)
 	return 0;
 }
 
-static int cifs_dump_full_key(struct cifs_tcon *tcon, unsigned long arg)
+static int cifs_dump_full_key(struct cifs_tcon *tcon, struct smb3_full_key_debug_info __user* in)
 {
-	struct smb3_full_key_debug_info pfull_key_inf;
-	__u64 suid;
-	struct list_head *tmp;
+	struct smb3_full_key_debug_info out;
 	struct cifs_ses *ses;
+	int rc = 0;
 	bool found = false;
+	u8 __user *end;
 
-	if (!smb3_encryption_required(tcon))
-		return -EOPNOTSUPP;
+	if (!smb3_encryption_required(tcon)) {
+		rc = -EOPNOTSUPP;
+		goto out;
+	}
+
+	/* copy user input into our output buffer */
+	if (copy_from_user(&out, in, sizeof(out))) {
+		rc = -EINVAL;
+		goto out;
+	}
+
+	if (!out.session_id) {
+		/* if ses id is 0, use current user session */
+		ses = tcon->ses;
+	} else {
+		/* otherwise if a session id is given, look for it in all our sessions */
+		struct cifs_ses *ses_it = NULL;
+		struct TCP_Server_Info *server_it = NULL;
 
-	ses = tcon->ses; /* default to user id for current user */
-	if (get_user(suid, (__u64 __user *)arg))
-		suid = 0;
-	if (suid) {
-		/* search to see if there is a session with a matching SMB UID */
 		spin_lock(&cifs_tcp_ses_lock);
-		list_for_each(tmp, &tcon->ses->server->smb_ses_list) {
-			ses = list_entry(tmp, struct cifs_ses, smb_ses_list);
-			if (ses->Suid == suid) {
-				found = true;
-				break;
+		list_for_each_entry(server_it, &cifs_tcp_ses_list, tcp_ses_list) {
+			list_for_each_entry(ses_it, &server_it->smb_ses_list, smb_ses_list) {
+				if (ses_it->Suid == out.session_id) {
+					ses = ses_it;
+					/*
+					 * since we are using the session outside the crit
+					 * section, we need to make sure it won't be released
+					 * so increment its refcount
+					 */
+					ses->ses_count++;
+					found = true;
+					goto search_end;
+				}
 			}
 		}
+search_end:
 		spin_unlock(&cifs_tcp_ses_lock);
-		if (found == false)
-			return -EINVAL;
-	} /* else uses default user's SMB UID (ie current user) */
-
-	pfull_key_inf.cipher_type = le16_to_cpu(ses->server->cipher_type);
-	pfull_key_inf.Suid = ses->Suid;
-	memcpy(pfull_key_inf.auth_key, ses->auth_key.response,
-	       16 /* SMB2_NTLMV2_SESSKEY_SIZE */);
-	memcpy(pfull_key_inf.smb3decryptionkey, ses->smb3decryptionkey,
-	       32 /* SMB3_ENC_DEC_KEY_SIZE */);
-	memcpy(pfull_key_inf.smb3encryptionkey,
-	       ses->smb3encryptionkey, 32 /* SMB3_ENC_DEC_KEY_SIZE */);
-	if (copy_to_user((void __user *)arg, &pfull_key_inf,
-			 sizeof(struct smb3_full_key_debug_info)))
-		return -EFAULT;
+		if (!found) {
+			rc = -ENOENT;
+			goto out;
+		}
+	}
 
-	return 0;
+	switch (ses->server->cipher_type) {
+	case SMB2_ENCRYPTION_AES128_CCM:
+	case SMB2_ENCRYPTION_AES128_GCM:
+		out.session_key_length = CIFS_SESS_KEY_SIZE;
+		out.server_in_key_length = out.server_out_key_length = SMB3_GCM128_CRYPTKEY_SIZE;
+		break;
+	case SMB2_ENCRYPTION_AES256_CCM:
+	case SMB2_ENCRYPTION_AES256_GCM:
+		out.session_key_length = CIFS_SESS_KEY_SIZE;
+		out.server_in_key_length = out.server_out_key_length = SMB3_GCM256_CRYPTKEY_SIZE;
+		break;
+	default:
+		rc = -EOPNOTSUPP;
+		goto out;
+	}
+
+	/* check if user buffer is big enough to store all the keys */
+	if (out.in_size < sizeof(out) + out.session_key_length + out.server_in_key_length
+	    + out.server_out_key_length) {
+		rc = -ENOBUFS;
+		goto out;
+	}
+
+	out.session_id = ses->Suid;
+	out.cipher_type = le16_to_cpu(ses->server->cipher_type);
+
+	/* overwrite user input with our output */
+	if (copy_to_user(in, &out, sizeof(out))) {
+		rc = -EINVAL;
+		goto out;
+	}
+
+	/* append all the keys at the end of the user buffer */
+	end = in->data;
+	if (copy_to_user(end, ses->auth_key.response, out.session_key_length)) {
+		rc = -EINVAL;
+		goto out;
+	}
+	end += out.session_key_length;
+
+	if (copy_to_user(end, ses->smb3encryptionkey, out.server_in_key_length)) {
+		rc = -EINVAL;
+		goto out;
+	}
+	end += out.server_in_key_length;
+
+	if (copy_to_user(end, ses->smb3decryptionkey, out.server_out_key_length)) {
+		rc = -EINVAL;
+		goto out;
+	}
+
+out:
+	if (found)
+		cifs_put_smb_ses(ses);
+	return rc;
 }
 
 long cifs_ioctl(struct file *filep, unsigned int command, unsigned long arg)
@@ -371,6 +436,10 @@ long cifs_ioctl(struct file *filep, unsigned int command, unsigned long arg)
 				rc = -EOPNOTSUPP;
 			break;
 		case CIFS_DUMP_KEY:
+			/*
+			 * Dump encryption keys. This is an old ioctl that only
+			 * handles AES-128-{CCM,GCM}.
+			 */
 			if (pSMBFile == NULL)
 				break;
 			if (!capable(CAP_SYS_ADMIN)) {
@@ -398,11 +467,10 @@ long cifs_ioctl(struct file *filep, unsigned int command, unsigned long arg)
 			else
 				rc = 0;
 			break;
-		/*
-		 * Dump full key (32 bytes instead of 16 bytes) is
-		 * needed if GCM256 (stronger encryption) negotiated
-		 */
 		case CIFS_DUMP_FULL_KEY:
+			/*
+			 * Dump encryption keys (handles any key sizes)
+			 */
 			if (pSMBFile == NULL)
 				break;
 			if (!capable(CAP_SYS_ADMIN)) {
@@ -410,8 +478,7 @@ long cifs_ioctl(struct file *filep, unsigned int command, unsigned long arg)
 				break;
 			}
 			tcon = tlink_tcon(pSMBFile->tlink);
-			rc = cifs_dump_full_key(tcon, arg);
-
+			rc = cifs_dump_full_key(tcon, (void __user *)arg);
 			break;
 		case CIFS_IOC_NOTIFY:
 			if (!S_ISDIR(inode->i_mode)) {
-- 
2.31.1

Re: [PATCH v1 0/2] Change CIFS_FULL_KEY_DUMP ioctl to return variable size keys

[Paulo Alcantara]

Aurélien Aptel <aaptel@suse.com> writes:

> From: Aurelien Aptel <aaptel@suse.com>
>
> This patchset changes the CIFS_FULL_KEY_DUMP ioctl to return variable
> size keys to userspace while keeping the same ioctl number.
>
> This version of the ioctl should be future proof if we ever add more
> cipher types or bigger keys.
>
> This also fixes the build error for ARM related to get_user() being
> undefined.
>
> I have tested this for AES-128 and AES-256.
>
> I have a separate patch for the smbinfo utility to make use of this
> new ioctl that I will send in a separate thread.
>
> Aurelien Aptel (2):
>   cifs: set server->cipher_type to AES-128-CCM for SMB3.0
>   cifs: change format of CIFS_FULL_KEY_DUMP ioctl
>
>  fs/cifs/cifs_ioctl.h |  25 ++++++--
>  fs/cifs/cifspdu.h    |   3 +-
>  fs/cifs/ioctl.c      | 143 +++++++++++++++++++++++++++++++------------
>  fs/cifs/smb2pdu.c    |   7 +++
>  4 files changed, 133 insertions(+), 45 deletions(-)

Looks good.

Acked-by: Paulo Alcantara (SUSE) <pc@cjr.nz>

Re: [PATCH v1 1/2] cifs: set server->cipher_type to AES-128-CCM for SMB3.0

[Steve French]

stable? How useful is it to know for debugging?

On Fri, May 21, 2021 at 10:19 AM Aurélien Aptel <aaptel@suse.com> wrote:
>
> From: Aurelien Aptel <aaptel@suse.com>
>
> SMB3.0 doesn't have encryption negotiate context but simply uses
> the SMB2_GLOBAL_CAP_ENCRYPTION flag.
>
> When that flag is present in the neg response cifs.ko uses AES-128-CCM
> which is the only cipher available in this context.
>
> cipher_type was set to the server cipher only when parsing encryption
> negotiate context (SMB3.1.1).
>
> For SMB3.0 it was set to 0. This means cipher_type value can be 0 or 1
> for AES-128-CCM.
>
> Fix this by checking for SMB3.0 and encryption capability and setting
> cipher_type appropriately.
>
> Signed-off-by: Aurelien Aptel <aaptel@suse.com>
> ---
>  fs/cifs/smb2pdu.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index 9f24eb88297a..c205f93e0a10 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -958,6 +958,13 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
>         /* Internal types */
>         server->capabilities |= SMB2_NT_FIND | SMB2_LARGE_FILES;
>
> +       /*
> +        * SMB3.0 supports only 1 cipher and doesn't have a encryption neg context
> +        * Set the cipher type manually.
> +        */
> +       if (server->dialect == SMB30_PROT_ID && (server->capabilities & SMB2_GLOBAL_CAP_ENCRYPTION))
> +               server->cipher_type = SMB2_ENCRYPTION_AES128_CCM;
> +
>         security_blob = smb2_get_data_area_len(&blob_offset, &blob_length,
>                                                (struct smb2_sync_hdr *)rsp);
>         /*
> --
> 2.31.1
>


-- 
Thanks,

Steve

Re: [PATCH v1 2/2] cifs: change format of CIFS_FULL_KEY_DUMP ioctl

[Steve French]

Two trivial checkpatch nits:

WARNING: Block comments should align the * on each line
#155: FILE: fs/cifs/cifs_ioctl.h:96:
+ * __u8 server_out_key[server_out_key_length];
+ */

ERROR: "foo* bar" should be "foo *bar"
#190: FILE: fs/cifs/ioctl.c:218:
+static int cifs_dump_full_key(struct cifs_tcon *tcon, struct
smb3_full_key_debug_info __user* i

On Fri, May 21, 2021 at 10:19 AM Aurélien Aptel <aaptel@suse.com> wrote:
>
> From: Aurelien Aptel <aaptel@suse.com>
>
> Make CIFS_FULL_KEY_DUMP ioctl able to return variable-length keys.
>
> * userspace needs to pass the struct size along with optional
>   session_id and some space at the end to store keys
> * if there is enough space kernel returns keys in the extra space and
>   sets the length of each key via xyz_key_length fields
>
> This also fixes the build error for get_user() on ARM.
>
> Sample program:
>
>         #include <stdlib.h>
>         #include <stdio.h>
>         #include <stdint.h>
>         #include <sys/fcntl.h>
>         #include <sys/ioctl.h>
>
>         struct smb3_full_key_debug_info {
>                 uint32_t   in_size;
>                 uint64_t   session_id;
>                 uint16_t   cipher_type;
>                 uint8_t    session_key_length;
>                 uint8_t    server_in_key_length;
>                 uint8_t    server_out_key_length;
>                 uint8_t    data[];
>                 /*
>                  * return this struct with the keys appended at the end:
>                  * uint8_t session_key[session_key_length];
>                  * uint8_t server_in_key[server_in_key_length];
>                  * uint8_t server_out_key[server_out_key_length];
>                  */
>         } __attribute__((packed));
>
>         #define CIFS_IOCTL_MAGIC 0xCF
>         #define CIFS_DUMP_FULL_KEY _IOWR(CIFS_IOCTL_MAGIC, 10, struct smb3_full_key_debug_info)
>
>         void dump(const void *p, size_t len) {
>                 const char *hex = "0123456789ABCDEF";
>                 const uint8_t *b = p;
>                 for (int i = 0; i < len; i++)
>                         printf("%c%c ", hex[(b[i]>>4)&0xf], hex[b[i]&0xf]);
>                 putchar('\n');
>         }
>
>         int main(int argc, char **argv)
>         {
>                 struct smb3_full_key_debug_info *keys;
>                 uint8_t buf[sizeof(*keys)+1024] = {0};
>                 size_t off = 0;
>                 int fd, rc;
>
>                 keys = (struct smb3_full_key_debug_info *)&buf;
>                 keys->in_size = sizeof(buf);
>
>                 fd = open(argv[1], O_RDONLY);
>                 if (fd < 0)
>                         perror("open"), exit(1);
>
>                 rc = ioctl(fd, CIFS_DUMP_FULL_KEY, keys);
>                 if (rc < 0)
>                         perror("ioctl"), exit(1);
>
>                 printf("SessionId      ");
>                 dump(&keys->session_id, 8);
>                 printf("Cipher         %04x\n", keys->cipher_type);
>
>                 printf("SessionKey     ");
>                 dump(keys->data+off, keys->session_key_length);
>                 off += keys->session_key_length;
>
>                 printf("ServerIn Key   ");
>                 dump(keys->data+off, keys->server_in_key_length);
>                 off += keys->server_in_key_length;
>
>                 printf("ServerOut Key  ");
>                 dump(keys->data+off, keys->server_out_key_length);
>
>                 return 0;
>         }
>
> Usage:
>
>         $ gcc -o dumpkeys dumpkeys.c
>
> Against Windows Server 2020 preview (with AES-256-GCM support):
>
>         # mount.cifs //$ip/test /mnt -o "username=administrator,password=foo,vers=3.0,seal"
>         # ./dumpkeys /mnt/somefile
>         SessionId      0D 00 00 00 00 0C 00 00
>         Cipher         0002
>         SessionKey     AB CD CC 0D E4 15 05 0C 6F 3C 92 90 19 F3 0D 25
>         ServerIn Key   73 C6 6A C8 6B 08 CF A2 CB 8E A5 7D 10 D1 5B DC
>         ServerOut Key  6D 7E 2B A1 71 9D D7 2B 94 7B BA C4 F0 A5 A4 F8
>         # umount /mnt
>
>         With 256 bit keys:
>
>         # echo 1 > /sys/module/cifs/parameters/require_gcm_256
>         # mount.cifs //$ip/test /mnt -o "username=administrator,password=foo,vers=3.11,seal"
>         # ./dumpkeys /mnt/somefile
>         SessionId      09 00 00 00 00 0C 00 00
>         Cipher         0004
>         SessionKey     93 F5 82 3B 2F B7 2A 50 0B B9 BA 26 FB 8C 8B 03
>         ServerIn Key   6C 6A 89 B2 CB 7B 78 E8 04 93 37 DA 22 53 47 DF B3 2C 5F 02 26 70 43 DB 8D 33 7B DC 66 D3 75 A9
>         ServerOut Key  04 11 AA D7 52 C7 A8 0F ED E3 93 3A 65 FE 03 AD 3F 63 03 01 2B C0 1B D7 D7 E5 52 19 7F CC 46 B4
>
> Signed-off-by: Aurelien Aptel <aaptel@suse.com>
> ---
>  fs/cifs/cifs_ioctl.h |  25 ++++++--
>  fs/cifs/cifspdu.h    |   3 +-
>  fs/cifs/ioctl.c      | 143 +++++++++++++++++++++++++++++++------------
>  3 files changed, 126 insertions(+), 45 deletions(-)
>
> diff --git a/fs/cifs/cifs_ioctl.h b/fs/cifs/cifs_ioctl.h
> index 4a97fe12006b..4b3d33b2838d 100644
> --- a/fs/cifs/cifs_ioctl.h
> +++ b/fs/cifs/cifs_ioctl.h
> @@ -72,15 +72,28 @@ struct smb3_key_debug_info {
>  } __packed;
>
>  /*
> - * Dump full key (32 byte encrypt/decrypt keys instead of 16 bytes)
> - * is needed if GCM256 (stronger encryption) negotiated
> + * Dump variable-sized keys
>   */
>  struct smb3_full_key_debug_info {
> -       __u64   Suid;
> +       /* INPUT: size of userspace buffer */
> +       __u32   in_size;
> +
> +       /*
> +        * INPUT: 0 for current user, otherwise session to dump
> +        * OUTPUT: session id that was dumped
> +        */
> +       __u64   session_id;
>         __u16   cipher_type;
> -       __u8    auth_key[16]; /* SMB2_NTLMV2_SESSKEY_SIZE */
> -       __u8    smb3encryptionkey[32]; /* SMB3_ENC_DEC_KEY_SIZE */
> -       __u8    smb3decryptionkey[32]; /* SMB3_ENC_DEC_KEY_SIZE */
> +       __u8    session_key_length;
> +       __u8    server_in_key_length;
> +       __u8    server_out_key_length;
> +       __u8    data[];
> +       /*
> +        * return this struct with the keys appended at the end:
> +        * __u8 session_key[session_key_length];
> +        * __u8 server_in_key[server_in_key_length];
> +        * __u8 server_out_key[server_out_key_length];
> +       */
>  } __packed;
>
>  struct smb3_notify {
> diff --git a/fs/cifs/cifspdu.h b/fs/cifs/cifspdu.h
> index b53a87db282f..554d64fe171e 100644
> --- a/fs/cifs/cifspdu.h
> +++ b/fs/cifs/cifspdu.h
> @@ -148,7 +148,8 @@
>  #define SMB3_SIGN_KEY_SIZE (16)
>
>  /*
> - * Size of the smb3 encryption/decryption keys
> + * Size of the smb3 encryption/decryption key storage.
> + * This size is big enough to store any cipher key types.
>   */
>  #define SMB3_ENC_DEC_KEY_SIZE (32)
>
> diff --git a/fs/cifs/ioctl.c b/fs/cifs/ioctl.c
> index 28ec8d7c521a..87b2f1309e6f 100644
> --- a/fs/cifs/ioctl.c
> +++ b/fs/cifs/ioctl.c
> @@ -33,6 +33,7 @@
>  #include "cifsfs.h"
>  #include "cifs_ioctl.h"
>  #include "smb2proto.h"
> +#include "smb2glob.h"
>  #include <linux/btrfs.h>
>
>  static long cifs_ioctl_query_info(unsigned int xid, struct file *filep,
> @@ -214,48 +215,112 @@ static int cifs_shutdown(struct super_block *sb, unsigned long arg)
>         return 0;
>  }
>
> -static int cifs_dump_full_key(struct cifs_tcon *tcon, unsigned long arg)
> +static int cifs_dump_full_key(struct cifs_tcon *tcon, struct smb3_full_key_debug_info __user* in)
>  {
> -       struct smb3_full_key_debug_info pfull_key_inf;
> -       __u64 suid;
> -       struct list_head *tmp;
> +       struct smb3_full_key_debug_info out;
>         struct cifs_ses *ses;
> +       int rc = 0;
>         bool found = false;
> +       u8 __user *end;
>
> -       if (!smb3_encryption_required(tcon))
> -               return -EOPNOTSUPP;
> +       if (!smb3_encryption_required(tcon)) {
> +               rc = -EOPNOTSUPP;
> +               goto out;
> +       }
> +
> +       /* copy user input into our output buffer */
> +       if (copy_from_user(&out, in, sizeof(out))) {
> +               rc = -EINVAL;
> +               goto out;
> +       }
> +
> +       if (!out.session_id) {
> +               /* if ses id is 0, use current user session */
> +               ses = tcon->ses;
> +       } else {
> +               /* otherwise if a session id is given, look for it in all our sessions */
> +               struct cifs_ses *ses_it = NULL;
> +               struct TCP_Server_Info *server_it = NULL;
>
> -       ses = tcon->ses; /* default to user id for current user */
> -       if (get_user(suid, (__u64 __user *)arg))
> -               suid = 0;
> -       if (suid) {
> -               /* search to see if there is a session with a matching SMB UID */
>                 spin_lock(&cifs_tcp_ses_lock);
> -               list_for_each(tmp, &tcon->ses->server->smb_ses_list) {
> -                       ses = list_entry(tmp, struct cifs_ses, smb_ses_list);
> -                       if (ses->Suid == suid) {
> -                               found = true;
> -                               break;
> +               list_for_each_entry(server_it, &cifs_tcp_ses_list, tcp_ses_list) {
> +                       list_for_each_entry(ses_it, &server_it->smb_ses_list, smb_ses_list) {
> +                               if (ses_it->Suid == out.session_id) {
> +                                       ses = ses_it;
> +                                       /*
> +                                        * since we are using the session outside the crit
> +                                        * section, we need to make sure it won't be released
> +                                        * so increment its refcount
> +                                        */
> +                                       ses->ses_count++;
> +                                       found = true;
> +                                       goto search_end;
> +                               }
>                         }
>                 }
> +search_end:
>                 spin_unlock(&cifs_tcp_ses_lock);
> -               if (found == false)
> -                       return -EINVAL;
> -       } /* else uses default user's SMB UID (ie current user) */
> -
> -       pfull_key_inf.cipher_type = le16_to_cpu(ses->server->cipher_type);
> -       pfull_key_inf.Suid = ses->Suid;
> -       memcpy(pfull_key_inf.auth_key, ses->auth_key.response,
> -              16 /* SMB2_NTLMV2_SESSKEY_SIZE */);
> -       memcpy(pfull_key_inf.smb3decryptionkey, ses->smb3decryptionkey,
> -              32 /* SMB3_ENC_DEC_KEY_SIZE */);
> -       memcpy(pfull_key_inf.smb3encryptionkey,
> -              ses->smb3encryptionkey, 32 /* SMB3_ENC_DEC_KEY_SIZE */);
> -       if (copy_to_user((void __user *)arg, &pfull_key_inf,
> -                        sizeof(struct smb3_full_key_debug_info)))
> -               return -EFAULT;
> +               if (!found) {
> +                       rc = -ENOENT;
> +                       goto out;
> +               }
> +       }
>
> -       return 0;
> +       switch (ses->server->cipher_type) {
> +       case SMB2_ENCRYPTION_AES128_CCM:
> +       case SMB2_ENCRYPTION_AES128_GCM:
> +               out.session_key_length = CIFS_SESS_KEY_SIZE;
> +               out.server_in_key_length = out.server_out_key_length = SMB3_GCM128_CRYPTKEY_SIZE;
> +               break;
> +       case SMB2_ENCRYPTION_AES256_CCM:
> +       case SMB2_ENCRYPTION_AES256_GCM:
> +               out.session_key_length = CIFS_SESS_KEY_SIZE;
> +               out.server_in_key_length = out.server_out_key_length = SMB3_GCM256_CRYPTKEY_SIZE;
> +               break;
> +       default:
> +               rc = -EOPNOTSUPP;
> +               goto out;
> +       }
> +
> +       /* check if user buffer is big enough to store all the keys */
> +       if (out.in_size < sizeof(out) + out.session_key_length + out.server_in_key_length
> +           + out.server_out_key_length) {
> +               rc = -ENOBUFS;
> +               goto out;
> +       }
> +
> +       out.session_id = ses->Suid;
> +       out.cipher_type = le16_to_cpu(ses->server->cipher_type);
> +
> +       /* overwrite user input with our output */
> +       if (copy_to_user(in, &out, sizeof(out))) {
> +               rc = -EINVAL;
> +               goto out;
> +       }
> +
> +       /* append all the keys at the end of the user buffer */
> +       end = in->data;
> +       if (copy_to_user(end, ses->auth_key.response, out.session_key_length)) {
> +               rc = -EINVAL;
> +               goto out;
> +       }
> +       end += out.session_key_length;
> +
> +       if (copy_to_user(end, ses->smb3encryptionkey, out.server_in_key_length)) {
> +               rc = -EINVAL;
> +               goto out;
> +       }
> +       end += out.server_in_key_length;
> +
> +       if (copy_to_user(end, ses->smb3decryptionkey, out.server_out_key_length)) {
> +               rc = -EINVAL;
> +               goto out;
> +       }
> +
> +out:
> +       if (found)
> +               cifs_put_smb_ses(ses);
> +       return rc;
>  }
>
>  long cifs_ioctl(struct file *filep, unsigned int command, unsigned long arg)
> @@ -371,6 +436,10 @@ long cifs_ioctl(struct file *filep, unsigned int command, unsigned long arg)
>                                 rc = -EOPNOTSUPP;
>                         break;
>                 case CIFS_DUMP_KEY:
> +                       /*
> +                        * Dump encryption keys. This is an old ioctl that only
> +                        * handles AES-128-{CCM,GCM}.
> +                        */
>                         if (pSMBFile == NULL)
>                                 break;
>                         if (!capable(CAP_SYS_ADMIN)) {
> @@ -398,11 +467,10 @@ long cifs_ioctl(struct file *filep, unsigned int command, unsigned long arg)
>                         else
>                                 rc = 0;
>                         break;
> -               /*
> -                * Dump full key (32 bytes instead of 16 bytes) is
> -                * needed if GCM256 (stronger encryption) negotiated
> -                */
>                 case CIFS_DUMP_FULL_KEY:
> +                       /*
> +                        * Dump encryption keys (handles any key sizes)
> +                        */
>                         if (pSMBFile == NULL)
>                                 break;
>                         if (!capable(CAP_SYS_ADMIN)) {
> @@ -410,8 +478,7 @@ long cifs_ioctl(struct file *filep, unsigned int command, unsigned long arg)
>                                 break;
>                         }
>                         tcon = tlink_tcon(pSMBFile->tlink);
> -                       rc = cifs_dump_full_key(tcon, arg);
> -
> +                       rc = cifs_dump_full_key(tcon, (void __user *)arg);
>                         break;
>                 case CIFS_IOC_NOTIFY:
>                         if (!S_ISDIR(inode->i_mode)) {
> --
> 2.31.1
>


-- 
Thanks,

Steve

Re: [PATCH v1 1/2] cifs: set server->cipher_type to AES-128-CCM for SMB3.0

[Steve French]

Added cc:stable

On Fri, May 21, 2021 at 10:19 AM Aurélien Aptel <aaptel@suse.com> wrote:
>
> From: Aurelien Aptel <aaptel@suse.com>
>
> SMB3.0 doesn't have encryption negotiate context but simply uses
> the SMB2_GLOBAL_CAP_ENCRYPTION flag.
>
> When that flag is present in the neg response cifs.ko uses AES-128-CCM
> which is the only cipher available in this context.
>
> cipher_type was set to the server cipher only when parsing encryption
> negotiate context (SMB3.1.1).
>
> For SMB3.0 it was set to 0. This means cipher_type value can be 0 or 1
> for AES-128-CCM.
>
> Fix this by checking for SMB3.0 and encryption capability and setting
> cipher_type appropriately.
>
> Signed-off-by: Aurelien Aptel <aaptel@suse.com>
> ---
>  fs/cifs/smb2pdu.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index 9f24eb88297a..c205f93e0a10 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -958,6 +958,13 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
>         /* Internal types */
>         server->capabilities |= SMB2_NT_FIND | SMB2_LARGE_FILES;
>
> +       /*
> +        * SMB3.0 supports only 1 cipher and doesn't have a encryption neg context
> +        * Set the cipher type manually.
> +        */
> +       if (server->dialect == SMB30_PROT_ID && (server->capabilities & SMB2_GLOBAL_CAP_ENCRYPTION))
> +               server->cipher_type = SMB2_ENCRYPTION_AES128_CCM;
> +
>         security_blob = smb2_get_data_area_len(&blob_offset, &blob_length,
>                                                (struct smb2_sync_hdr *)rsp);
>         /*
> --
> 2.31.1
>


-- 
Thanks,

Steve

Re: [PATCH v1 1/2] cifs: set server->cipher_type to AES-128-CCM for SMB3.0

[Aurélien Aptel]

Steve French <smfrench@gmail.com> writes:
> stable? How useful is it to know for debugging?

I would say it's more correct and avoids checking for 2 values when
checking for 128-CCM. Not much difference in debugging.

Cheers,
-- 
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München)