linux-cifs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Add support for setting owner info, dos attributes, and create time
@ 2019-12-19 16:52 Boris Protopopov
  2020-01-09 19:10 ` Steve French
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Boris Protopopov @ 2019-12-19 16:52 UTC (permalink / raw)
  To: linux-cifs
  Cc: sblbir, boris.v.protopopov, Boris Protopopov, Steve French,
	samba-technical, linux-kernel

Add extended attribute "system.cifs_ntsd" (and alias "system.smb3_ntsd")
to allow for setting owner and DACL in the security descriptor. This is in
addition to the existing "system.cifs_acl" and "system.smb3_acl" attributes
that allow for setting DACL only. Add support for setting creation time and
dos attributes using set_file_info() calls to complement the existing
support for getting these attributes via query_path_info() calls. 

Signed-off-by: Boris Protopopov <bprotopopov@hotmail.com>
---
 fs/cifs/xattr.c | 128 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 117 insertions(+), 11 deletions(-)

diff --git a/fs/cifs/xattr.c b/fs/cifs/xattr.c
index 9076150758d8..c41856e6fa22 100644
--- a/fs/cifs/xattr.c
+++ b/fs/cifs/xattr.c
@@ -32,7 +32,8 @@
 #include "cifs_unicode.h"
 
 #define MAX_EA_VALUE_SIZE CIFSMaxBufSize
-#define CIFS_XATTR_CIFS_ACL "system.cifs_acl"
+#define CIFS_XATTR_CIFS_ACL "system.cifs_acl" /* DACL only */
+#define CIFS_XATTR_CIFS_NTSD "system.cifs_ntsd" /* owner plus DACL */
 #define CIFS_XATTR_ATTRIB "cifs.dosattrib"  /* full name: user.cifs.dosattrib */
 #define CIFS_XATTR_CREATETIME "cifs.creationtime"  /* user.cifs.creationtime */
 /*
@@ -40,12 +41,62 @@
  * confusing users and using the 20+ year old term 'cifs' when it is no longer
  * secure, replaced by SMB2 (then even more highly secure SMB3) many years ago
  */
-#define SMB3_XATTR_CIFS_ACL "system.smb3_acl"
+#define SMB3_XATTR_CIFS_ACL "system.smb3_acl" /* DACL only */
+#define SMB3_XATTR_CIFS_NTSD "system.smb3_ntsd" /* owner plus DACL */
 #define SMB3_XATTR_ATTRIB "smb3.dosattrib"  /* full name: user.smb3.dosattrib */
 #define SMB3_XATTR_CREATETIME "smb3.creationtime"  /* user.smb3.creationtime */
 /* BB need to add server (Samba e.g) support for security and trusted prefix */
 
-enum { XATTR_USER, XATTR_CIFS_ACL, XATTR_ACL_ACCESS, XATTR_ACL_DEFAULT };
+enum { XATTR_USER, XATTR_CIFS_ACL, XATTR_ACL_ACCESS, XATTR_ACL_DEFAULT,
+	XATTR_CIFS_NTSD };
+
+static int cifs_attrib_set(unsigned int xid, struct cifs_tcon *pTcon,
+			   struct inode *inode, char *full_path,
+			   const void *value, size_t size)
+{
+	ssize_t rc = -EOPNOTSUPP;
+	__u32 *pattrib = (__u32 *)value;
+	__u32 attrib;
+	FILE_BASIC_INFO info_buf;
+
+	if ((value == NULL) || (size != sizeof(__u32)))
+		return -ERANGE;
+
+	memset(&info_buf, 0, sizeof(info_buf));
+	info_buf.Attributes = attrib = cpu_to_le32(*pattrib);
+
+	if (pTcon->ses->server->ops->set_file_info)
+		rc = pTcon->ses->server->ops->set_file_info(inode, full_path,
+				&info_buf, xid);
+	if (rc == 0)
+		CIFS_I(inode)->cifsAttrs = attrib;
+
+	return rc;
+}
+
+static int cifs_creation_time_set(unsigned int xid, struct cifs_tcon *pTcon,
+				  struct inode *inode, char *full_path,
+				  const void *value, size_t size)
+{
+	ssize_t rc = -EOPNOTSUPP;
+	__u64 *pcreation_time = (__u64 *)value;
+	__u64 creation_time;
+	FILE_BASIC_INFO info_buf;
+
+	if ((value == NULL) || (size != sizeof(__u64)))
+		return -ERANGE;
+
+	memset(&info_buf, 0, sizeof(info_buf));
+	info_buf.CreationTime = creation_time = cpu_to_le64(*pcreation_time);
+
+	if (pTcon->ses->server->ops->set_file_info)
+		rc = pTcon->ses->server->ops->set_file_info(inode, full_path,
+				&info_buf, xid);
+	if (rc == 0)
+		CIFS_I(inode)->createtime = creation_time;
+
+	return rc;
+}
 
 static int cifs_xattr_set(const struct xattr_handler *handler,
 			  struct dentry *dentry, struct inode *inode,
@@ -86,6 +137,23 @@ static int cifs_xattr_set(const struct xattr_handler *handler,
 
 	switch (handler->flags) {
 	case XATTR_USER:
+		cifs_dbg(FYI, "%s:setting user xattr %s\n", __func__, name);
+		if ((strcmp(name, CIFS_XATTR_ATTRIB) == 0) ||
+		    (strcmp(name, SMB3_XATTR_ATTRIB) == 0)) {
+			rc = cifs_attrib_set(xid, pTcon, inode, full_path,
+					value, size);
+			if (rc == 0) /* force revalidate of the inode */
+				CIFS_I(inode)->time = 0;
+			break;
+		} else if ((strcmp(name, CIFS_XATTR_CREATETIME) == 0) ||
+			   (strcmp(name, SMB3_XATTR_CREATETIME) == 0)) {
+			rc = cifs_creation_time_set(xid, pTcon, inode,
+					full_path, value, size);
+			if (rc == 0) /* force revalidate of the inode */
+				CIFS_I(inode)->time = 0;
+			break;
+		}
+
 		if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_XATTR)
 			goto out;
 
@@ -95,7 +163,8 @@ static int cifs_xattr_set(const struct xattr_handler *handler,
 				cifs_sb->local_nls, cifs_sb);
 		break;
 
-	case XATTR_CIFS_ACL: {
+	case XATTR_CIFS_ACL:
+	case XATTR_CIFS_NTSD: {
 		struct cifs_ntsd *pacl;
 
 		if (!value)
@@ -106,12 +175,25 @@ static int cifs_xattr_set(const struct xattr_handler *handler,
 		} else {
 			memcpy(pacl, value, size);
 			if (value &&
-			    pTcon->ses->server->ops->set_acl)
-				rc = pTcon->ses->server->ops->set_acl(pacl,
-						size, inode,
-						full_path, CIFS_ACL_DACL);
-			else
+			    pTcon->ses->server->ops->set_acl) {
+				rc = 0;
+				if (handler->flags == XATTR_CIFS_NTSD) {
+					/* set owner and DACL */
+					rc = pTcon->ses->server->ops->set_acl(
+							pacl, size, inode,
+							full_path,
+							CIFS_ACL_OWNER);
+				}
+				if (rc == 0) {
+					/* set DACL */
+					rc = pTcon->ses->server->ops->set_acl(
+							pacl, size, inode,
+							full_path,
+							CIFS_ACL_DACL);
+				}
+			} else {
 				rc = -EOPNOTSUPP;
+			}
 			if (rc == 0) /* force revalidate of the inode */
 				CIFS_I(inode)->time = 0;
 			kfree(pacl);
@@ -179,7 +261,7 @@ static int cifs_creation_time_get(struct dentry *dentry, struct inode *inode,
 				  void *value, size_t size)
 {
 	ssize_t rc;
-	__u64 * pcreatetime;
+	__u64 *pcreatetime;
 
 	rc = cifs_revalidate_dentry_attr(dentry);
 	if (rc)
@@ -244,7 +326,9 @@ static int cifs_xattr_get(const struct xattr_handler *handler,
 				full_path, name, value, size, cifs_sb);
 		break;
 
-	case XATTR_CIFS_ACL: {
+	case XATTR_CIFS_ACL:
+	case XATTR_CIFS_NTSD: {
+		/* the whole ntsd is fetched regardless */
 		u32 acllen;
 		struct cifs_ntsd *pacl;
 
@@ -382,6 +466,26 @@ static const struct xattr_handler smb3_acl_xattr_handler = {
 	.set = cifs_xattr_set,
 };
 
+static const struct xattr_handler cifs_cifs_ntsd_xattr_handler = {
+	.name = CIFS_XATTR_CIFS_NTSD,
+	.flags = XATTR_CIFS_NTSD,
+	.get = cifs_xattr_get,
+	.set = cifs_xattr_set,
+};
+
+/*
+ * Although this is just an alias for the above, need to move away from
+ * confusing users and using the 20 year old term 'cifs' when it is no
+ * longer secure and was replaced by SMB2/SMB3 a long time ago, and
+ * SMB3 and later are highly secure.
+ */
+static const struct xattr_handler smb3_ntsd_xattr_handler = {
+	.name = SMB3_XATTR_CIFS_NTSD,
+	.flags = XATTR_CIFS_NTSD,
+	.get = cifs_xattr_get,
+	.set = cifs_xattr_set,
+};
+
 static const struct xattr_handler cifs_posix_acl_access_xattr_handler = {
 	.name = XATTR_NAME_POSIX_ACL_ACCESS,
 	.flags = XATTR_ACL_ACCESS,
@@ -401,6 +505,8 @@ const struct xattr_handler *cifs_xattr_handlers[] = {
 	&cifs_os2_xattr_handler,
 	&cifs_cifs_acl_xattr_handler,
 	&smb3_acl_xattr_handler, /* alias for above since avoiding "cifs" */
+	&cifs_cifs_ntsd_xattr_handler,
+	&smb3_ntsd_xattr_handler, /* alias for above since avoiding "cifs" */
 	&cifs_posix_acl_access_xattr_handler,
 	&cifs_posix_acl_default_xattr_handler,
 	NULL
-- 
2.14.5


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] Add support for setting owner info, dos attributes, and create time
  2019-12-19 16:52 [PATCH] Add support for setting owner info, dos attributes, and create time Boris Protopopov
@ 2020-01-09 19:10 ` Steve French
  2020-01-13 20:17   ` Boris Protopopov
  2020-01-13 20:26   ` Andreas Dilger
  2020-01-15 21:18 ` Steve French
  2020-01-17  2:25 ` Steve French
  2 siblings, 2 replies; 9+ messages in thread
From: Steve French @ 2020-01-09 19:10 UTC (permalink / raw)
  To: Boris Protopopov, CIFS
  Cc: samba-technical, linux-fsdevel, Pavel Shilovsky, David Howells

One loosely related question ...

Your patch adds the ability to set creation time (birth time) which
can be useful for backup/restore cases, but doesn't address the other
hole in Linux (the inability to restore a files ctime).

In Linux the ability to set timestamps seems quite limited (utimes
only allows setting mtime and atime).

Since setting all 4 timestamps is allowed over SMB3 (and all older
dialects as well), should we extend this to allow setting ctime not
just creation time over SMB3/CIFS mounts?

On Thu, Dec 19, 2019 at 10:53 AM Boris Protopopov
<boris.v.protopopov@gmail.com> wrote:
>
> Add extended attribute "system.cifs_ntsd" (and alias "system.smb3_ntsd")
> to allow for setting owner and DACL in the security descriptor. This is in
> addition to the existing "system.cifs_acl" and "system.smb3_acl" attributes
> that allow for setting DACL only. Add support for setting creation time and
> dos attributes using set_file_info() calls to complement the existing
> support for getting these attributes via query_path_info() calls.
>
> Signed-off-by: Boris Protopopov <bprotopopov@hotmail.com>
> ---
>  fs/cifs/xattr.c | 128 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 117 insertions(+), 11 deletions(-)
>
> diff --git a/fs/cifs/xattr.c b/fs/cifs/xattr.c
> index 9076150758d8..c41856e6fa22 100644
> --- a/fs/cifs/xattr.c
> +++ b/fs/cifs/xattr.c
> @@ -32,7 +32,8 @@
>  #include "cifs_unicode.h"
>
>  #define MAX_EA_VALUE_SIZE CIFSMaxBufSize
> -#define CIFS_XATTR_CIFS_ACL "system.cifs_acl"
> +#define CIFS_XATTR_CIFS_ACL "system.cifs_acl" /* DACL only */
> +#define CIFS_XATTR_CIFS_NTSD "system.cifs_ntsd" /* owner plus DACL */
>  #define CIFS_XATTR_ATTRIB "cifs.dosattrib"  /* full name: user.cifs.dosattrib */
>  #define CIFS_XATTR_CREATETIME "cifs.creationtime"  /* user.cifs.creationtime */
>  /*
> @@ -40,12 +41,62 @@
>   * confusing users and using the 20+ year old term 'cifs' when it is no longer
>   * secure, replaced by SMB2 (then even more highly secure SMB3) many years ago
>   */
> -#define SMB3_XATTR_CIFS_ACL "system.smb3_acl"
> +#define SMB3_XATTR_CIFS_ACL "system.smb3_acl" /* DACL only */
> +#define SMB3_XATTR_CIFS_NTSD "system.smb3_ntsd" /* owner plus DACL */
>  #define SMB3_XATTR_ATTRIB "smb3.dosattrib"  /* full name: user.smb3.dosattrib */
>  #define SMB3_XATTR_CREATETIME "smb3.creationtime"  /* user.smb3.creationtime */
>  /* BB need to add server (Samba e.g) support for security and trusted prefix */
>
> -enum { XATTR_USER, XATTR_CIFS_ACL, XATTR_ACL_ACCESS, XATTR_ACL_DEFAULT };
> +enum { XATTR_USER, XATTR_CIFS_ACL, XATTR_ACL_ACCESS, XATTR_ACL_DEFAULT,
> +       XATTR_CIFS_NTSD };
> +
> +static int cifs_attrib_set(unsigned int xid, struct cifs_tcon *pTcon,
> +                          struct inode *inode, char *full_path,
> +                          const void *value, size_t size)
> +{
> +       ssize_t rc = -EOPNOTSUPP;
> +       __u32 *pattrib = (__u32 *)value;
> +       __u32 attrib;
> +       FILE_BASIC_INFO info_buf;
> +
> +       if ((value == NULL) || (size != sizeof(__u32)))
> +               return -ERANGE;
> +
> +       memset(&info_buf, 0, sizeof(info_buf));
> +       info_buf.Attributes = attrib = cpu_to_le32(*pattrib);
> +
> +       if (pTcon->ses->server->ops->set_file_info)
> +               rc = pTcon->ses->server->ops->set_file_info(inode, full_path,
> +                               &info_buf, xid);
> +       if (rc == 0)
> +               CIFS_I(inode)->cifsAttrs = attrib;
> +
> +       return rc;
> +}
> +
> +static int cifs_creation_time_set(unsigned int xid, struct cifs_tcon *pTcon,
> +                                 struct inode *inode, char *full_path,
> +                                 const void *value, size_t size)
> +{
> +       ssize_t rc = -EOPNOTSUPP;
> +       __u64 *pcreation_time = (__u64 *)value;
> +       __u64 creation_time;
> +       FILE_BASIC_INFO info_buf;
> +
> +       if ((value == NULL) || (size != sizeof(__u64)))
> +               return -ERANGE;
> +
> +       memset(&info_buf, 0, sizeof(info_buf));
> +       info_buf.CreationTime = creation_time = cpu_to_le64(*pcreation_time);
> +
> +       if (pTcon->ses->server->ops->set_file_info)
> +               rc = pTcon->ses->server->ops->set_file_info(inode, full_path,
> +                               &info_buf, xid);
> +       if (rc == 0)
> +               CIFS_I(inode)->createtime = creation_time;
> +
> +       return rc;
> +}
>
>  static int cifs_xattr_set(const struct xattr_handler *handler,
>                           struct dentry *dentry, struct inode *inode,
> @@ -86,6 +137,23 @@ static int cifs_xattr_set(const struct xattr_handler *handler,
>
>         switch (handler->flags) {
>         case XATTR_USER:
> +               cifs_dbg(FYI, "%s:setting user xattr %s\n", __func__, name);
> +               if ((strcmp(name, CIFS_XATTR_ATTRIB) == 0) ||
> +                   (strcmp(name, SMB3_XATTR_ATTRIB) == 0)) {
> +                       rc = cifs_attrib_set(xid, pTcon, inode, full_path,
> +                                       value, size);
> +                       if (rc == 0) /* force revalidate of the inode */
> +                               CIFS_I(inode)->time = 0;
> +                       break;
> +               } else if ((strcmp(name, CIFS_XATTR_CREATETIME) == 0) ||
> +                          (strcmp(name, SMB3_XATTR_CREATETIME) == 0)) {
> +                       rc = cifs_creation_time_set(xid, pTcon, inode,
> +                                       full_path, value, size);
> +                       if (rc == 0) /* force revalidate of the inode */
> +                               CIFS_I(inode)->time = 0;
> +                       break;
> +               }
> +
>                 if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_XATTR)
>                         goto out;
>
> @@ -95,7 +163,8 @@ static int cifs_xattr_set(const struct xattr_handler *handler,
>                                 cifs_sb->local_nls, cifs_sb);
>                 break;
>
> -       case XATTR_CIFS_ACL: {
> +       case XATTR_CIFS_ACL:
> +       case XATTR_CIFS_NTSD: {
>                 struct cifs_ntsd *pacl;
>
>                 if (!value)
> @@ -106,12 +175,25 @@ static int cifs_xattr_set(const struct xattr_handler *handler,
>                 } else {
>                         memcpy(pacl, value, size);
>                         if (value &&
> -                           pTcon->ses->server->ops->set_acl)
> -                               rc = pTcon->ses->server->ops->set_acl(pacl,
> -                                               size, inode,
> -                                               full_path, CIFS_ACL_DACL);
> -                       else
> +                           pTcon->ses->server->ops->set_acl) {
> +                               rc = 0;
> +                               if (handler->flags == XATTR_CIFS_NTSD) {
> +                                       /* set owner and DACL */
> +                                       rc = pTcon->ses->server->ops->set_acl(
> +                                                       pacl, size, inode,
> +                                                       full_path,
> +                                                       CIFS_ACL_OWNER);
> +                               }
> +                               if (rc == 0) {
> +                                       /* set DACL */
> +                                       rc = pTcon->ses->server->ops->set_acl(
> +                                                       pacl, size, inode,
> +                                                       full_path,
> +                                                       CIFS_ACL_DACL);
> +                               }
> +                       } else {
>                                 rc = -EOPNOTSUPP;
> +                       }
>                         if (rc == 0) /* force revalidate of the inode */
>                                 CIFS_I(inode)->time = 0;
>                         kfree(pacl);
> @@ -179,7 +261,7 @@ static int cifs_creation_time_get(struct dentry *dentry, struct inode *inode,
>                                   void *value, size_t size)
>  {
>         ssize_t rc;
> -       __u64 * pcreatetime;
> +       __u64 *pcreatetime;
>
>         rc = cifs_revalidate_dentry_attr(dentry);
>         if (rc)
> @@ -244,7 +326,9 @@ static int cifs_xattr_get(const struct xattr_handler *handler,
>                                 full_path, name, value, size, cifs_sb);
>                 break;
>
> -       case XATTR_CIFS_ACL: {
> +       case XATTR_CIFS_ACL:
> +       case XATTR_CIFS_NTSD: {
> +               /* the whole ntsd is fetched regardless */
>                 u32 acllen;
>                 struct cifs_ntsd *pacl;
>
> @@ -382,6 +466,26 @@ static const struct xattr_handler smb3_acl_xattr_handler = {
>         .set = cifs_xattr_set,
>  };
>
> +static const struct xattr_handler cifs_cifs_ntsd_xattr_handler = {
> +       .name = CIFS_XATTR_CIFS_NTSD,
> +       .flags = XATTR_CIFS_NTSD,
> +       .get = cifs_xattr_get,
> +       .set = cifs_xattr_set,
> +};
> +
> +/*
> + * Although this is just an alias for the above, need to move away from
> + * confusing users and using the 20 year old term 'cifs' when it is no
> + * longer secure and was replaced by SMB2/SMB3 a long time ago, and
> + * SMB3 and later are highly secure.
> + */
> +static const struct xattr_handler smb3_ntsd_xattr_handler = {
> +       .name = SMB3_XATTR_CIFS_NTSD,
> +       .flags = XATTR_CIFS_NTSD,
> +       .get = cifs_xattr_get,
> +       .set = cifs_xattr_set,
> +};
> +
>  static const struct xattr_handler cifs_posix_acl_access_xattr_handler = {
>         .name = XATTR_NAME_POSIX_ACL_ACCESS,
>         .flags = XATTR_ACL_ACCESS,
> @@ -401,6 +505,8 @@ const struct xattr_handler *cifs_xattr_handlers[] = {
>         &cifs_os2_xattr_handler,
>         &cifs_cifs_acl_xattr_handler,
>         &smb3_acl_xattr_handler, /* alias for above since avoiding "cifs" */
> +       &cifs_cifs_ntsd_xattr_handler,
> +       &smb3_ntsd_xattr_handler, /* alias for above since avoiding "cifs" */
>         &cifs_posix_acl_access_xattr_handler,
>         &cifs_posix_acl_default_xattr_handler,
>         NULL
> --
> 2.14.5
>


-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Add support for setting owner info, dos attributes, and create time
  2020-01-09 19:10 ` Steve French
@ 2020-01-13 20:17   ` Boris Protopopov
  2020-01-13 20:26   ` Andreas Dilger
  1 sibling, 0 replies; 9+ messages in thread
From: Boris Protopopov @ 2020-01-13 20:17 UTC (permalink / raw)
  To: Steve French
  Cc: CIFS, samba-technical, linux-fsdevel, Pavel Shilovsky, David Howells

I have not looked into that, but seems like a good idea. Would this be
done via a well-known extended attributes as well ?

On Thu, Jan 9, 2020 at 2:10 PM Steve French <smfrench@gmail.com> wrote:
>
> One loosely related question ...
>
> Your patch adds the ability to set creation time (birth time) which
> can be useful for backup/restore cases, but doesn't address the other
> hole in Linux (the inability to restore a files ctime).
>
> In Linux the ability to set timestamps seems quite limited (utimes
> only allows setting mtime and atime).
>
> Since setting all 4 timestamps is allowed over SMB3 (and all older
> dialects as well), should we extend this to allow setting ctime not
> just creation time over SMB3/CIFS mounts?
>
> On Thu, Dec 19, 2019 at 10:53 AM Boris Protopopov
> <boris.v.protopopov@gmail.com> wrote:
> >
> > Add extended attribute "system.cifs_ntsd" (and alias "system.smb3_ntsd")
> > to allow for setting owner and DACL in the security descriptor. This is in
> > addition to the existing "system.cifs_acl" and "system.smb3_acl" attributes
> > that allow for setting DACL only. Add support for setting creation time and
> > dos attributes using set_file_info() calls to complement the existing
> > support for getting these attributes via query_path_info() calls.
> >
> > Signed-off-by: Boris Protopopov <bprotopopov@hotmail.com>
> > ---
> >  fs/cifs/xattr.c | 128 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
> >  1 file changed, 117 insertions(+), 11 deletions(-)
> >
> > diff --git a/fs/cifs/xattr.c b/fs/cifs/xattr.c
> > index 9076150758d8..c41856e6fa22 100644
> > --- a/fs/cifs/xattr.c
> > +++ b/fs/cifs/xattr.c
> > @@ -32,7 +32,8 @@
> >  #include "cifs_unicode.h"
> >
> >  #define MAX_EA_VALUE_SIZE CIFSMaxBufSize
> > -#define CIFS_XATTR_CIFS_ACL "system.cifs_acl"
> > +#define CIFS_XATTR_CIFS_ACL "system.cifs_acl" /* DACL only */
> > +#define CIFS_XATTR_CIFS_NTSD "system.cifs_ntsd" /* owner plus DACL */
> >  #define CIFS_XATTR_ATTRIB "cifs.dosattrib"  /* full name: user.cifs.dosattrib */
> >  #define CIFS_XATTR_CREATETIME "cifs.creationtime"  /* user.cifs.creationtime */
> >  /*
> > @@ -40,12 +41,62 @@
> >   * confusing users and using the 20+ year old term 'cifs' when it is no longer
> >   * secure, replaced by SMB2 (then even more highly secure SMB3) many years ago
> >   */
> > -#define SMB3_XATTR_CIFS_ACL "system.smb3_acl"
> > +#define SMB3_XATTR_CIFS_ACL "system.smb3_acl" /* DACL only */
> > +#define SMB3_XATTR_CIFS_NTSD "system.smb3_ntsd" /* owner plus DACL */
> >  #define SMB3_XATTR_ATTRIB "smb3.dosattrib"  /* full name: user.smb3.dosattrib */
> >  #define SMB3_XATTR_CREATETIME "smb3.creationtime"  /* user.smb3.creationtime */
> >  /* BB need to add server (Samba e.g) support for security and trusted prefix */
> >
> > -enum { XATTR_USER, XATTR_CIFS_ACL, XATTR_ACL_ACCESS, XATTR_ACL_DEFAULT };
> > +enum { XATTR_USER, XATTR_CIFS_ACL, XATTR_ACL_ACCESS, XATTR_ACL_DEFAULT,
> > +       XATTR_CIFS_NTSD };
> > +
> > +static int cifs_attrib_set(unsigned int xid, struct cifs_tcon *pTcon,
> > +                          struct inode *inode, char *full_path,
> > +                          const void *value, size_t size)
> > +{
> > +       ssize_t rc = -EOPNOTSUPP;
> > +       __u32 *pattrib = (__u32 *)value;
> > +       __u32 attrib;
> > +       FILE_BASIC_INFO info_buf;
> > +
> > +       if ((value == NULL) || (size != sizeof(__u32)))
> > +               return -ERANGE;
> > +
> > +       memset(&info_buf, 0, sizeof(info_buf));
> > +       info_buf.Attributes = attrib = cpu_to_le32(*pattrib);
> > +
> > +       if (pTcon->ses->server->ops->set_file_info)
> > +               rc = pTcon->ses->server->ops->set_file_info(inode, full_path,
> > +                               &info_buf, xid);
> > +       if (rc == 0)
> > +               CIFS_I(inode)->cifsAttrs = attrib;
> > +
> > +       return rc;
> > +}
> > +
> > +static int cifs_creation_time_set(unsigned int xid, struct cifs_tcon *pTcon,
> > +                                 struct inode *inode, char *full_path,
> > +                                 const void *value, size_t size)
> > +{
> > +       ssize_t rc = -EOPNOTSUPP;
> > +       __u64 *pcreation_time = (__u64 *)value;
> > +       __u64 creation_time;
> > +       FILE_BASIC_INFO info_buf;
> > +
> > +       if ((value == NULL) || (size != sizeof(__u64)))
> > +               return -ERANGE;
> > +
> > +       memset(&info_buf, 0, sizeof(info_buf));
> > +       info_buf.CreationTime = creation_time = cpu_to_le64(*pcreation_time);
> > +
> > +       if (pTcon->ses->server->ops->set_file_info)
> > +               rc = pTcon->ses->server->ops->set_file_info(inode, full_path,
> > +                               &info_buf, xid);
> > +       if (rc == 0)
> > +               CIFS_I(inode)->createtime = creation_time;
> > +
> > +       return rc;
> > +}
> >
> >  static int cifs_xattr_set(const struct xattr_handler *handler,
> >                           struct dentry *dentry, struct inode *inode,
> > @@ -86,6 +137,23 @@ static int cifs_xattr_set(const struct xattr_handler *handler,
> >
> >         switch (handler->flags) {
> >         case XATTR_USER:
> > +               cifs_dbg(FYI, "%s:setting user xattr %s\n", __func__, name);
> > +               if ((strcmp(name, CIFS_XATTR_ATTRIB) == 0) ||
> > +                   (strcmp(name, SMB3_XATTR_ATTRIB) == 0)) {
> > +                       rc = cifs_attrib_set(xid, pTcon, inode, full_path,
> > +                                       value, size);
> > +                       if (rc == 0) /* force revalidate of the inode */
> > +                               CIFS_I(inode)->time = 0;
> > +                       break;
> > +               } else if ((strcmp(name, CIFS_XATTR_CREATETIME) == 0) ||
> > +                          (strcmp(name, SMB3_XATTR_CREATETIME) == 0)) {
> > +                       rc = cifs_creation_time_set(xid, pTcon, inode,
> > +                                       full_path, value, size);
> > +                       if (rc == 0) /* force revalidate of the inode */
> > +                               CIFS_I(inode)->time = 0;
> > +                       break;
> > +               }
> > +
> >                 if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_XATTR)
> >                         goto out;
> >
> > @@ -95,7 +163,8 @@ static int cifs_xattr_set(const struct xattr_handler *handler,
> >                                 cifs_sb->local_nls, cifs_sb);
> >                 break;
> >
> > -       case XATTR_CIFS_ACL: {
> > +       case XATTR_CIFS_ACL:
> > +       case XATTR_CIFS_NTSD: {
> >                 struct cifs_ntsd *pacl;
> >
> >                 if (!value)
> > @@ -106,12 +175,25 @@ static int cifs_xattr_set(const struct xattr_handler *handler,
> >                 } else {
> >                         memcpy(pacl, value, size);
> >                         if (value &&
> > -                           pTcon->ses->server->ops->set_acl)
> > -                               rc = pTcon->ses->server->ops->set_acl(pacl,
> > -                                               size, inode,
> > -                                               full_path, CIFS_ACL_DACL);
> > -                       else
> > +                           pTcon->ses->server->ops->set_acl) {
> > +                               rc = 0;
> > +                               if (handler->flags == XATTR_CIFS_NTSD) {
> > +                                       /* set owner and DACL */
> > +                                       rc = pTcon->ses->server->ops->set_acl(
> > +                                                       pacl, size, inode,
> > +                                                       full_path,
> > +                                                       CIFS_ACL_OWNER);
> > +                               }
> > +                               if (rc == 0) {
> > +                                       /* set DACL */
> > +                                       rc = pTcon->ses->server->ops->set_acl(
> > +                                                       pacl, size, inode,
> > +                                                       full_path,
> > +                                                       CIFS_ACL_DACL);
> > +                               }
> > +                       } else {
> >                                 rc = -EOPNOTSUPP;
> > +                       }
> >                         if (rc == 0) /* force revalidate of the inode */
> >                                 CIFS_I(inode)->time = 0;
> >                         kfree(pacl);
> > @@ -179,7 +261,7 @@ static int cifs_creation_time_get(struct dentry *dentry, struct inode *inode,
> >                                   void *value, size_t size)
> >  {
> >         ssize_t rc;
> > -       __u64 * pcreatetime;
> > +       __u64 *pcreatetime;
> >
> >         rc = cifs_revalidate_dentry_attr(dentry);
> >         if (rc)
> > @@ -244,7 +326,9 @@ static int cifs_xattr_get(const struct xattr_handler *handler,
> >                                 full_path, name, value, size, cifs_sb);
> >                 break;
> >
> > -       case XATTR_CIFS_ACL: {
> > +       case XATTR_CIFS_ACL:
> > +       case XATTR_CIFS_NTSD: {
> > +               /* the whole ntsd is fetched regardless */
> >                 u32 acllen;
> >                 struct cifs_ntsd *pacl;
> >
> > @@ -382,6 +466,26 @@ static const struct xattr_handler smb3_acl_xattr_handler = {
> >         .set = cifs_xattr_set,
> >  };
> >
> > +static const struct xattr_handler cifs_cifs_ntsd_xattr_handler = {
> > +       .name = CIFS_XATTR_CIFS_NTSD,
> > +       .flags = XATTR_CIFS_NTSD,
> > +       .get = cifs_xattr_get,
> > +       .set = cifs_xattr_set,
> > +};
> > +
> > +/*
> > + * Although this is just an alias for the above, need to move away from
> > + * confusing users and using the 20 year old term 'cifs' when it is no
> > + * longer secure and was replaced by SMB2/SMB3 a long time ago, and
> > + * SMB3 and later are highly secure.
> > + */
> > +static const struct xattr_handler smb3_ntsd_xattr_handler = {
> > +       .name = SMB3_XATTR_CIFS_NTSD,
> > +       .flags = XATTR_CIFS_NTSD,
> > +       .get = cifs_xattr_get,
> > +       .set = cifs_xattr_set,
> > +};
> > +
> >  static const struct xattr_handler cifs_posix_acl_access_xattr_handler = {
> >         .name = XATTR_NAME_POSIX_ACL_ACCESS,
> >         .flags = XATTR_ACL_ACCESS,
> > @@ -401,6 +505,8 @@ const struct xattr_handler *cifs_xattr_handlers[] = {
> >         &cifs_os2_xattr_handler,
> >         &cifs_cifs_acl_xattr_handler,
> >         &smb3_acl_xattr_handler, /* alias for above since avoiding "cifs" */
> > +       &cifs_cifs_ntsd_xattr_handler,
> > +       &smb3_ntsd_xattr_handler, /* alias for above since avoiding "cifs" */
> >         &cifs_posix_acl_access_xattr_handler,
> >         &cifs_posix_acl_default_xattr_handler,
> >         NULL
> > --
> > 2.14.5
> >
>
>
> --
> Thanks,
>
> Steve

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Add support for setting owner info, dos attributes, and create time
  2020-01-09 19:10 ` Steve French
  2020-01-13 20:17   ` Boris Protopopov
@ 2020-01-13 20:26   ` Andreas Dilger
  2020-01-13 20:36     ` Jeremy Allison
  1 sibling, 1 reply; 9+ messages in thread
From: Andreas Dilger @ 2020-01-13 20:26 UTC (permalink / raw)
  To: Steve French
  Cc: Boris Protopopov, CIFS, samba-technical, linux-fsdevel,
	Pavel Shilovsky, David Howells

[-- Attachment #1: Type: text/plain, Size: 11300 bytes --]

On Jan 9, 2020, at 12:10 PM, Steve French <smfrench@gmail.com> wrote:
> 
> One loosely related question ...
> 
> Your patch adds the ability to set creation time (birth time) which
> can be useful for backup/restore cases, but doesn't address the other
> hole in Linux (the inability to restore a files ctime).
> 
> In Linux the ability to set timestamps seems quite limited (utimes
> only allows setting mtime and atime).

The whole point of not being able to change ctime and btime as a regular
user is so that it is possible to determine when a file was actually
created on the filesystem and last modified.  That is often useful for
debugging or forensics reasons.

I think if this is something that SMB/CIFS wants to do, it should save
these attributes into an xattr of its own (e.g. user.dos or whatever),
rather than using the ctime and btime(crtime) fields in the filesystem.

Cheers, Andreas

> Since setting all 4 timestamps is allowed over SMB3 (and all older
> dialects as well), should we extend this to allow setting ctime not
> just creation time over SMB3/CIFS mounts?
> 
> On Thu, Dec 19, 2019 at 10:53 AM Boris Protopopov
> <boris.v.protopopov@gmail.com> wrote:
>> 
>> Add extended attribute "system.cifs_ntsd" (and alias "system.smb3_ntsd")
>> to allow for setting owner and DACL in the security descriptor. This is in
>> addition to the existing "system.cifs_acl" and "system.smb3_acl" attributes
>> that allow for setting DACL only. Add support for setting creation time and
>> dos attributes using set_file_info() calls to complement the existing
>> support for getting these attributes via query_path_info() calls.
>> 
>> Signed-off-by: Boris Protopopov <bprotopopov@hotmail.com>
>> ---
>> fs/cifs/xattr.c | 128 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
>> 1 file changed, 117 insertions(+), 11 deletions(-)
>> 
>> diff --git a/fs/cifs/xattr.c b/fs/cifs/xattr.c
>> index 9076150758d8..c41856e6fa22 100644
>> --- a/fs/cifs/xattr.c
>> +++ b/fs/cifs/xattr.c
>> @@ -32,7 +32,8 @@
>> #include "cifs_unicode.h"
>> 
>> #define MAX_EA_VALUE_SIZE CIFSMaxBufSize
>> -#define CIFS_XATTR_CIFS_ACL "system.cifs_acl"
>> +#define CIFS_XATTR_CIFS_ACL "system.cifs_acl" /* DACL only */
>> +#define CIFS_XATTR_CIFS_NTSD "system.cifs_ntsd" /* owner plus DACL */
>> #define CIFS_XATTR_ATTRIB "cifs.dosattrib"  /* full name: user.cifs.dosattrib */
>> #define CIFS_XATTR_CREATETIME "cifs.creationtime"  /* user.cifs.creationtime */
>> /*
>> @@ -40,12 +41,62 @@
>>  * confusing users and using the 20+ year old term 'cifs' when it is no longer
>>  * secure, replaced by SMB2 (then even more highly secure SMB3) many years ago
>>  */
>> -#define SMB3_XATTR_CIFS_ACL "system.smb3_acl"
>> +#define SMB3_XATTR_CIFS_ACL "system.smb3_acl" /* DACL only */
>> +#define SMB3_XATTR_CIFS_NTSD "system.smb3_ntsd" /* owner plus DACL */
>> #define SMB3_XATTR_ATTRIB "smb3.dosattrib"  /* full name: user.smb3.dosattrib */
>> #define SMB3_XATTR_CREATETIME "smb3.creationtime"  /* user.smb3.creationtime */
>> /* BB need to add server (Samba e.g) support for security and trusted prefix */
>> 
>> -enum { XATTR_USER, XATTR_CIFS_ACL, XATTR_ACL_ACCESS, XATTR_ACL_DEFAULT };
>> +enum { XATTR_USER, XATTR_CIFS_ACL, XATTR_ACL_ACCESS, XATTR_ACL_DEFAULT,
>> +       XATTR_CIFS_NTSD };
>> +
>> +static int cifs_attrib_set(unsigned int xid, struct cifs_tcon *pTcon,
>> +                          struct inode *inode, char *full_path,
>> +                          const void *value, size_t size)
>> +{
>> +       ssize_t rc = -EOPNOTSUPP;
>> +       __u32 *pattrib = (__u32 *)value;
>> +       __u32 attrib;
>> +       FILE_BASIC_INFO info_buf;
>> +
>> +       if ((value == NULL) || (size != sizeof(__u32)))
>> +               return -ERANGE;
>> +
>> +       memset(&info_buf, 0, sizeof(info_buf));
>> +       info_buf.Attributes = attrib = cpu_to_le32(*pattrib);
>> +
>> +       if (pTcon->ses->server->ops->set_file_info)
>> +               rc = pTcon->ses->server->ops->set_file_info(inode, full_path,
>> +                               &info_buf, xid);
>> +       if (rc == 0)
>> +               CIFS_I(inode)->cifsAttrs = attrib;
>> +
>> +       return rc;
>> +}
>> +
>> +static int cifs_creation_time_set(unsigned int xid, struct cifs_tcon *pTcon,
>> +                                 struct inode *inode, char *full_path,
>> +                                 const void *value, size_t size)
>> +{
>> +       ssize_t rc = -EOPNOTSUPP;
>> +       __u64 *pcreation_time = (__u64 *)value;
>> +       __u64 creation_time;
>> +       FILE_BASIC_INFO info_buf;
>> +
>> +       if ((value == NULL) || (size != sizeof(__u64)))
>> +               return -ERANGE;
>> +
>> +       memset(&info_buf, 0, sizeof(info_buf));
>> +       info_buf.CreationTime = creation_time = cpu_to_le64(*pcreation_time);
>> +
>> +       if (pTcon->ses->server->ops->set_file_info)
>> +               rc = pTcon->ses->server->ops->set_file_info(inode, full_path,
>> +                               &info_buf, xid);
>> +       if (rc == 0)
>> +               CIFS_I(inode)->createtime = creation_time;
>> +
>> +       return rc;
>> +}
>> 
>> static int cifs_xattr_set(const struct xattr_handler *handler,
>>                          struct dentry *dentry, struct inode *inode,
>> @@ -86,6 +137,23 @@ static int cifs_xattr_set(const struct xattr_handler *handler,
>> 
>>        switch (handler->flags) {
>>        case XATTR_USER:
>> +               cifs_dbg(FYI, "%s:setting user xattr %s\n", __func__, name);
>> +               if ((strcmp(name, CIFS_XATTR_ATTRIB) == 0) ||
>> +                   (strcmp(name, SMB3_XATTR_ATTRIB) == 0)) {
>> +                       rc = cifs_attrib_set(xid, pTcon, inode, full_path,
>> +                                       value, size);
>> +                       if (rc == 0) /* force revalidate of the inode */
>> +                               CIFS_I(inode)->time = 0;
>> +                       break;
>> +               } else if ((strcmp(name, CIFS_XATTR_CREATETIME) == 0) ||
>> +                          (strcmp(name, SMB3_XATTR_CREATETIME) == 0)) {
>> +                       rc = cifs_creation_time_set(xid, pTcon, inode,
>> +                                       full_path, value, size);
>> +                       if (rc == 0) /* force revalidate of the inode */
>> +                               CIFS_I(inode)->time = 0;
>> +                       break;
>> +               }
>> +
>>                if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_XATTR)
>>                        goto out;
>> 
>> @@ -95,7 +163,8 @@ static int cifs_xattr_set(const struct xattr_handler *handler,
>>                                cifs_sb->local_nls, cifs_sb);
>>                break;
>> 
>> -       case XATTR_CIFS_ACL: {
>> +       case XATTR_CIFS_ACL:
>> +       case XATTR_CIFS_NTSD: {
>>                struct cifs_ntsd *pacl;
>> 
>>                if (!value)
>> @@ -106,12 +175,25 @@ static int cifs_xattr_set(const struct xattr_handler *handler,
>>                } else {
>>                        memcpy(pacl, value, size);
>>                        if (value &&
>> -                           pTcon->ses->server->ops->set_acl)
>> -                               rc = pTcon->ses->server->ops->set_acl(pacl,
>> -                                               size, inode,
>> -                                               full_path, CIFS_ACL_DACL);
>> -                       else
>> +                           pTcon->ses->server->ops->set_acl) {
>> +                               rc = 0;
>> +                               if (handler->flags == XATTR_CIFS_NTSD) {
>> +                                       /* set owner and DACL */
>> +                                       rc = pTcon->ses->server->ops->set_acl(
>> +                                                       pacl, size, inode,
>> +                                                       full_path,
>> +                                                       CIFS_ACL_OWNER);
>> +                               }
>> +                               if (rc == 0) {
>> +                                       /* set DACL */
>> +                                       rc = pTcon->ses->server->ops->set_acl(
>> +                                                       pacl, size, inode,
>> +                                                       full_path,
>> +                                                       CIFS_ACL_DACL);
>> +                               }
>> +                       } else {
>>                                rc = -EOPNOTSUPP;
>> +                       }
>>                        if (rc == 0) /* force revalidate of the inode */
>>                                CIFS_I(inode)->time = 0;
>>                        kfree(pacl);
>> @@ -179,7 +261,7 @@ static int cifs_creation_time_get(struct dentry *dentry, struct inode *inode,
>>                                  void *value, size_t size)
>> {
>>        ssize_t rc;
>> -       __u64 * pcreatetime;
>> +       __u64 *pcreatetime;
>> 
>>        rc = cifs_revalidate_dentry_attr(dentry);
>>        if (rc)
>> @@ -244,7 +326,9 @@ static int cifs_xattr_get(const struct xattr_handler *handler,
>>                                full_path, name, value, size, cifs_sb);
>>                break;
>> 
>> -       case XATTR_CIFS_ACL: {
>> +       case XATTR_CIFS_ACL:
>> +       case XATTR_CIFS_NTSD: {
>> +               /* the whole ntsd is fetched regardless */
>>                u32 acllen;
>>                struct cifs_ntsd *pacl;
>> 
>> @@ -382,6 +466,26 @@ static const struct xattr_handler smb3_acl_xattr_handler = {
>>        .set = cifs_xattr_set,
>> };
>> 
>> +static const struct xattr_handler cifs_cifs_ntsd_xattr_handler = {
>> +       .name = CIFS_XATTR_CIFS_NTSD,
>> +       .flags = XATTR_CIFS_NTSD,
>> +       .get = cifs_xattr_get,
>> +       .set = cifs_xattr_set,
>> +};
>> +
>> +/*
>> + * Although this is just an alias for the above, need to move away from
>> + * confusing users and using the 20 year old term 'cifs' when it is no
>> + * longer secure and was replaced by SMB2/SMB3 a long time ago, and
>> + * SMB3 and later are highly secure.
>> + */
>> +static const struct xattr_handler smb3_ntsd_xattr_handler = {
>> +       .name = SMB3_XATTR_CIFS_NTSD,
>> +       .flags = XATTR_CIFS_NTSD,
>> +       .get = cifs_xattr_get,
>> +       .set = cifs_xattr_set,
>> +};
>> +
>> static const struct xattr_handler cifs_posix_acl_access_xattr_handler = {
>>        .name = XATTR_NAME_POSIX_ACL_ACCESS,
>>        .flags = XATTR_ACL_ACCESS,
>> @@ -401,6 +505,8 @@ const struct xattr_handler *cifs_xattr_handlers[] = {
>>        &cifs_os2_xattr_handler,
>>        &cifs_cifs_acl_xattr_handler,
>>        &smb3_acl_xattr_handler, /* alias for above since avoiding "cifs" */
>> +       &cifs_cifs_ntsd_xattr_handler,
>> +       &smb3_ntsd_xattr_handler, /* alias for above since avoiding "cifs" */
>>        &cifs_posix_acl_access_xattr_handler,
>>        &cifs_posix_acl_default_xattr_handler,
>>        NULL
>> --
>> 2.14.5
>> 
> 
> 
> --
> Thanks,
> 
> Steve


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Add support for setting owner info, dos attributes, and create time
  2020-01-13 20:26   ` Andreas Dilger
@ 2020-01-13 20:36     ` Jeremy Allison
  2020-01-13 22:17       ` Boris Protopopov
  0 siblings, 1 reply; 9+ messages in thread
From: Jeremy Allison @ 2020-01-13 20:36 UTC (permalink / raw)
  To: Andreas Dilger
  Cc: Steve French, CIFS, Boris Protopopov, samba-technical,
	David Howells, linux-fsdevel

On Mon, Jan 13, 2020 at 01:26:39PM -0700, Andreas Dilger via samba-technical wrote:
> On Jan 9, 2020, at 12:10 PM, Steve French <smfrench@gmail.com> wrote:
> > 
> > One loosely related question ...
> > 
> > Your patch adds the ability to set creation time (birth time) which
> > can be useful for backup/restore cases, but doesn't address the other
> > hole in Linux (the inability to restore a files ctime).
> > 
> > In Linux the ability to set timestamps seems quite limited (utimes
> > only allows setting mtime and atime).
> 
> The whole point of not being able to change ctime and btime as a regular
> user is so that it is possible to determine when a file was actually
> created on the filesystem and last modified.  That is often useful for
> debugging or forensics reasons.
> 
> I think if this is something that SMB/CIFS wants to do, it should save
> these attributes into an xattr of its own (e.g. user.dos or whatever),
> rather than using the ctime and btime(crtime) fields in the filesystem.

FYI, we (Samba) already do this for create time to store/fetch it
on systems and filesystems that don't store a create time. It's
easy to add extra info here.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Add support for setting owner info, dos attributes, and create time
  2020-01-13 20:36     ` Jeremy Allison
@ 2020-01-13 22:17       ` Boris Protopopov
  0 siblings, 0 replies; 9+ messages in thread
From: Boris Protopopov @ 2020-01-13 22:17 UTC (permalink / raw)
  To: Jeremy Allison
  Cc: Andreas Dilger, Steve French, CIFS, samba-technical,
	David Howells, linux-fsdevel

OK, I will look into adding ctime-related attribute, and model it after btime.

On Mon, Jan 13, 2020 at 3:36 PM Jeremy Allison <jra@samba.org> wrote:
>
> On Mon, Jan 13, 2020 at 01:26:39PM -0700, Andreas Dilger via samba-technical wrote:
> > On Jan 9, 2020, at 12:10 PM, Steve French <smfrench@gmail.com> wrote:
> > >
> > > One loosely related question ...
> > >
> > > Your patch adds the ability to set creation time (birth time) which
> > > can be useful for backup/restore cases, but doesn't address the other
> > > hole in Linux (the inability to restore a files ctime).
> > >
> > > In Linux the ability to set timestamps seems quite limited (utimes
> > > only allows setting mtime and atime).
> >
> > The whole point of not being able to change ctime and btime as a regular
> > user is so that it is possible to determine when a file was actually
> > created on the filesystem and last modified.  That is often useful for
> > debugging or forensics reasons.
> >
> > I think if this is something that SMB/CIFS wants to do, it should save
> > these attributes into an xattr of its own (e.g. user.dos or whatever),
> > rather than using the ctime and btime(crtime) fields in the filesystem.
>
> FYI, we (Samba) already do this for create time to store/fetch it
> on systems and filesystems that don't store a create time. It's
> easy to add extra info here.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Add support for setting owner info, dos attributes, and create time
  2019-12-19 16:52 [PATCH] Add support for setting owner info, dos attributes, and create time Boris Protopopov
  2020-01-09 19:10 ` Steve French
@ 2020-01-15 21:18 ` Steve French
  2020-01-16 21:20   ` Boris Protopopov
  2020-01-17  2:25 ` Steve French
  2 siblings, 1 reply; 9+ messages in thread
From: Steve French @ 2020-01-15 21:18 UTC (permalink / raw)
  To: Boris Protopopov
  Cc: CIFS, sblbir, Boris Protopopov, Steve French, samba-technical, LKML

lightly updated the description and merged into cifs-2.6.git for-next

Boris,
Might be helpful to add a small test (e.g. in the xfstest cifs subdir)
for this, Ronnie had added a few small ones for automated testing that
we use in the buildbot that are cifs specific and it is useful to
reduce the chance of future regressions

On Thu, Dec 19, 2019 at 10:53 AM Boris Protopopov
<boris.v.protopopov@gmail.com> wrote:
>
> Add extended attribute "system.cifs_ntsd" (and alias "system.smb3_ntsd")
> to allow for setting owner and DACL in the security descriptor. This is in
> addition to the existing "system.cifs_acl" and "system.smb3_acl" attributes
> that allow for setting DACL only. Add support for setting creation time and
> dos attributes using set_file_info() calls to complement the existing
> support for getting these attributes via query_path_info() calls.
>
> Signed-off-by: Boris Protopopov <bprotopopov@hotmail.com>
> ---
>  fs/cifs/xattr.c | 128 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 117 insertions(+), 11 deletions(-)
>
> diff --git a/fs/cifs/xattr.c b/fs/cifs/xattr.c
> index 9076150758d8..c41856e6fa22 100644
> --- a/fs/cifs/xattr.c
> +++ b/fs/cifs/xattr.c
> @@ -32,7 +32,8 @@
>  #include "cifs_unicode.h"
>
>  #define MAX_EA_VALUE_SIZE CIFSMaxBufSize
> -#define CIFS_XATTR_CIFS_ACL "system.cifs_acl"
> +#define CIFS_XATTR_CIFS_ACL "system.cifs_acl" /* DACL only */
> +#define CIFS_XATTR_CIFS_NTSD "system.cifs_ntsd" /* owner plus DACL */
>  #define CIFS_XATTR_ATTRIB "cifs.dosattrib"  /* full name: user.cifs.dosattrib */
>  #define CIFS_XATTR_CREATETIME "cifs.creationtime"  /* user.cifs.creationtime */
>  /*
> @@ -40,12 +41,62 @@
>   * confusing users and using the 20+ year old term 'cifs' when it is no longer
>   * secure, replaced by SMB2 (then even more highly secure SMB3) many years ago
>   */
> -#define SMB3_XATTR_CIFS_ACL "system.smb3_acl"
> +#define SMB3_XATTR_CIFS_ACL "system.smb3_acl" /* DACL only */
> +#define SMB3_XATTR_CIFS_NTSD "system.smb3_ntsd" /* owner plus DACL */
>  #define SMB3_XATTR_ATTRIB "smb3.dosattrib"  /* full name: user.smb3.dosattrib */
>  #define SMB3_XATTR_CREATETIME "smb3.creationtime"  /* user.smb3.creationtime */
>  /* BB need to add server (Samba e.g) support for security and trusted prefix */
>
> -enum { XATTR_USER, XATTR_CIFS_ACL, XATTR_ACL_ACCESS, XATTR_ACL_DEFAULT };
> +enum { XATTR_USER, XATTR_CIFS_ACL, XATTR_ACL_ACCESS, XATTR_ACL_DEFAULT,
> +       XATTR_CIFS_NTSD };
> +
> +static int cifs_attrib_set(unsigned int xid, struct cifs_tcon *pTcon,
> +                          struct inode *inode, char *full_path,
> +                          const void *value, size_t size)
> +{
> +       ssize_t rc = -EOPNOTSUPP;
> +       __u32 *pattrib = (__u32 *)value;
> +       __u32 attrib;
> +       FILE_BASIC_INFO info_buf;
> +
> +       if ((value == NULL) || (size != sizeof(__u32)))
> +               return -ERANGE;
> +
> +       memset(&info_buf, 0, sizeof(info_buf));
> +       info_buf.Attributes = attrib = cpu_to_le32(*pattrib);
> +
> +       if (pTcon->ses->server->ops->set_file_info)
> +               rc = pTcon->ses->server->ops->set_file_info(inode, full_path,
> +                               &info_buf, xid);
> +       if (rc == 0)
> +               CIFS_I(inode)->cifsAttrs = attrib;
> +
> +       return rc;
> +}
> +
> +static int cifs_creation_time_set(unsigned int xid, struct cifs_tcon *pTcon,
> +                                 struct inode *inode, char *full_path,
> +                                 const void *value, size_t size)
> +{
> +       ssize_t rc = -EOPNOTSUPP;
> +       __u64 *pcreation_time = (__u64 *)value;
> +       __u64 creation_time;
> +       FILE_BASIC_INFO info_buf;
> +
> +       if ((value == NULL) || (size != sizeof(__u64)))
> +               return -ERANGE;
> +
> +       memset(&info_buf, 0, sizeof(info_buf));
> +       info_buf.CreationTime = creation_time = cpu_to_le64(*pcreation_time);
> +
> +       if (pTcon->ses->server->ops->set_file_info)
> +               rc = pTcon->ses->server->ops->set_file_info(inode, full_path,
> +                               &info_buf, xid);
> +       if (rc == 0)
> +               CIFS_I(inode)->createtime = creation_time;
> +
> +       return rc;
> +}
>
>  static int cifs_xattr_set(const struct xattr_handler *handler,
>                           struct dentry *dentry, struct inode *inode,
> @@ -86,6 +137,23 @@ static int cifs_xattr_set(const struct xattr_handler *handler,
>
>         switch (handler->flags) {
>         case XATTR_USER:
> +               cifs_dbg(FYI, "%s:setting user xattr %s\n", __func__, name);
> +               if ((strcmp(name, CIFS_XATTR_ATTRIB) == 0) ||
> +                   (strcmp(name, SMB3_XATTR_ATTRIB) == 0)) {
> +                       rc = cifs_attrib_set(xid, pTcon, inode, full_path,
> +                                       value, size);
> +                       if (rc == 0) /* force revalidate of the inode */
> +                               CIFS_I(inode)->time = 0;
> +                       break;
> +               } else if ((strcmp(name, CIFS_XATTR_CREATETIME) == 0) ||
> +                          (strcmp(name, SMB3_XATTR_CREATETIME) == 0)) {
> +                       rc = cifs_creation_time_set(xid, pTcon, inode,
> +                                       full_path, value, size);
> +                       if (rc == 0) /* force revalidate of the inode */
> +                               CIFS_I(inode)->time = 0;
> +                       break;
> +               }
> +
>                 if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_XATTR)
>                         goto out;
>
> @@ -95,7 +163,8 @@ static int cifs_xattr_set(const struct xattr_handler *handler,
>                                 cifs_sb->local_nls, cifs_sb);
>                 break;
>
> -       case XATTR_CIFS_ACL: {
> +       case XATTR_CIFS_ACL:
> +       case XATTR_CIFS_NTSD: {
>                 struct cifs_ntsd *pacl;
>
>                 if (!value)
> @@ -106,12 +175,25 @@ static int cifs_xattr_set(const struct xattr_handler *handler,
>                 } else {
>                         memcpy(pacl, value, size);
>                         if (value &&
> -                           pTcon->ses->server->ops->set_acl)
> -                               rc = pTcon->ses->server->ops->set_acl(pacl,
> -                                               size, inode,
> -                                               full_path, CIFS_ACL_DACL);
> -                       else
> +                           pTcon->ses->server->ops->set_acl) {
> +                               rc = 0;
> +                               if (handler->flags == XATTR_CIFS_NTSD) {
> +                                       /* set owner and DACL */
> +                                       rc = pTcon->ses->server->ops->set_acl(
> +                                                       pacl, size, inode,
> +                                                       full_path,
> +                                                       CIFS_ACL_OWNER);
> +                               }
> +                               if (rc == 0) {
> +                                       /* set DACL */
> +                                       rc = pTcon->ses->server->ops->set_acl(
> +                                                       pacl, size, inode,
> +                                                       full_path,
> +                                                       CIFS_ACL_DACL);
> +                               }
> +                       } else {
>                                 rc = -EOPNOTSUPP;
> +                       }
>                         if (rc == 0) /* force revalidate of the inode */
>                                 CIFS_I(inode)->time = 0;
>                         kfree(pacl);
> @@ -179,7 +261,7 @@ static int cifs_creation_time_get(struct dentry *dentry, struct inode *inode,
>                                   void *value, size_t size)
>  {
>         ssize_t rc;
> -       __u64 * pcreatetime;
> +       __u64 *pcreatetime;
>
>         rc = cifs_revalidate_dentry_attr(dentry);
>         if (rc)
> @@ -244,7 +326,9 @@ static int cifs_xattr_get(const struct xattr_handler *handler,
>                                 full_path, name, value, size, cifs_sb);
>                 break;
>
> -       case XATTR_CIFS_ACL: {
> +       case XATTR_CIFS_ACL:
> +       case XATTR_CIFS_NTSD: {
> +               /* the whole ntsd is fetched regardless */
>                 u32 acllen;
>                 struct cifs_ntsd *pacl;
>
> @@ -382,6 +466,26 @@ static const struct xattr_handler smb3_acl_xattr_handler = {
>         .set = cifs_xattr_set,
>  };
>
> +static const struct xattr_handler cifs_cifs_ntsd_xattr_handler = {
> +       .name = CIFS_XATTR_CIFS_NTSD,
> +       .flags = XATTR_CIFS_NTSD,
> +       .get = cifs_xattr_get,
> +       .set = cifs_xattr_set,
> +};
> +
> +/*
> + * Although this is just an alias for the above, need to move away from
> + * confusing users and using the 20 year old term 'cifs' when it is no
> + * longer secure and was replaced by SMB2/SMB3 a long time ago, and
> + * SMB3 and later are highly secure.
> + */
> +static const struct xattr_handler smb3_ntsd_xattr_handler = {
> +       .name = SMB3_XATTR_CIFS_NTSD,
> +       .flags = XATTR_CIFS_NTSD,
> +       .get = cifs_xattr_get,
> +       .set = cifs_xattr_set,
> +};
> +
>  static const struct xattr_handler cifs_posix_acl_access_xattr_handler = {
>         .name = XATTR_NAME_POSIX_ACL_ACCESS,
>         .flags = XATTR_ACL_ACCESS,
> @@ -401,6 +505,8 @@ const struct xattr_handler *cifs_xattr_handlers[] = {
>         &cifs_os2_xattr_handler,
>         &cifs_cifs_acl_xattr_handler,
>         &smb3_acl_xattr_handler, /* alias for above since avoiding "cifs" */
> +       &cifs_cifs_ntsd_xattr_handler,
> +       &smb3_ntsd_xattr_handler, /* alias for above since avoiding "cifs" */
>         &cifs_posix_acl_access_xattr_handler,
>         &cifs_posix_acl_default_xattr_handler,
>         NULL
> --
> 2.14.5
>


-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Add support for setting owner info, dos attributes, and create time
  2020-01-15 21:18 ` Steve French
@ 2020-01-16 21:20   ` Boris Protopopov
  0 siblings, 0 replies; 9+ messages in thread
From: Boris Protopopov @ 2020-01-16 21:20 UTC (permalink / raw)
  To: Steve French
  Cc: CIFS, sblbir, Boris Protopopov, Steve French, samba-technical, LKML

OK, Steve, I will look into it.

On Wed, Jan 15, 2020 at 4:18 PM Steve French <smfrench@gmail.com> wrote:
>
> lightly updated the description and merged into cifs-2.6.git for-next
>
> Boris,
> Might be helpful to add a small test (e.g. in the xfstest cifs subdir)
> for this, Ronnie had added a few small ones for automated testing that
> we use in the buildbot that are cifs specific and it is useful to
> reduce the chance of future regressions
>
> On Thu, Dec 19, 2019 at 10:53 AM Boris Protopopov
> <boris.v.protopopov@gmail.com> wrote:
> >
> > Add extended attribute "system.cifs_ntsd" (and alias "system.smb3_ntsd")
> > to allow for setting owner and DACL in the security descriptor. This is in
> > addition to the existing "system.cifs_acl" and "system.smb3_acl" attributes
> > that allow for setting DACL only. Add support for setting creation time and
> > dos attributes using set_file_info() calls to complement the existing
> > support for getting these attributes via query_path_info() calls.
> >
> > Signed-off-by: Boris Protopopov <bprotopopov@hotmail.com>
> > ---
> >  fs/cifs/xattr.c | 128 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
> >  1 file changed, 117 insertions(+), 11 deletions(-)
> >
> > diff --git a/fs/cifs/xattr.c b/fs/cifs/xattr.c
> > index 9076150758d8..c41856e6fa22 100644
> > --- a/fs/cifs/xattr.c
> > +++ b/fs/cifs/xattr.c
> > @@ -32,7 +32,8 @@
> >  #include "cifs_unicode.h"
> >
> >  #define MAX_EA_VALUE_SIZE CIFSMaxBufSize
> > -#define CIFS_XATTR_CIFS_ACL "system.cifs_acl"
> > +#define CIFS_XATTR_CIFS_ACL "system.cifs_acl" /* DACL only */
> > +#define CIFS_XATTR_CIFS_NTSD "system.cifs_ntsd" /* owner plus DACL */
> >  #define CIFS_XATTR_ATTRIB "cifs.dosattrib"  /* full name: user.cifs.dosattrib */
> >  #define CIFS_XATTR_CREATETIME "cifs.creationtime"  /* user.cifs.creationtime */
> >  /*
> > @@ -40,12 +41,62 @@
> >   * confusing users and using the 20+ year old term 'cifs' when it is no longer
> >   * secure, replaced by SMB2 (then even more highly secure SMB3) many years ago
> >   */
> > -#define SMB3_XATTR_CIFS_ACL "system.smb3_acl"
> > +#define SMB3_XATTR_CIFS_ACL "system.smb3_acl" /* DACL only */
> > +#define SMB3_XATTR_CIFS_NTSD "system.smb3_ntsd" /* owner plus DACL */
> >  #define SMB3_XATTR_ATTRIB "smb3.dosattrib"  /* full name: user.smb3.dosattrib */
> >  #define SMB3_XATTR_CREATETIME "smb3.creationtime"  /* user.smb3.creationtime */
> >  /* BB need to add server (Samba e.g) support for security and trusted prefix */
> >
> > -enum { XATTR_USER, XATTR_CIFS_ACL, XATTR_ACL_ACCESS, XATTR_ACL_DEFAULT };
> > +enum { XATTR_USER, XATTR_CIFS_ACL, XATTR_ACL_ACCESS, XATTR_ACL_DEFAULT,
> > +       XATTR_CIFS_NTSD };
> > +
> > +static int cifs_attrib_set(unsigned int xid, struct cifs_tcon *pTcon,
> > +                          struct inode *inode, char *full_path,
> > +                          const void *value, size_t size)
> > +{
> > +       ssize_t rc = -EOPNOTSUPP;
> > +       __u32 *pattrib = (__u32 *)value;
> > +       __u32 attrib;
> > +       FILE_BASIC_INFO info_buf;
> > +
> > +       if ((value == NULL) || (size != sizeof(__u32)))
> > +               return -ERANGE;
> > +
> > +       memset(&info_buf, 0, sizeof(info_buf));
> > +       info_buf.Attributes = attrib = cpu_to_le32(*pattrib);
> > +
> > +       if (pTcon->ses->server->ops->set_file_info)
> > +               rc = pTcon->ses->server->ops->set_file_info(inode, full_path,
> > +                               &info_buf, xid);
> > +       if (rc == 0)
> > +               CIFS_I(inode)->cifsAttrs = attrib;
> > +
> > +       return rc;
> > +}
> > +
> > +static int cifs_creation_time_set(unsigned int xid, struct cifs_tcon *pTcon,
> > +                                 struct inode *inode, char *full_path,
> > +                                 const void *value, size_t size)
> > +{
> > +       ssize_t rc = -EOPNOTSUPP;
> > +       __u64 *pcreation_time = (__u64 *)value;
> > +       __u64 creation_time;
> > +       FILE_BASIC_INFO info_buf;
> > +
> > +       if ((value == NULL) || (size != sizeof(__u64)))
> > +               return -ERANGE;
> > +
> > +       memset(&info_buf, 0, sizeof(info_buf));
> > +       info_buf.CreationTime = creation_time = cpu_to_le64(*pcreation_time);
> > +
> > +       if (pTcon->ses->server->ops->set_file_info)
> > +               rc = pTcon->ses->server->ops->set_file_info(inode, full_path,
> > +                               &info_buf, xid);
> > +       if (rc == 0)
> > +               CIFS_I(inode)->createtime = creation_time;
> > +
> > +       return rc;
> > +}
> >
> >  static int cifs_xattr_set(const struct xattr_handler *handler,
> >                           struct dentry *dentry, struct inode *inode,
> > @@ -86,6 +137,23 @@ static int cifs_xattr_set(const struct xattr_handler *handler,
> >
> >         switch (handler->flags) {
> >         case XATTR_USER:
> > +               cifs_dbg(FYI, "%s:setting user xattr %s\n", __func__, name);
> > +               if ((strcmp(name, CIFS_XATTR_ATTRIB) == 0) ||
> > +                   (strcmp(name, SMB3_XATTR_ATTRIB) == 0)) {
> > +                       rc = cifs_attrib_set(xid, pTcon, inode, full_path,
> > +                                       value, size);
> > +                       if (rc == 0) /* force revalidate of the inode */
> > +                               CIFS_I(inode)->time = 0;
> > +                       break;
> > +               } else if ((strcmp(name, CIFS_XATTR_CREATETIME) == 0) ||
> > +                          (strcmp(name, SMB3_XATTR_CREATETIME) == 0)) {
> > +                       rc = cifs_creation_time_set(xid, pTcon, inode,
> > +                                       full_path, value, size);
> > +                       if (rc == 0) /* force revalidate of the inode */
> > +                               CIFS_I(inode)->time = 0;
> > +                       break;
> > +               }
> > +
> >                 if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_XATTR)
> >                         goto out;
> >
> > @@ -95,7 +163,8 @@ static int cifs_xattr_set(const struct xattr_handler *handler,
> >                                 cifs_sb->local_nls, cifs_sb);
> >                 break;
> >
> > -       case XATTR_CIFS_ACL: {
> > +       case XATTR_CIFS_ACL:
> > +       case XATTR_CIFS_NTSD: {
> >                 struct cifs_ntsd *pacl;
> >
> >                 if (!value)
> > @@ -106,12 +175,25 @@ static int cifs_xattr_set(const struct xattr_handler *handler,
> >                 } else {
> >                         memcpy(pacl, value, size);
> >                         if (value &&
> > -                           pTcon->ses->server->ops->set_acl)
> > -                               rc = pTcon->ses->server->ops->set_acl(pacl,
> > -                                               size, inode,
> > -                                               full_path, CIFS_ACL_DACL);
> > -                       else
> > +                           pTcon->ses->server->ops->set_acl) {
> > +                               rc = 0;
> > +                               if (handler->flags == XATTR_CIFS_NTSD) {
> > +                                       /* set owner and DACL */
> > +                                       rc = pTcon->ses->server->ops->set_acl(
> > +                                                       pacl, size, inode,
> > +                                                       full_path,
> > +                                                       CIFS_ACL_OWNER);
> > +                               }
> > +                               if (rc == 0) {
> > +                                       /* set DACL */
> > +                                       rc = pTcon->ses->server->ops->set_acl(
> > +                                                       pacl, size, inode,
> > +                                                       full_path,
> > +                                                       CIFS_ACL_DACL);
> > +                               }
> > +                       } else {
> >                                 rc = -EOPNOTSUPP;
> > +                       }
> >                         if (rc == 0) /* force revalidate of the inode */
> >                                 CIFS_I(inode)->time = 0;
> >                         kfree(pacl);
> > @@ -179,7 +261,7 @@ static int cifs_creation_time_get(struct dentry *dentry, struct inode *inode,
> >                                   void *value, size_t size)
> >  {
> >         ssize_t rc;
> > -       __u64 * pcreatetime;
> > +       __u64 *pcreatetime;
> >
> >         rc = cifs_revalidate_dentry_attr(dentry);
> >         if (rc)
> > @@ -244,7 +326,9 @@ static int cifs_xattr_get(const struct xattr_handler *handler,
> >                                 full_path, name, value, size, cifs_sb);
> >                 break;
> >
> > -       case XATTR_CIFS_ACL: {
> > +       case XATTR_CIFS_ACL:
> > +       case XATTR_CIFS_NTSD: {
> > +               /* the whole ntsd is fetched regardless */
> >                 u32 acllen;
> >                 struct cifs_ntsd *pacl;
> >
> > @@ -382,6 +466,26 @@ static const struct xattr_handler smb3_acl_xattr_handler = {
> >         .set = cifs_xattr_set,
> >  };
> >
> > +static const struct xattr_handler cifs_cifs_ntsd_xattr_handler = {
> > +       .name = CIFS_XATTR_CIFS_NTSD,
> > +       .flags = XATTR_CIFS_NTSD,
> > +       .get = cifs_xattr_get,
> > +       .set = cifs_xattr_set,
> > +};
> > +
> > +/*
> > + * Although this is just an alias for the above, need to move away from
> > + * confusing users and using the 20 year old term 'cifs' when it is no
> > + * longer secure and was replaced by SMB2/SMB3 a long time ago, and
> > + * SMB3 and later are highly secure.
> > + */
> > +static const struct xattr_handler smb3_ntsd_xattr_handler = {
> > +       .name = SMB3_XATTR_CIFS_NTSD,
> > +       .flags = XATTR_CIFS_NTSD,
> > +       .get = cifs_xattr_get,
> > +       .set = cifs_xattr_set,
> > +};
> > +
> >  static const struct xattr_handler cifs_posix_acl_access_xattr_handler = {
> >         .name = XATTR_NAME_POSIX_ACL_ACCESS,
> >         .flags = XATTR_ACL_ACCESS,
> > @@ -401,6 +505,8 @@ const struct xattr_handler *cifs_xattr_handlers[] = {
> >         &cifs_os2_xattr_handler,
> >         &cifs_cifs_acl_xattr_handler,
> >         &smb3_acl_xattr_handler, /* alias for above since avoiding "cifs" */
> > +       &cifs_cifs_ntsd_xattr_handler,
> > +       &smb3_ntsd_xattr_handler, /* alias for above since avoiding "cifs" */
> >         &cifs_posix_acl_access_xattr_handler,
> >         &cifs_posix_acl_default_xattr_handler,
> >         NULL
> > --
> > 2.14.5
> >
>
>
> --
> Thanks,
>
> Steve

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Add support for setting owner info, dos attributes, and create time
  2019-12-19 16:52 [PATCH] Add support for setting owner info, dos attributes, and create time Boris Protopopov
  2020-01-09 19:10 ` Steve French
  2020-01-15 21:18 ` Steve French
@ 2020-01-17  2:25 ` Steve French
  2 siblings, 0 replies; 9+ messages in thread
From: Steve French @ 2020-01-17  2:25 UTC (permalink / raw)
  To: Boris Protopopov
  Cc: CIFS, sblbir, Boris Protopopov, Steve French, samba-technical, LKML

[-- Attachment #1: Type: text/plain, Size: 9818 bytes --]

updated patch to fix minor endian errors and remerged into cifs-2.6.git for-next


On Thu, Dec 19, 2019 at 10:53 AM Boris Protopopov
<boris.v.protopopov@gmail.com> wrote:
>
> Add extended attribute "system.cifs_ntsd" (and alias "system.smb3_ntsd")
> to allow for setting owner and DACL in the security descriptor. This is in
> addition to the existing "system.cifs_acl" and "system.smb3_acl" attributes
> that allow for setting DACL only. Add support for setting creation time and
> dos attributes using set_file_info() calls to complement the existing
> support for getting these attributes via query_path_info() calls.
>
> Signed-off-by: Boris Protopopov <bprotopopov@hotmail.com>
> ---
>  fs/cifs/xattr.c | 128 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 117 insertions(+), 11 deletions(-)
>
> diff --git a/fs/cifs/xattr.c b/fs/cifs/xattr.c
> index 9076150758d8..c41856e6fa22 100644
> --- a/fs/cifs/xattr.c
> +++ b/fs/cifs/xattr.c
> @@ -32,7 +32,8 @@
>  #include "cifs_unicode.h"
>
>  #define MAX_EA_VALUE_SIZE CIFSMaxBufSize
> -#define CIFS_XATTR_CIFS_ACL "system.cifs_acl"
> +#define CIFS_XATTR_CIFS_ACL "system.cifs_acl" /* DACL only */
> +#define CIFS_XATTR_CIFS_NTSD "system.cifs_ntsd" /* owner plus DACL */
>  #define CIFS_XATTR_ATTRIB "cifs.dosattrib"  /* full name: user.cifs.dosattrib */
>  #define CIFS_XATTR_CREATETIME "cifs.creationtime"  /* user.cifs.creationtime */
>  /*
> @@ -40,12 +41,62 @@
>   * confusing users and using the 20+ year old term 'cifs' when it is no longer
>   * secure, replaced by SMB2 (then even more highly secure SMB3) many years ago
>   */
> -#define SMB3_XATTR_CIFS_ACL "system.smb3_acl"
> +#define SMB3_XATTR_CIFS_ACL "system.smb3_acl" /* DACL only */
> +#define SMB3_XATTR_CIFS_NTSD "system.smb3_ntsd" /* owner plus DACL */
>  #define SMB3_XATTR_ATTRIB "smb3.dosattrib"  /* full name: user.smb3.dosattrib */
>  #define SMB3_XATTR_CREATETIME "smb3.creationtime"  /* user.smb3.creationtime */
>  /* BB need to add server (Samba e.g) support for security and trusted prefix */
>
> -enum { XATTR_USER, XATTR_CIFS_ACL, XATTR_ACL_ACCESS, XATTR_ACL_DEFAULT };
> +enum { XATTR_USER, XATTR_CIFS_ACL, XATTR_ACL_ACCESS, XATTR_ACL_DEFAULT,
> +       XATTR_CIFS_NTSD };
> +
> +static int cifs_attrib_set(unsigned int xid, struct cifs_tcon *pTcon,
> +                          struct inode *inode, char *full_path,
> +                          const void *value, size_t size)
> +{
> +       ssize_t rc = -EOPNOTSUPP;
> +       __u32 *pattrib = (__u32 *)value;
> +       __u32 attrib;
> +       FILE_BASIC_INFO info_buf;
> +
> +       if ((value == NULL) || (size != sizeof(__u32)))
> +               return -ERANGE;
> +
> +       memset(&info_buf, 0, sizeof(info_buf));
> +       info_buf.Attributes = attrib = cpu_to_le32(*pattrib);
> +
> +       if (pTcon->ses->server->ops->set_file_info)
> +               rc = pTcon->ses->server->ops->set_file_info(inode, full_path,
> +                               &info_buf, xid);
> +       if (rc == 0)
> +               CIFS_I(inode)->cifsAttrs = attrib;
> +
> +       return rc;
> +}
> +
> +static int cifs_creation_time_set(unsigned int xid, struct cifs_tcon *pTcon,
> +                                 struct inode *inode, char *full_path,
> +                                 const void *value, size_t size)
> +{
> +       ssize_t rc = -EOPNOTSUPP;
> +       __u64 *pcreation_time = (__u64 *)value;
> +       __u64 creation_time;
> +       FILE_BASIC_INFO info_buf;
> +
> +       if ((value == NULL) || (size != sizeof(__u64)))
> +               return -ERANGE;
> +
> +       memset(&info_buf, 0, sizeof(info_buf));
> +       info_buf.CreationTime = creation_time = cpu_to_le64(*pcreation_time);
> +
> +       if (pTcon->ses->server->ops->set_file_info)
> +               rc = pTcon->ses->server->ops->set_file_info(inode, full_path,
> +                               &info_buf, xid);
> +       if (rc == 0)
> +               CIFS_I(inode)->createtime = creation_time;
> +
> +       return rc;
> +}
>
>  static int cifs_xattr_set(const struct xattr_handler *handler,
>                           struct dentry *dentry, struct inode *inode,
> @@ -86,6 +137,23 @@ static int cifs_xattr_set(const struct xattr_handler *handler,
>
>         switch (handler->flags) {
>         case XATTR_USER:
> +               cifs_dbg(FYI, "%s:setting user xattr %s\n", __func__, name);
> +               if ((strcmp(name, CIFS_XATTR_ATTRIB) == 0) ||
> +                   (strcmp(name, SMB3_XATTR_ATTRIB) == 0)) {
> +                       rc = cifs_attrib_set(xid, pTcon, inode, full_path,
> +                                       value, size);
> +                       if (rc == 0) /* force revalidate of the inode */
> +                               CIFS_I(inode)->time = 0;
> +                       break;
> +               } else if ((strcmp(name, CIFS_XATTR_CREATETIME) == 0) ||
> +                          (strcmp(name, SMB3_XATTR_CREATETIME) == 0)) {
> +                       rc = cifs_creation_time_set(xid, pTcon, inode,
> +                                       full_path, value, size);
> +                       if (rc == 0) /* force revalidate of the inode */
> +                               CIFS_I(inode)->time = 0;
> +                       break;
> +               }
> +
>                 if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_XATTR)
>                         goto out;
>
> @@ -95,7 +163,8 @@ static int cifs_xattr_set(const struct xattr_handler *handler,
>                                 cifs_sb->local_nls, cifs_sb);
>                 break;
>
> -       case XATTR_CIFS_ACL: {
> +       case XATTR_CIFS_ACL:
> +       case XATTR_CIFS_NTSD: {
>                 struct cifs_ntsd *pacl;
>
>                 if (!value)
> @@ -106,12 +175,25 @@ static int cifs_xattr_set(const struct xattr_handler *handler,
>                 } else {
>                         memcpy(pacl, value, size);
>                         if (value &&
> -                           pTcon->ses->server->ops->set_acl)
> -                               rc = pTcon->ses->server->ops->set_acl(pacl,
> -                                               size, inode,
> -                                               full_path, CIFS_ACL_DACL);
> -                       else
> +                           pTcon->ses->server->ops->set_acl) {
> +                               rc = 0;
> +                               if (handler->flags == XATTR_CIFS_NTSD) {
> +                                       /* set owner and DACL */
> +                                       rc = pTcon->ses->server->ops->set_acl(
> +                                                       pacl, size, inode,
> +                                                       full_path,
> +                                                       CIFS_ACL_OWNER);
> +                               }
> +                               if (rc == 0) {
> +                                       /* set DACL */
> +                                       rc = pTcon->ses->server->ops->set_acl(
> +                                                       pacl, size, inode,
> +                                                       full_path,
> +                                                       CIFS_ACL_DACL);
> +                               }
> +                       } else {
>                                 rc = -EOPNOTSUPP;
> +                       }
>                         if (rc == 0) /* force revalidate of the inode */
>                                 CIFS_I(inode)->time = 0;
>                         kfree(pacl);
> @@ -179,7 +261,7 @@ static int cifs_creation_time_get(struct dentry *dentry, struct inode *inode,
>                                   void *value, size_t size)
>  {
>         ssize_t rc;
> -       __u64 * pcreatetime;
> +       __u64 *pcreatetime;
>
>         rc = cifs_revalidate_dentry_attr(dentry);
>         if (rc)
> @@ -244,7 +326,9 @@ static int cifs_xattr_get(const struct xattr_handler *handler,
>                                 full_path, name, value, size, cifs_sb);
>                 break;
>
> -       case XATTR_CIFS_ACL: {
> +       case XATTR_CIFS_ACL:
> +       case XATTR_CIFS_NTSD: {
> +               /* the whole ntsd is fetched regardless */
>                 u32 acllen;
>                 struct cifs_ntsd *pacl;
>
> @@ -382,6 +466,26 @@ static const struct xattr_handler smb3_acl_xattr_handler = {
>         .set = cifs_xattr_set,
>  };
>
> +static const struct xattr_handler cifs_cifs_ntsd_xattr_handler = {
> +       .name = CIFS_XATTR_CIFS_NTSD,
> +       .flags = XATTR_CIFS_NTSD,
> +       .get = cifs_xattr_get,
> +       .set = cifs_xattr_set,
> +};
> +
> +/*
> + * Although this is just an alias for the above, need to move away from
> + * confusing users and using the 20 year old term 'cifs' when it is no
> + * longer secure and was replaced by SMB2/SMB3 a long time ago, and
> + * SMB3 and later are highly secure.
> + */
> +static const struct xattr_handler smb3_ntsd_xattr_handler = {
> +       .name = SMB3_XATTR_CIFS_NTSD,
> +       .flags = XATTR_CIFS_NTSD,
> +       .get = cifs_xattr_get,
> +       .set = cifs_xattr_set,
> +};
> +
>  static const struct xattr_handler cifs_posix_acl_access_xattr_handler = {
>         .name = XATTR_NAME_POSIX_ACL_ACCESS,
>         .flags = XATTR_ACL_ACCESS,
> @@ -401,6 +505,8 @@ const struct xattr_handler *cifs_xattr_handlers[] = {
>         &cifs_os2_xattr_handler,
>         &cifs_cifs_acl_xattr_handler,
>         &smb3_acl_xattr_handler, /* alias for above since avoiding "cifs" */
> +       &cifs_cifs_ntsd_xattr_handler,
> +       &smb3_ntsd_xattr_handler, /* alias for above since avoiding "cifs" */
>         &cifs_posix_acl_access_xattr_handler,
>         &cifs_posix_acl_default_xattr_handler,
>         NULL
> --
> 2.14.5
>


-- 
Thanks,

Steve

[-- Attachment #2: 0001-CIFS-Add-support-for-setting-owner-info-dos-attribut.patch --]
[-- Type: text/x-patch, Size: 7591 bytes --]

From ff4dddcf50d206373b2914cf72f459a70030ab92 Mon Sep 17 00:00:00 2001
From: Boris Protopopov <bprotopopov@hotmail.com>
Date: Thu, 19 Dec 2019 16:52:50 +0000
Subject: [PATCH 1/3] CIFS: Add support for setting owner info, dos attributes,
 and create time

This is needed for backup/restore scenarios among others.

Add extended attribute "system.cifs_ntsd" (and alias "system.smb3_ntsd")
to allow for setting owner and DACL in the security descriptor. This is in
addition to the existing "system.cifs_acl" and "system.smb3_acl" attributes
that allow for setting DACL only. Add support for setting creation time and
dos attributes using set_file_info() calls to complement the existing
support for getting these attributes via query_path_info() calls.

Signed-off-by: Boris Protopopov <bprotopopov@hotmail.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/cifs/xattr.c | 128 +++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 117 insertions(+), 11 deletions(-)

diff --git a/fs/cifs/xattr.c b/fs/cifs/xattr.c
index db4ba8f6077e..4142c9599f68 100644
--- a/fs/cifs/xattr.c
+++ b/fs/cifs/xattr.c
@@ -32,7 +32,8 @@
 #include "cifs_unicode.h"
 
 #define MAX_EA_VALUE_SIZE CIFSMaxBufSize
-#define CIFS_XATTR_CIFS_ACL "system.cifs_acl"
+#define CIFS_XATTR_CIFS_ACL "system.cifs_acl" /* DACL only */
+#define CIFS_XATTR_CIFS_NTSD "system.cifs_ntsd" /* owner plus DACL */
 #define CIFS_XATTR_ATTRIB "cifs.dosattrib"  /* full name: user.cifs.dosattrib */
 #define CIFS_XATTR_CREATETIME "cifs.creationtime"  /* user.cifs.creationtime */
 /*
@@ -40,12 +41,62 @@
  * confusing users and using the 20+ year old term 'cifs' when it is no longer
  * secure, replaced by SMB2 (then even more highly secure SMB3) many years ago
  */
-#define SMB3_XATTR_CIFS_ACL "system.smb3_acl"
+#define SMB3_XATTR_CIFS_ACL "system.smb3_acl" /* DACL only */
+#define SMB3_XATTR_CIFS_NTSD "system.smb3_ntsd" /* owner plus DACL */
 #define SMB3_XATTR_ATTRIB "smb3.dosattrib"  /* full name: user.smb3.dosattrib */
 #define SMB3_XATTR_CREATETIME "smb3.creationtime"  /* user.smb3.creationtime */
 /* BB need to add server (Samba e.g) support for security and trusted prefix */
 
-enum { XATTR_USER, XATTR_CIFS_ACL, XATTR_ACL_ACCESS, XATTR_ACL_DEFAULT };
+enum { XATTR_USER, XATTR_CIFS_ACL, XATTR_ACL_ACCESS, XATTR_ACL_DEFAULT,
+	XATTR_CIFS_NTSD };
+
+static int cifs_attrib_set(unsigned int xid, struct cifs_tcon *pTcon,
+			   struct inode *inode, char *full_path,
+			   const void *value, size_t size)
+{
+	ssize_t rc = -EOPNOTSUPP;
+	__u32 *pattrib = (__u32 *)value;
+	__u32 attrib;
+	FILE_BASIC_INFO info_buf;
+
+	if ((value == NULL) || (size != sizeof(__u32)))
+		return -ERANGE;
+
+	memset(&info_buf, 0, sizeof(info_buf));
+	attrib = *pattrib;
+	info_buf.Attributes = cpu_to_le32(attrib);
+	if (pTcon->ses->server->ops->set_file_info)
+		rc = pTcon->ses->server->ops->set_file_info(inode, full_path,
+				&info_buf, xid);
+	if (rc == 0)
+		CIFS_I(inode)->cifsAttrs = attrib;
+
+	return rc;
+}
+
+static int cifs_creation_time_set(unsigned int xid, struct cifs_tcon *pTcon,
+				  struct inode *inode, char *full_path,
+				  const void *value, size_t size)
+{
+	ssize_t rc = -EOPNOTSUPP;
+	__u64 *pcreation_time = (__u64 *)value;
+	__u64 creation_time;
+	FILE_BASIC_INFO info_buf;
+
+	if ((value == NULL) || (size != sizeof(__u64)))
+		return -ERANGE;
+
+	memset(&info_buf, 0, sizeof(info_buf));
+	creation_time = *pcreation_time;
+	info_buf.CreationTime = cpu_to_le64(creation_time);
+	if (pTcon->ses->server->ops->set_file_info)
+		rc = pTcon->ses->server->ops->set_file_info(inode, full_path,
+				&info_buf, xid);
+	if (rc == 0)
+		CIFS_I(inode)->createtime = creation_time;
+
+	return rc;
+}
 
 static int cifs_xattr_set(const struct xattr_handler *handler,
 			  struct dentry *dentry, struct inode *inode,
@@ -86,6 +137,23 @@ static int cifs_xattr_set(const struct xattr_handler *handler,
 
 	switch (handler->flags) {
 	case XATTR_USER:
+		cifs_dbg(FYI, "%s:setting user xattr %s\n", __func__, name);
+		if ((strcmp(name, CIFS_XATTR_ATTRIB) == 0) ||
+		    (strcmp(name, SMB3_XATTR_ATTRIB) == 0)) {
+			rc = cifs_attrib_set(xid, pTcon, inode, full_path,
+					value, size);
+			if (rc == 0) /* force revalidate of the inode */
+				CIFS_I(inode)->time = 0;
+			break;
+		} else if ((strcmp(name, CIFS_XATTR_CREATETIME) == 0) ||
+			   (strcmp(name, SMB3_XATTR_CREATETIME) == 0)) {
+			rc = cifs_creation_time_set(xid, pTcon, inode,
+					full_path, value, size);
+			if (rc == 0) /* force revalidate of the inode */
+				CIFS_I(inode)->time = 0;
+			break;
+		}
+
 		if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_XATTR)
 			goto out;
 
@@ -95,7 +163,8 @@ static int cifs_xattr_set(const struct xattr_handler *handler,
 				cifs_sb->local_nls, cifs_sb);
 		break;
 
-	case XATTR_CIFS_ACL: {
+	case XATTR_CIFS_ACL:
+	case XATTR_CIFS_NTSD: {
 		struct cifs_ntsd *pacl;
 
 		if (!value)
@@ -106,12 +175,25 @@ static int cifs_xattr_set(const struct xattr_handler *handler,
 		} else {
 			memcpy(pacl, value, size);
 			if (value &&
-			    pTcon->ses->server->ops->set_acl)
-				rc = pTcon->ses->server->ops->set_acl(pacl,
-						size, inode,
-						full_path, CIFS_ACL_DACL);
-			else
+			    pTcon->ses->server->ops->set_acl) {
+				rc = 0;
+				if (handler->flags == XATTR_CIFS_NTSD) {
+					/* set owner and DACL */
+					rc = pTcon->ses->server->ops->set_acl(
+							pacl, size, inode,
+							full_path,
+							CIFS_ACL_OWNER);
+				}
+				if (rc == 0) {
+					/* set DACL */
+					rc = pTcon->ses->server->ops->set_acl(
+							pacl, size, inode,
+							full_path,
+							CIFS_ACL_DACL);
+				}
+			} else {
 				rc = -EOPNOTSUPP;
+			}
 			if (rc == 0) /* force revalidate of the inode */
 				CIFS_I(inode)->time = 0;
 			kfree(pacl);
@@ -179,7 +261,7 @@ static int cifs_creation_time_get(struct dentry *dentry, struct inode *inode,
 				  void *value, size_t size)
 {
 	ssize_t rc;
-	__u64 * pcreatetime;
+	__u64 *pcreatetime;
 
 	rc = cifs_revalidate_dentry_attr(dentry);
 	if (rc)
@@ -244,7 +326,9 @@ static int cifs_xattr_get(const struct xattr_handler *handler,
 				full_path, name, value, size, cifs_sb);
 		break;
 
-	case XATTR_CIFS_ACL: {
+	case XATTR_CIFS_ACL:
+	case XATTR_CIFS_NTSD: {
+		/* the whole ntsd is fetched regardless */
 		u32 acllen;
 		struct cifs_ntsd *pacl;
 
@@ -382,6 +466,26 @@ static const struct xattr_handler smb3_acl_xattr_handler = {
 	.set = cifs_xattr_set,
 };
 
+static const struct xattr_handler cifs_cifs_ntsd_xattr_handler = {
+	.name = CIFS_XATTR_CIFS_NTSD,
+	.flags = XATTR_CIFS_NTSD,
+	.get = cifs_xattr_get,
+	.set = cifs_xattr_set,
+};
+
+/*
+ * Although this is just an alias for the above, need to move away from
+ * confusing users and using the 20 year old term 'cifs' when it is no
+ * longer secure and was replaced by SMB2/SMB3 a long time ago, and
+ * SMB3 and later are highly secure.
+ */
+static const struct xattr_handler smb3_ntsd_xattr_handler = {
+	.name = SMB3_XATTR_CIFS_NTSD,
+	.flags = XATTR_CIFS_NTSD,
+	.get = cifs_xattr_get,
+	.set = cifs_xattr_set,
+};
+
 static const struct xattr_handler cifs_posix_acl_access_xattr_handler = {
 	.name = XATTR_NAME_POSIX_ACL_ACCESS,
 	.flags = XATTR_ACL_ACCESS,
@@ -401,6 +505,8 @@ const struct xattr_handler *cifs_xattr_handlers[] = {
 	&cifs_os2_xattr_handler,
 	&cifs_cifs_acl_xattr_handler,
 	&smb3_acl_xattr_handler, /* alias for above since avoiding "cifs" */
+	&cifs_cifs_ntsd_xattr_handler,
+	&smb3_ntsd_xattr_handler, /* alias for above since avoiding "cifs" */
 	&cifs_posix_acl_access_xattr_handler,
 	&cifs_posix_acl_default_xattr_handler,
 	NULL
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-01-17  2:25 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-19 16:52 [PATCH] Add support for setting owner info, dos attributes, and create time Boris Protopopov
2020-01-09 19:10 ` Steve French
2020-01-13 20:17   ` Boris Protopopov
2020-01-13 20:26   ` Andreas Dilger
2020-01-13 20:36     ` Jeremy Allison
2020-01-13 22:17       ` Boris Protopopov
2020-01-15 21:18 ` Steve French
2020-01-16 21:20   ` Boris Protopopov
2020-01-17  2:25 ` Steve French

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).