Skip to contentSkip to conversation

Add support for DFS in SMB2+

PATCH Superseded Aurelien Aptel 2 participants 13 messages Feb 23, 2017 – Mar 1, 2017 6 messages not in the archive

Subsystems (2)

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

v2 cover Add support for DFS in SMB2+

This series of patch tries to implement the get_dfs_refer operation for SMB2+.

In SMB2+, DFS resolving is now done through an FSCTL (patch #3). The relevant Microsoft specifications for this are:

MS-SMB2: 3.2.4.20 Application Requests an IO Control Code Operation
MS-SMB2: 3.2.4.20.3 Application Requests DFS Referral Information
MS-SMB2: 3.2.5.14 Receiving an SMB2 IOCTL Response
MS-SMB2: 3.2.5.14.4 Handling a DFS Referral Information Response

MS-DFSC: 2.2 Message Syntax (but really the whole document is useful)

The DFS response payload however is identical. Patch #1 moves the DFS response parsing out of SMB1 code and makes it work on any version of SMB.

DFS code has 2 "main" entry points: initial mounting and automount (when a DFS link is accessed/traversed).

When automounting, cifs.ko calls build_path_from_dentry() which only makes treename-prefixed paths when the tcon has the SMB_IN_DFS_SHARE. This flag is checked in tcon->Flags which is never touched by SMB2 code as it sets tcon->share_flags on connexion.

* CIFS requires to prefix all pathnames with the treename prefix when connected to a DFS server, so the current build_path_from_dentry() makes sense for CIFS.
* For SMB2+ it seems only the Create request requires the treename prefix. Simply making the function check for both CIFS SMB2+ flag for DFS to add a prefix does not work as the server has different expectations about which packet can have/require a DFS pathname.

Patch #2 adds build_path_from_dentry_optional_prefix() with an extra bool arg to decide to prefix or not. The automount code path always ask for it. Patch #5 modifies SMB2_open() to add the treename prefix to the given path if the server requires it.

As part of the mouting process a IPC tcon is made (I suspect we don't need it anymore in SMB3). This tcon doesn't respect the signing requirement. This is fixed by patch #4.

Finally the SMB2+ implementation of the get_dfs_referral operation is added in all supported SMB versions in patch #6.

I've sucessfuly tested this against a Windows Server 2016 DFS setup. Samba requires a patch as currently it only accepts DFS referrals requests on an IPC connexion, which is in violation of the spec. A patch was sent on samba-technical:

https://lists.samba.org/archive/samba-technical/2017-February/118859.html

Changes since v1:

* The fsctl is made on the first available tcon to the server, which is allowed by the spec. This way we avoid hardcoding additional rules in SMB2_ioctl().
* add signing flag in smb2 header if required
* no copy/pasting, the DFS parsing code is now generic and works for both SMB versions
* add treename prefix in SMB2_open() when required

Aurelien Aptel (6):
CIFS: move DFS response parsing out of SMB1 code
CIFS: add build_path_from_dentry_optional_prefix()
CIFS: implement get_dfs_refer for SMB2+
CIFS: set signing flag in SMB2+ TreeConnect if needed
CIFS: use DFS pathnames in SMB2+ Create requests
CIFS: enable get_dfs_refer for SMB2+

 fs/cifs/cifs_dfs_ref.c |   4 +-
 fs/cifs/cifspdu.h      |  16 ++++---
 fs/cifs/cifsproto.h    |   7 +++
 fs/cifs/cifssmb.c      | 119 +++----------------------------------------------
 fs/cifs/dir.c          |  13 +++++-
 fs/cifs/misc.c         | 105 +++++++++++++++++++++++++++++++++++++++++++
 fs/cifs/smb2ops.c      |  76 +++++++++++++++++++++++++++++++
 fs/cifs/smb2pdu.c      |  97 ++++++++++++++++++++++++++++++++--------
 fs/cifs/smb2pdu.h      |   8 ++++
 9 files changed, 305 insertions(+), 140 deletions(-)
Signature · 1 line
2.10.2

A message that is not in the archive (sent off-list or not yet received): 20170223144334.22320-1-aaptel-IBi9RG/[email protected]

#2 lore raw

PATCH v2 1/6 CIFS: move DFS response parsing out of SMB1 code

since the DFS payload is not tied to the SMB version we can:
* isolate the DFS payload in its own struct, and include that struct in packet structs
* move the function that parses the response to misc.c and make it work on the new DFS payload struct (add payload size and utf16 flag as a result).

---
 fs/cifs/cifspdu.h   |  16 ++++---
 fs/cifs/cifsproto.h |   5 +++
 fs/cifs/cifssmb.c   | 119 +++-------------------------------------------------
 fs/cifs/misc.c      | 105 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 125 insertions(+), 120 deletions(-)
fs/cifs/cifspdu.h +10 -6
index f5b8730..1ce733f 100644
@@ -2086,17 +2086,21 @@ typedef struct dfs_referral_level_3 { /* version 4 is same, + one flag bit */
 	__u8   ServiceSiteGuid[16];  /* MBZ, ignored */
 } __attribute__((packed)) REFERRAL3;
 
-typedef struct smb_com_transaction_get_dfs_refer_rsp {
-	struct smb_hdr hdr;	/* wct = 10 */
-	struct trans2_resp t2;
-	__u16 ByteCount;
-	__u8 Pad;
+struct get_dfs_referral_rsp {
 	__le16 PathConsumed;
 	__le16 NumberOfReferrals;
 	__le32 DFSFlags;
 	REFERRAL3 referrals[1];	/* array of level 3 dfs_referral structures */
 	/* followed by the strings pointed to by the referral structures */
-} __attribute__((packed)) TRANSACTION2_GET_DFS_REFER_RSP;
+} __packed;
+
+typedef struct smb_com_transaction_get_dfs_refer_rsp {
+	struct smb_hdr hdr;	/* wct = 10 */
+	struct trans2_resp t2;
+	__u16 ByteCount;
+	__u8 Pad;
+	struct get_dfs_referral_rsp dfs_data;
+} __packed TRANSACTION2_GET_DFS_REFER_RSP;
 
 /* DFS Flags */
 #define DFSREF_REFERRAL_SERVER  0x00000001 /* all targets are DFS roots */
fs/cifs/cifsproto.h +5 -0
index 406d2c1..c097830 100644
@@ -284,6 +284,11 @@ extern int get_dfs_path(const unsigned int xid, struct cifs_ses *ses,
 			const struct nls_table *nls_codepage,
 			unsigned int *num_referrals,
 			struct dfs_info3_param **referrals, int remap);
+extern int parse_dfs_referrals(struct get_dfs_referral_rsp *rsp, u32 rsp_size,
+			       unsigned int *num_of_nodes,
+			       struct dfs_info3_param **target_nodes,
+			       const struct nls_table *nls_codepage, int remap,
+			       const char *searchName, bool is_unicode);
 extern void reset_cifs_unix_caps(unsigned int xid, struct cifs_tcon *tcon,
 				 struct cifs_sb_info *cifs_sb,
 				 struct smb_vol *vol);
fs/cifs/cifssmb.c +5 -114

131 lines not on this page · raw message

fs/cifs/misc.c +105 -0

108 lines not on this page · raw message

Signature · 1 line
2.10.2
#3 lore raw

PATCH v2 2/6 CIFS: add build_path_from_dentry_optional_prefix()

In reply to a message not in the archive

this function does the same thing as add build_path_from_dentry() but takes a boolean parameter to decide whether or not to prefix the path with the tree name.

we cannot rely on tcon->Flags & SMB_SHARE_IS_IN_DFS for SMB2 as smb2 code never sets tcon->Flags but it sets tcon->share_flags and it seems the SMB_SHARE_IS_IN_DFS has different semantics in SMB2: the prefix shouldn't be added everytime it was in SMB1.

---
 fs/cifs/cifs_dfs_ref.c |  4 +++-
 fs/cifs/cifsproto.h    |  2 ++
 fs/cifs/dir.c          | 13 ++++++++++++-
 3 files changed, 17 insertions(+), 2 deletions(-)
fs/cifs/cifs_dfs_ref.c +3 -1
index ec9dbbc..8a9ebfd 100644
@@ -302,7 +302,9 @@ static struct vfsmount *cifs_dfs_do_automount(struct dentry *mntpt)
 	 * gives us the latter, so we must adjust the result.
 	 */
 	mnt = ERR_PTR(-ENOMEM);
-	full_path = build_path_from_dentry(mntpt);
+
+	/* always use tree name prefix */
+	full_path = build_path_from_dentry_optional_prefix(mntpt, true);
 	if (full_path == NULL)
 		goto cdda_exit;
 
fs/cifs/cifsproto.h +2 -0
index c097830..9ee46c1 100644
@@ -61,6 +61,8 @@ extern void exit_cifs_idmap(void);
 extern int init_cifs_spnego(void);
 extern void exit_cifs_spnego(void);
 extern char *build_path_from_dentry(struct dentry *);
+extern char *build_path_from_dentry_optional_prefix(struct dentry *direntry,
+						    bool prefix);
 extern char *cifs_build_path_to_root(struct smb_vol *vol,
 				     struct cifs_sb_info *cifs_sb,
 				     struct cifs_tcon *tcon,
fs/cifs/dir.c +12 -1
index 2c227a9..56366e9 100644
@@ -81,6 +81,17 @@ cifs_build_path_to_root(struct smb_vol *vol, struct cifs_sb_info *cifs_sb,
 char *
 build_path_from_dentry(struct dentry *direntry)
 {
+	struct cifs_sb_info *cifs_sb = CIFS_SB(direntry->d_sb);
+	struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb);
+	bool prefix = tcon->Flags & SMB_SHARE_IS_IN_DFS;
+
+	return build_path_from_dentry_optional_prefix(direntry,
+						      prefix);
+}
+
+char *
+build_path_from_dentry_optional_prefix(struct dentry *direntry, bool prefix)
+{
 	struct dentry *temp;
 	int namelen;
 	int dfsplen;
@@ -92,7 +103,7 @@ build_path_from_dentry(struct dentry *direntry)
 	unsigned seq;
 
 	dirsep = CIFS_DIR_SEP(cifs_sb);
-	if (tcon->Flags & SMB_SHARE_IS_IN_DFS)
+	if (prefix)
 		dfsplen = strnlen(tcon->treeName, MAX_TREE_SIZE + 1);
 	else
 		dfsplen = 0;
Signature · 1 line
2.10.2
#4 lore raw

PATCH v2 3/6 CIFS: implement get_dfs_refer for SMB2+

In reply to a message not in the archive
---
 fs/cifs/smb2ops.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/cifs/smb2pdu.h |  8 +++++++
 2 files changed, 80 insertions(+)
fs/cifs/smb2ops.c +72 -0
index a44b4db..2563fe8 100644
@@ -1097,6 +1097,78 @@ smb2_new_lease_key(struct cifs_fid *fid)
 	generate_random_uuid(fid->lease_key);
 }
 
+static int
+smb2_get_dfs_refer(const unsigned int xid, struct cifs_ses *ses,
+		   const char *search_name,
+		   struct dfs_info3_param **target_nodes,
+		   unsigned int *num_of_nodes,
+		   const struct nls_table *nls_codepage, int remap)
+{
+	int rc = -EINVAL;
+	__le16 *utf16_path = NULL;
+	int utf16_path_len = 0;
+	struct cifs_tcon *tcon;
+	struct fsctl_get_dfs_referral_req *dfs_req = NULL;
+	struct get_dfs_referral_rsp *dfs_rsp = NULL;
+	u32 dfs_req_size = 0, dfs_rsp_size = 0;
+
+	cifs_dbg(FYI, "smb2_get_dfs_refer path <%s>\n", search_name);
+
+	/*
+	 * Use any tcon from the current session. Here, the first one.
+	 */
+	tcon = list_first_entry(&ses->tcon_list, struct cifs_tcon, tcon_list);
+	if (!tcon)
+		goto out;
+
+	utf16_path = cifs_strndup_to_utf16(search_name, PATH_MAX,
+					   &utf16_path_len,
+					   nls_codepage, remap);
+	if (!utf16_path) {
+		rc = -ENOMEM;
+		goto out;
+	}
+
+	dfs_req_size = sizeof(*dfs_req) + utf16_path_len;
+	dfs_req = kzalloc(dfs_req_size, GFP_KERNEL);
+	if (!dfs_req) {
+		rc = -ENOMEM;
+		goto out;
+	}
+
+	/* Highest DFS referral version understood */
+	dfs_req->MaxReferralLevel = cpu_to_le16(DFS_VERSION);
+
+	/* Path to resolve in an UTF-16 null-terminated string */
+	memcpy(dfs_req->RequestFileName, utf16_path, utf16_path_len);
+
+	do {
+		rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
+				FSCTL_DFS_GET_REFERRALS, true /* is_fsctl */,
+				(char *)dfs_req, dfs_req_size,
+				(char **)&dfs_rsp, &dfs_rsp_size);
+	} while (rc == -EAGAIN);
+
+	if (rc) {
+		cifs_dbg(VFS, "ioctl error in smb2_get_dfs_refer rc=%d\n", rc);
+		goto out;
+	}
+
+	rc = parse_dfs_referrals(dfs_rsp, dfs_rsp_size,
+				 num_of_nodes, target_nodes,
+				 nls_codepage, remap, search_name,
+				 true /* is_unicode */);
+	if (rc) {
+		cifs_dbg(VFS, "parse error in smb2_get_dfs_refer rc=%d\n", rc);
+		goto out;
+	}
+
+ out:
+	kfree(utf16_path);
+	kfree(dfs_req);
+	kfree(dfs_rsp);
+	return rc;
+}
 #define SMB2_SYMLINK_STRUCT_SIZE \
 	(sizeof(struct smb2_err_rsp) - 1 + sizeof(struct smb2_symlink_err_rsp))
 
fs/cifs/smb2pdu.h +8 -0
index c03b252..18700fd 100644
@@ -695,6 +695,14 @@ struct fsctl_get_integrity_information_rsp {
 /* Integrity flags for above */
 #define FSCTL_INTEGRITY_FLAG_CHECKSUM_ENFORCEMENT_OFF	0x00000001
 
+/* See MS-DFSC 2.2.2 */
+struct fsctl_get_dfs_referral_req {
+	__le16 MaxReferralLevel;
+	__u8 RequestFileName[];
+} __packed;
+
+/* DFS response is struct get_dfs_refer_rsp */
+
 /* See MS-SMB2 2.2.31.3 */
 struct network_resiliency_req {
 	__le32 Timeout;
Signature · 1 line
2.10.2

A message that is not in the archive (sent off-list or not yet received): 20170223144334.22320-4-aaptel-IBi9RG/[email protected]

#6 lore raw

Re: [PATCH v2 3/6] CIFS: implement get_dfs_refer for SMB2+

2017-02-23 6:43 GMT-08:00 Aurelien Aptel <aaptel-IBi9RG/[email protected]>:

Quoted 27 lines
---
 fs/cifs/smb2ops.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/cifs/smb2pdu.h |  8 +++++++
 2 files changed, 80 insertions(+)
fs/cifs/smb2ops.c +13 -0
index a44b4db..2563fe8 100644
@@ -1097,6 +1097,78 @@ smb2_new_lease_key(struct cifs_fid *fid)
        generate_random_uuid(fid->lease_key);
 }
 
+static int
+smb2_get_dfs_refer(const unsigned int xid, struct cifs_ses *ses,
+                  const char *search_name,
+                  struct dfs_info3_param **target_nodes,
+                  unsigned int *num_of_nodes,
+                  const struct nls_table *nls_codepage, int remap)
+{
+       int rc = -EINVAL;
+       __le16 *utf16_path = NULL;
+       int utf16_path_len = 0;
+       struct cifs_tcon *tcon;
+       struct fsctl_get_dfs_referral_req *dfs_req = NULL;
+       struct get_dfs_referral_rsp *dfs_rsp = NULL;

fs/cifs/smb2ops.c smb2_new_lease_key(struct cifs_fid *fid) · line 1122

Quoted 10 lines
fs/cifs/smb2ops.c +10 -0
+       u32 dfs_req_size = 0, dfs_rsp_size = 0;
+
+       cifs_dbg(FYI, "smb2_get_dfs_refer path <%s>\n", search_name);
+
+       /*
+        * Use any tcon from the current session. Here, the first one.
+        */
+       tcon = list_first_entry(&ses->tcon_list, struct cifs_tcon, tcon_list);
+       if (!tcon)
+               goto out;

Accessing the tcon_list should be protected by spin_locks and also a reference to the tcon should be acquired before using it safely.

Also you can extend SMB2_ioctl argument list to allow passing ipc_tid. In this case SMB2_ioctl() can choose what to use - ipc_tid or tcon; if tcon is passed -use it, otherwise - use ipc_tid.

In this case you can fallback to ipc_tid if no tcon is found in the list.

Quoted 78 lines
fs/cifs/smb2ops.c +49 -0
+
+       utf16_path = cifs_strndup_to_utf16(search_name, PATH_MAX,
+                                          &utf16_path_len,
+                                          nls_codepage, remap);
+       if (!utf16_path) {
+               rc = -ENOMEM;
+               goto out;
+       }
+
+       dfs_req_size = sizeof(*dfs_req) + utf16_path_len;
+       dfs_req = kzalloc(dfs_req_size, GFP_KERNEL);
+       if (!dfs_req) {
+               rc = -ENOMEM;
+               goto out;
+       }
+
+       /* Highest DFS referral version understood */
+       dfs_req->MaxReferralLevel = cpu_to_le16(DFS_VERSION);
+
+       /* Path to resolve in an UTF-16 null-terminated string */
+       memcpy(dfs_req->RequestFileName, utf16_path, utf16_path_len);
+
+       do {
+               rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
+                               FSCTL_DFS_GET_REFERRALS, true /* is_fsctl */,
+                               (char *)dfs_req, dfs_req_size,
+                               (char **)&dfs_rsp, &dfs_rsp_size);
+       } while (rc == -EAGAIN);
+
+       if (rc) {
+               cifs_dbg(VFS, "ioctl error in smb2_get_dfs_refer rc=%d\n", rc);
+               goto out;
+       }
+
+       rc = parse_dfs_referrals(dfs_rsp, dfs_rsp_size,
+                                num_of_nodes, target_nodes,
+                                nls_codepage, remap, search_name,
+                                true /* is_unicode */);
+       if (rc) {
+               cifs_dbg(VFS, "parse error in smb2_get_dfs_refer rc=%d\n", rc);
+               goto out;
+       }
+
+ out:
+       kfree(utf16_path);
+       kfree(dfs_req);
+       kfree(dfs_rsp);
+       return rc;
+}
 #define SMB2_SYMLINK_STRUCT_SIZE \
        (sizeof(struct smb2_err_rsp) - 1 + sizeof(struct smb2_symlink_err_rsp))
 
fs/cifs/smb2pdu.h +8 -1
index c03b252..18700fd 100644
@@ -695,6 +695,14 @@ struct fsctl_get_integrity_information_rsp {
 /* Integrity flags for above */
 #define FSCTL_INTEGRITY_FLAG_CHECKSUM_ENFORCEMENT_OFF  0x00000001
 
+/* See MS-DFSC 2.2.2 */
+struct fsctl_get_dfs_referral_req {
+       __le16 MaxReferralLevel;
+       __u8 RequestFileName[];
+} __packed;
+
+/* DFS response is struct get_dfs_refer_rsp */
+
 /* See MS-SMB2 2.2.31.3 */
 struct network_resiliency_req {
        __le32 Timeout;
--

2.10.2

--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 2 lines
Best regards,
Pavel Shilovsky

A message that is not in the archive (sent off-list or not yet received): CAKywueSCHszmzBbfLfJTP66=w6FcKgR2p1UJJig4uxcgC4QSfA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

#8 lore raw

Re: [PATCH v2 3/6] CIFS: implement get_dfs_refer for SMB2+

Hi Pavel,

Quoted 2 lines · Pavel Shilovsky

Accessing the tcon_list should be protected by spin_locks and also a reference to the tcon should be acquired before using it safely.

I've noticed we have the cifs_tcp_ses_lock but also a per-session session_mutex. I'm guessing we want cifs_tcp_ses_lock here.

By acquiring a reference you mean incrementing/decrementing tc_count. Do we need to protect this inc/dec operation with cifs_tcp_ses_lock too?

Quoted 6 lines

Also you can extend SMB2_ioctl argument list to allow passing ipc_tid. In this case SMB2_ioctl() can choose what to use - ipc_tid or tcon; if tcon is passed -use it, otherwise - use ipc_tid.

In this case you can fallback to ipc_tid if no tcon is found in the list.

SMB2_ioctl() uses the tcon for many things. I don't see how we can just use ipc_tid number if we don't pass a tcon.

Or do you mean: add a use_ipc bool to SMB2_ioctl arg list and use the tcon as usual except overwrite the Tid field with ses->ipc_tid before sending?

Thanks,

Signature · 4 lines
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
SUSE Linux GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)

A message that is not in the archive (sent off-list or not yet received): [email protected]

#10 lore raw

Re: [PATCH v2 3/6] CIFS: implement get_dfs_refer for SMB2+

2017-02-24 7:25 GMT-08:00 Aurélien Aptel <aaptel-IBi9RG/[email protected]>:

Quoted 11 lines

Hi Pavel,

Quoted 2 lines · Pavel Shilovsky

Accessing the tcon_list should be protected by spin_locks and also a reference to the tcon should be acquired before using it safely.

I've noticed we have the cifs_tcp_ses_lock but also a per-session session_mutex. I'm guessing we want cifs_tcp_ses_lock here.

By acquiring a reference you mean incrementing/decrementing tc_count. Do we need to protect this inc/dec operation with cifs_tcp_ses_lock too?

Yes.

Quoted 5 lines
Quoted 5 lines

Also you can extend SMB2_ioctl argument list to allow passing ipc_tid. In this case SMB2_ioctl() can choose what to use - ipc_tid or tcon; if tcon is passed -use it, otherwise - use ipc_tid.

Quoted 8 lines
Quoted 2 lines

In this case you can fallback to ipc_tid if no tcon is found in the list.

SMB2_ioctl() uses the tcon for many things. I don't see how we can just use ipc_tid number if we don't pass a tcon.

Or do you mean: add a use_ipc bool to SMB2_ioctl arg list and use the tcon as usual except overwrite the Tid field with ses->ipc_tid before sending?

We can pass both arguments: tcon and ipc_tid. If the 1st is specified SMB_ioctl uses it, otherwise - use ipc_tid.

Since we call get_dfs_refer() once we get ipc_tid, may be it is worth to try ipc_tid at first and then, if EAGAIN is returned (probably due to reconnection that makes ipc_tid equal 0), use the 1st tcon from list for the session. This will also workaround the bug in Samba.

Quoted 8 lines

Thanks,

--
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97 8C99 03C8 A49B 521B D5D3
SUSE Linux GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)

Signature · 2 lines
Best regards,
Pavel Shilovsky

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): [email protected]

#13 lore raw

Re: [PATCH v2 3/6] CIFS: implement get_dfs_refer for SMB2+

Quoted 11 lines · Aurélien Aptel

What I'm proposing is to add a bool flag use_ipc to SMB2_ioctl. And do something like this:

    if use_ipc:
        if tcon->ses->ipc_tid == 0:
            return -ENOTCONN
        else:
            req->tid = tcon->ses->ipc_tid

Now in smb2_get_dfs_referral can first try to call it with use_ipc=true and if that fails with ENOTCONN try again with false.

I went ahead and implemented this along with small improvements in a v3 patchset. I'm still open to rework on this if can understand better what you were proposing.

Cheers,

Signature · 4 lines
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
SUSE Linux GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
#14 lore raw

Re: [PATCH v2 3/6] CIFS: implement get_dfs_refer for SMB2+

In reply to a message not in the archive

2017-02-27 6:17 GMT-08:00 Aurélien Aptel <aaptel-IBi9RG/[email protected]>:

Quoted 11 lines
Quoted 2 lines · Pavel Shilovsky

We can pass both arguments: tcon and ipc_tid. If the 1st is specified SMB_ioctl uses it, otherwise - use ipc_tid.

I still don't get it. When tcon is not specified (I'm assuming that means passing NULL) and we just have ipc_tid (an int) we cannot:
- access the struct cifs_ses*
- access the struct TCP_Server_Info*
- check if encryption is required
- call any of the functions that requires the session or the server struct (i.e. all the functions that do the real work).

Yes, we still need to pass "ses" argument which will create a whole mess. But we probably need to clean this anyway because the number of argument are getting too big.

Quoted 9 lines
Quoted 5 lines

Since we call get_dfs_refer() once we get ipc_tid, may be it is worth to try ipc_tid at first and then, if EAGAIN is returned (probably due to reconnection that makes ipc_tid equal 0), use the 1st tcon from list for the session. This will also workaround the bug in Samba.

What I'm proposing is to add a bool flag use_ipc to SMB2_ioctl. And do something like this:

Quoted 8 lines
    if use_ipc:
        if tcon->ses->ipc_tid == 0:
            return -ENOTCONN
        else:
            req->tid = tcon->ses->ipc_tid

Now in smb2_get_dfs_referral can first try to call it with use_ipc=true and if that fails with ENOTCONN try again with false.

This also works although it doesn't allow to call SMB2_ioctl() without having at least one tcon.

Signature · 2 lines
Best regards,
Pavel Shilovsky
#15 lore raw

PATCH v2 4/6 CIFS: set signing flag in SMB2+ TreeConnect if needed

In reply to a message not in the archive

cifs_enable_signing() already sets server->sign according to what the server requires/offers and what mount options allows/forbids, so use that.

this is required for IPC tcon that connects to signing-required servers.

---
 fs/cifs/smb2pdu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
fs/cifs/smb2pdu.c +2 -2
index ad83b3d..c9a1d71 100644
@@ -1167,8 +1167,8 @@ SMB2_tcon(const unsigned int xid, struct cifs_ses *ses, const char *tree,
 
 		/* since no tcon, smb2_init can not do this, so do here */
 		req->hdr.sync_hdr.SessionId = ses->Suid;
-		/* if (ses->server->sec_mode & SECMODE_SIGN_REQUIRED)
-			req->hdr.Flags |= SMB2_FLAGS_SIGNED; */
+		if (ses->server->sign)
+			req->hdr.sync_hdr.Flags |= SMB2_FLAGS_SIGNED;
 	} else if (encryption_required(tcon))
 		flags |= CIFS_TRANSFORM_REQ;
 
Signature · 1 line
2.10.2
#16 lore raw

PATCH v2 5/6 CIFS: use DFS pathnames in SMB2+ Create requests

In reply to a message not in the archive

When connected to a DFS capable share, the client must set the SMB2_FLAGS_DFS_OPERATIONS flag in the SMB2 header and use DFS path names: "<server>\<share>\<path>" *without* leading \\.

Sources:

[MS-SMB2] 3.2.5.5 Receiving an SMB2 TREE_CONNECT Response

Quoted 2 lines

TreeConnect.IsDfsShare MUST be set to TRUE, if the SMB2_SHARE_CAP_DFS bit is set in the Capabilities field of the response.

[MS-SMB2] 3.2.4.3 Application Requests Opening a File

Quoted 2 lines

If TreeConnect.IsDfsShare is TRUE, the SMB2_FLAGS_DFS_OPERATIONS flag is set in the Flags field.

[MS-SMB2] 2.2.13 SMB2 CREATE Request, NameOffset:

Quoted 5 lines

If SMB2_FLAGS_DFS_OPERATIONS is set in the Flags field of the SMB2 header, the file name includes a prefix that will be processed during DFS name normalization as specified in section 3.3.5.9. Otherwise, the file name is relative to the share that is identified by the TreeId in the SMB2 header.

---
 fs/cifs/smb2pdu.c | 93 +++++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 77 insertions(+), 16 deletions(-)
fs/cifs/smb2pdu.c +77 -16
index c9a1d71..9e9f449 100644
@@ -1527,6 +1527,48 @@ add_durable_context(struct kvec *iov, unsigned int *num_iovec,
 	return 0;
 }
 
+static int
+alloc_path_with_tree_prefix(__le16 **out_path, int *out_size, int *out_len,
+			    const char *treename, const __le16 *path)
+{
+	int treename_len, path_len;
+	const __le16 sep[] = {cpu_to_le16('\\'), cpu_to_le16(0x0000)};
+
+	/*
+	 * skip leading "\\"
+	 */
+	treename_len = strlen(treename);
+	if (treename_len < 2 || !(treename[0] == '\\' && treename[1] == '\\'))
+		return -EINVAL;
+
+	treename += 2;
+	treename_len -= 2;
+
+	path_len = UniStrnlen((wchar_t *)path, PATH_MAX);
+
+	/*
+	 * make room for one path separator between the treename and
+	 * path
+	 */
+	*out_len = treename_len + 1 + path_len;
+
+	/*
+	 * final path needs to be null-terminated UTF16 with a
+	 * size aligned to 8
+	 */
+
+	*out_size = roundup((*out_len+1)*2, 8);
+	*out_path = kzalloc(*out_size, GFP_KERNEL);
+	if (!*out_path)
+		return -ENOMEM;
+
+	cifs_strtoUTF16(*out_path, treename, treename_len, load_nls_default());
+	UniStrcat(*out_path, sep);
+	UniStrcat(*out_path, path);
+
+	return 0;
+}
+
 int
 SMB2_open(const unsigned int xid, struct cifs_open_parms *oparms, __le16 *path,
 	  __u8 *oplock, struct smb2_file_all_info *buf,
@@ -1575,30 +1617,49 @@ SMB2_open(const unsigned int xid, struct cifs_open_parms *oparms, __le16 *path,
 	req->ShareAccess = FILE_SHARE_ALL_LE;
 	req->CreateDisposition = cpu_to_le32(oparms->disposition);
 	req->CreateOptions = cpu_to_le32(oparms->create_options & CREATE_OPTIONS_MASK);
-	uni_path_len = (2 * UniStrnlen((wchar_t *)path, PATH_MAX)) + 2;
-	/* do not count rfc1001 len field */
-	req->NameOffset = cpu_to_le16(sizeof(struct smb2_create_req) - 4);
 
 	iov[0].iov_base = (char *)req;
 	/* 4 for rfc1002 length field */
 	iov[0].iov_len = get_rfc1002_length(req) + 4;
-
-	/* MUST set path len (NameLength) to 0 opening root of share */
-	req->NameLength = cpu_to_le16(uni_path_len - 2);
 	/* -1 since last byte is buf[0] which is sent below (path) */
 	iov[0].iov_len--;
-	if (uni_path_len % 8 != 0) {
-		copy_size = uni_path_len / 8 * 8;
-		if (copy_size < uni_path_len)
-			copy_size += 8;
-
-		copy_path = kzalloc(copy_size, GFP_KERNEL);
-		if (!copy_path)
-			return -ENOMEM;
-		memcpy((char *)copy_path, (const char *)path,
-			uni_path_len);
+
+	req->NameOffset = cpu_to_le16(sizeof(struct smb2_create_req) - 4);
+
+	/* [MS-SMB2] 2.2.13 NameOffset:
+	 * If SMB2_FLAGS_DFS_OPERATIONS is set in the Flags field of
+	 * the SMB2 header, the file name includes a prefix that will
+	 * be processed during DFS name normalization as specified in
+	 * section 3.3.5.9. Otherwise, the file name is relative to
+	 * the share that is identified by the TreeId in the SMB2
+	 * header.
+	 */
+	if (tcon->share_flags & SHI1005_FLAGS_DFS) {
+		int name_len;
+
+		req->hdr.sync_hdr.Flags |= SMB2_FLAGS_DFS_OPERATIONS;
+		rc = alloc_path_with_tree_prefix(&copy_path, &copy_size,
+						 &name_len,
+						 tcon->treeName, path);
+		if (rc)
+			return rc;
+		req->NameLength = cpu_to_le16(name_len * 2);
 		uni_path_len = copy_size;
 		path = copy_path;
+	} else {
+		uni_path_len = (2 * UniStrnlen((wchar_t *)path, PATH_MAX)) + 2;
+		/* MUST set path len (NameLength) to 0 opening root of share */
+		req->NameLength = cpu_to_le16(uni_path_len - 2);
+		if (uni_path_len % 8 != 0) {
+			copy_size = roundup(uni_path_len, 8);
+			copy_path = kzalloc(copy_size, GFP_KERNEL);
+			if (!copy_path)
+				return -ENOMEM;
+			memcpy((char *)copy_path, (const char *)path,
+			       uni_path_len);
+			uni_path_len = copy_size;
+			path = copy_path;
+		}
 	}
 
 	iov[1].iov_len = uni_path_len;
Signature · 1 line
2.10.2
#17 lore raw

PATCH v2 6/6 CIFS: enable get_dfs_refer for SMB2+

In reply to a message not in the archive

Add the newly defined operation to the relevant structures.

---
 fs/cifs/smb2ops.c | 4 ++++
 1 file changed, 4 insertions(+)
fs/cifs/smb2ops.c +4 -0
index 2563fe8..1537cc8 100644
@@ -2326,6 +2326,7 @@ struct smb_version_operations smb20_operations = {
 	.clone_range = smb2_clone_range,
 	.wp_retry_size = smb2_wp_retry_size,
 	.dir_needs_close = smb2_dir_needs_close,
+	.get_dfs_refer = smb2_get_dfs_refer,
 };
 
 struct smb_version_operations smb21_operations = {
@@ -2407,6 +2408,7 @@ struct smb_version_operations smb21_operations = {
 	.wp_retry_size = smb2_wp_retry_size,
 	.dir_needs_close = smb2_dir_needs_close,
 	.enum_snapshots = smb3_enum_snapshots,
+	.get_dfs_refer = smb2_get_dfs_refer,
 };
 
 struct smb_version_operations smb30_operations = {
@@ -2498,6 +2500,7 @@ struct smb_version_operations smb30_operations = {
 	.free_transform_rq = smb3_free_transform_rq,
 	.is_transform_hdr = smb3_is_transform_hdr,
 	.receive_transform = smb3_receive_transform,
+	.get_dfs_refer = smb2_get_dfs_refer,
 };
 
 #ifdef CONFIG_CIFS_SMB311
@@ -2590,6 +2593,7 @@ struct smb_version_operations smb311_operations = {
 	.free_transform_rq = smb3_free_transform_rq,
 	.is_transform_hdr = smb3_is_transform_hdr,
 	.receive_transform = smb3_receive_transform,
+	.get_dfs_refer = smb2_get_dfs_refer,
 };
 #endif /* CIFS_SMB311 */
 
Signature · 1 line
2.10.2
Reviewed-by
#18 lore raw
In reply to a message not in the archive

2017-02-23 6:43 GMT-08:00 Aurelien Aptel <aaptel-IBi9RG/[email protected]>:

Quoted 81 lines

This series of patch tries to implement the get_dfs_refer operation for SMB2+.

In SMB2+, DFS resolving is now done through an FSCTL (patch #3). The relevant Microsoft specifications for this are:

MS-SMB2: 3.2.4.20 Application Requests an IO Control Code Operation
MS-SMB2: 3.2.4.20.3 Application Requests DFS Referral Information
MS-SMB2: 3.2.5.14 Receiving an SMB2 IOCTL Response
MS-SMB2: 3.2.5.14.4 Handling a DFS Referral Information Response

MS-DFSC: 2.2 Message Syntax (but really the whole document is useful)

The DFS response payload however is identical. Patch #1 moves the DFS response parsing out of SMB1 code and makes it work on any version of SMB.

DFS code has 2 "main" entry points: initial mounting and automount (when a DFS link is accessed/traversed).

When automounting, cifs.ko calls build_path_from_dentry() which only makes treename-prefixed paths when the tcon has the SMB_IN_DFS_SHARE. This flag is checked in tcon->Flags which is never touched by SMB2 code as it sets tcon->share_flags on connexion.

* CIFS requires to prefix all pathnames with the treename prefix when connected to a DFS server, so the current build_path_from_dentry() makes sense for CIFS.
* For SMB2+ it seems only the Create request requires the treename prefix. Simply making the function check for both CIFS SMB2+ flag for DFS to add a prefix does not work as the server has different expectations about which packet can have/require a DFS pathname.

Patch #2 adds build_path_from_dentry_optional_prefix() with an extra bool arg to decide to prefix or not. The automount code path always ask for it. Patch #5 modifies SMB2_open() to add the treename prefix to the given path if the server requires it.

As part of the mouting process a IPC tcon is made (I suspect we don't need it anymore in SMB3). This tcon doesn't respect the signing requirement. This is fixed by patch #4.

Finally the SMB2+ implementation of the get_dfs_referral operation is added in all supported SMB versions in patch #6.

I've sucessfuly tested this against a Windows Server 2016 DFS setup. Samba requires a patch as currently it only accepts DFS referrals requests on an IPC connexion, which is in violation of the spec. A patch was sent on samba-technical:

https://lists.samba.org/archive/samba-technical/2017-February/118859.html

Changes since v1:

* The fsctl is made on the first available tcon to the server, which is allowed by the spec. This way we avoid hardcoding additional rules in SMB2_ioctl().
* add signing flag in smb2 header if required
* no copy/pasting, the DFS parsing code is now generic and works for both SMB versions
* add treename prefix in SMB2_open() when required

Aurelien Aptel (6):
CIFS: move DFS response parsing out of SMB1 code
CIFS: add build_path_from_dentry_optional_prefix()
CIFS: implement get_dfs_refer for SMB2+
CIFS: set signing flag in SMB2+ TreeConnect if needed
CIFS: use DFS pathnames in SMB2+ Create requests
CIFS: enable get_dfs_refer for SMB2+

 fs/cifs/cifs_dfs_ref.c |   4 +-
 fs/cifs/cifspdu.h      |  16 ++++---
 fs/cifs/cifsproto.h    |   7 +++
 fs/cifs/cifssmb.c      | 119 +++----------------------------------------------
 fs/cifs/dir.c          |  13 +++++-
 fs/cifs/misc.c         | 105 +++++++++++++++++++++++++++++++++++++++++++
 fs/cifs/smb2ops.c      |  76 +++++++++++++++++++++++++++++++
 fs/cifs/smb2pdu.c      |  97 ++++++++++++++++++++++++++++++++--------
 fs/cifs/smb2pdu.h      |   8 ++++
 9 files changed, 305 insertions(+), 140 deletions(-)
Quoted 7 lines

--
2.10.2

--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

I would merge patches #3 and #6 because a static function which is introduced in #3 should raise compiler warnings. Other than that the patchset looks good.

Signature · 2 lines
Best regards,
Pavel Shilovsky