vfs_streams_xattr: stream name prefix and type suffix

Ralph Böhme rb at sernet.de
Mon May 26 10:14:23 MDT 2014


Hi Volker

On Fri, May 23, 2014 at 06:52:05PM +0200, Volker Lendecke wrote:
> On Wed, May 21, 2014 at 01:47:53PM +0200, Ralph Böhme wrote:
> > With the patches applied, adding the following to smb.conf
> > 
> >       streams_xattr:prefix = user.
> >       streams_xattr:store_stream_type = no
> > 
> > gives the desired behaviour.
> > 
> > Thoughts?
> 
> The idea sounds good.

great.

> 
> > >From 066fcf3c5bb414b84a349a1a5fda07e3e0308593 Mon Sep 17 00:00:00 2001
> > From: Ralph Boehme <rb at sernet.de>
> > Date: Wed, 21 May 2014 11:52:27 +0200
> > Subject: [PATCH 1/2] Convert samba_private_attr_name() to a public function
> > 
> > Signed-off-by: Ralph Boehme <rb at sernet.de>
> > ---
> >  source3/modules/vfs_streams_xattr.c | 3 +++
> >  source3/smbd/proto.h                | 1 +
> >  source3/smbd/trans2.c               | 2 +-
> >  3 files changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/source3/modules/vfs_streams_xattr.c b/source3/modules/vfs_streams_xattr.c
> > index 5e9bd3e..5ff4f82 100644
> > --- a/source3/modules/vfs_streams_xattr.c
> > +++ b/source3/modules/vfs_streams_xattr.c
> > @@ -674,6 +674,9 @@ static NTSTATUS walk_xattr_streams(connection_struct *conn, files_struct *fsp,
> >  			    prefix_len) != 0) {
> >  			continue;
> >  		}
> > +		if (samba_private_attr_name(names[i])) {
> > +			continue;
> > +		}
> 
> This hunk is a bit more than is explained in the commit
> message. Can you expand the message a bit with something
> like ... in preparation of ... or split this into a separate
> patch?

Oops, sorry, my bad. That hunk shouldn't have been going into this
commit at all, but into the second one.

I've now folded it into the second commit, as I don't see any
functionality that would set it apart from the larger rest of the
second commit. However, I've added some description for this to the
commit message.

> 
> >  	if (stype == NULL) {
> >  		/* Append an explicit stream type if one wasn't specified. */
> > -		*xattr_name = talloc_asprintf(ctx, "%s:$DATA",
> > -					       *xattr_name);
> > +		*xattr_name = talloc_asprintf(ctx, "%s%s",
> > +					      *xattr_name,
> > +					      config->store_stream_type ? ":$DATA" : "");
> 
> Stylistic comment: Here I would do something like
> 
> 	if ((stype == NULL) && (config->store_stream_type != NULL))
> 
> and just not do the talloc_asprintf in the NULL case.

I've reordered the if clause, avoids the needless talloc_asprintf and
the logic should be easier to grasp.

> 
> > +static int streams_xattr_connect(vfs_handle_struct *handle, const char *service, const char *user)
> > +{
> > +	struct streams_xattr_config *config;
> > +	const char *default_prefix = SAMBA_XATTR_DOSSTREAM_PREFIX;
> > +	const char **prefix;
> > +
> > +	config = talloc_zero(handle->conn, struct streams_xattr_config);
> > +	if (!config) {
> > +		DEBUG(1, ("talloc_zero() failed\n"));
> > +		errno = ENOMEM;
> > +		return -1;
> > +	}
> > +
> > +	prefix = lp_parm_string_list(SNUM(handle->conn),
> > +				     "streams_xattr", "prefix",
> > +				     &default_prefix);
> > +	if (prefix) {
> > +		config->prefix = *prefix;
> 
> Does this maintain the correct talloc hierarchy? Isn't
> *prefix gone after the next tevent round with
> talloc_free(frame)?

thanks for spotting this one. Added an explicit talloc_strdup()
hanging the string to off right talloc context.

Finally added a third commit with a patch for the manpage.

Fixed patches attached.

-Ralph

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de,mailto:kontakt@sernet.de
-------------- next part --------------
>From 63a8a88ec37f71230fc6036fefe0842d7afd5ac8 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <rb at sernet.de>
Date: Wed, 21 May 2014 11:52:27 +0200
Subject: [PATCH 1/3] Convert samba_private_attr_name() to a public function

Signed-off-by: Ralph Boehme <rb at sernet.de>
---
 source3/smbd/proto.h  | 1 +
 source3/smbd/trans2.c | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index f598816..3c6accf 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -1056,6 +1056,7 @@ NTSTATUS check_access(connection_struct *conn,
 				uint32_t access_mask);
 uint64_t smb_roundup(connection_struct *conn, uint64_t val);
 uint64_t get_FileIndex(connection_struct *conn, const SMB_STRUCT_STAT *psbuf);
+bool samba_private_attr_name(const char *unix_ea_name);
 NTSTATUS get_ea_value(TALLOC_CTX *mem_ctx, connection_struct *conn,
 		      files_struct *fsp, const char *fname,
 		      const char *ea_name, struct ea_struct *pea);
diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c
index 4afb27e..aeada68 100644
--- a/source3/smbd/trans2.c
+++ b/source3/smbd/trans2.c
@@ -120,7 +120,7 @@ uint64_t get_FileIndex(connection_struct *conn, const SMB_STRUCT_STAT *psbuf)
  Refuse to allow clients to overwrite our private xattrs.
 ****************************************************************************/
 
-static bool samba_private_attr_name(const char *unix_ea_name)
+bool samba_private_attr_name(const char *unix_ea_name)
 {
 	static const char * const prohibited_ea_names[] = {
 		SAMBA_POSIX_INHERITANCE_EA_NAME,
-- 
1.8.5.3

-------------- next part --------------
>From 86bf2daeb50ef72ef55a81f44fdc94c3d5ac8c2d Mon Sep 17 00:00:00 2001
From: Ralph Boehme <rb at sernet.de>
Date: Tue, 20 May 2014 15:17:01 +0200
Subject: [PATCH 2/3] vfs_streams_xattr: add options "prefix" and 
 "store_stream_type"

Add module options that can be used to configure the stream prefix the
module uses (option "prefix", a string) and whether the stream type
"$DATA" is appended to the xattr name on disk (option
"store_stream_type", a boolean).

The default "prefix" is "user.DosStream" and the default for
"store_stream_type" is true, this gives unchanged default behaviour
when not specifying this option.

OS X SMB clients will send xattrs as named streams over the wire, by
setting the options to the following values

  streams_xattr:prefix = user.
  streams_xattr:store_stream_type = no

OS X xattrs will be stored on disk on the server with their unmodified
names and as such provide interoperability with other protocols like
AFP.

In order to prevent access to our internal Samba xattrs, check the
xattr name with the function samba_private_attr_name() made public by
the previous commit.

Signed-off-by: Ralph Boehme <rb at sernet.de>
---
 source3/modules/vfs_streams_xattr.c | 111 +++++++++++++++++++++++++++---------
 1 file changed, 85 insertions(+), 26 deletions(-)

diff --git a/source3/modules/vfs_streams_xattr.c b/source3/modules/vfs_streams_xattr.c
index 5e9bd3e..f2b6882 100644
--- a/source3/modules/vfs_streams_xattr.c
+++ b/source3/modules/vfs_streams_xattr.c
@@ -29,6 +29,12 @@
 #undef DBGC_CLASS
 #define DBGC_CLASS DBGC_VFS
 
+struct streams_xattr_config {
+	const char *prefix;
+	size_t prefix_len;
+	bool store_stream_type;
+};
+
 struct stream_io {
 	char *base;
 	char *xattr_name;
@@ -95,33 +101,37 @@ static ssize_t get_xattr_size(connection_struct *conn,
  * Given a stream name, populate xattr_name with the xattr name to use for
  * accessing the stream.
  */
-static NTSTATUS streams_xattr_get_name(TALLOC_CTX *ctx,
+static NTSTATUS streams_xattr_get_name(vfs_handle_struct *handle,
+				       TALLOC_CTX *ctx,
 				       const char *stream_name,
 				       char **xattr_name)
 {
 	char *stype;
+	struct streams_xattr_config *config;
+
+	SMB_VFS_HANDLE_GET_DATA(handle, config, struct streams_xattr_config,
+				return NT_STATUS_UNSUCCESSFUL);
 
 	stype = strchr_m(stream_name + 1, ':');
 
 	*xattr_name = talloc_asprintf(ctx, "%s%s",
-				      SAMBA_XATTR_DOSSTREAM_PREFIX,
+				      config->prefix,
 				      stream_name + 1);
 	if (*xattr_name == NULL) {
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	if (stype == NULL) {
-		/* Append an explicit stream type if one wasn't specified. */
-		*xattr_name = talloc_asprintf(ctx, "%s:$DATA",
-					       *xattr_name);
-		if (*xattr_name == NULL) {
-			return NT_STATUS_NO_MEMORY;
-		}
-	} else {
+	if (stype) {
 		/* Normalize the stream type to upercase. */
 		if (!strupper_m(strrchr_m(*xattr_name, ':') + 1)) {
 			return NT_STATUS_INVALID_PARAMETER;
 		}
+	} else if (config->store_stream_type == true) {
+		/* Append an explicit stream type if one wasn't specified. */
+		*xattr_name = talloc_asprintf(ctx, "%s%s", *xattr_name, ":$DATA");
+		if (*xattr_name == NULL) {
+			return NT_STATUS_NO_MEMORY;
+		}
 	}
 
 	DEBUG(10, ("xattr_name: %s, stream_name: %s\n", *xattr_name,
@@ -145,7 +155,7 @@ static bool streams_xattr_recheck(struct stream_io *sio)
 		return false;
 	}
 
-	status = streams_xattr_get_name(talloc_tos(),
+	status = streams_xattr_get_name(sio->handle, talloc_tos(),
 					sio->fsp->fsp_name->stream_name,
 					&xattr_name);
 	if (!NT_STATUS_IS_OK(status)) {
@@ -271,7 +281,7 @@ static int streams_xattr_stat(vfs_handle_struct *handle,
 	}
 
 	/* Derive the xattr name to lookup. */
-	status = streams_xattr_get_name(talloc_tos(), smb_fname->stream_name,
+	status = streams_xattr_get_name(handle, talloc_tos(), smb_fname->stream_name,
 					&xattr_name);
 	if (!NT_STATUS_IS_OK(status)) {
 		errno = map_errno_from_nt_status(status);
@@ -322,7 +332,7 @@ static int streams_xattr_lstat(vfs_handle_struct *handle,
 	}
 
 	/* Derive the xattr name to lookup. */
-	status = streams_xattr_get_name(talloc_tos(), smb_fname->stream_name,
+	status = streams_xattr_get_name(handle, talloc_tos(), smb_fname->stream_name,
 					&xattr_name);
 	if (!NT_STATUS_IS_OK(status)) {
 		errno = map_errno_from_nt_status(status);
@@ -386,7 +396,7 @@ static int streams_xattr_open(vfs_handle_struct *handle,
 		return ret;
 	}
 
-	status = streams_xattr_get_name(talloc_tos(), smb_fname->stream_name,
+	status = streams_xattr_get_name(handle, talloc_tos(), smb_fname->stream_name,
 					&xattr_name);
 	if (!NT_STATUS_IS_OK(status)) {
 		errno = map_errno_from_nt_status(status);
@@ -541,7 +551,7 @@ static int streams_xattr_unlink(vfs_handle_struct *handle,
 		return ret;
 	}
 
-	status = streams_xattr_get_name(talloc_tos(), smb_fname->stream_name,
+	status = streams_xattr_get_name(handle, talloc_tos(), smb_fname->stream_name,
 					&xattr_name);
 	if (!NT_STATUS_IS_OK(status)) {
 		errno = map_errno_from_nt_status(status);
@@ -597,14 +607,14 @@ static int streams_xattr_rename(vfs_handle_struct *handle,
 	}
 
 	/* Get the xattr names. */
-	status = streams_xattr_get_name(talloc_tos(),
+	status = streams_xattr_get_name(handle, talloc_tos(),
 					smb_fname_src->stream_name,
 					&src_xattr_name);
 	if (!NT_STATUS_IS_OK(status)) {
 		errno = map_errno_from_nt_status(status);
 		goto fail;
 	}
-	status = streams_xattr_get_name(talloc_tos(),
+	status = streams_xattr_get_name(handle, talloc_tos(),
 					smb_fname_dst->stream_name,
 					&dst_xattr_name);
 	if (!NT_STATUS_IS_OK(status)) {
@@ -650,7 +660,7 @@ static int streams_xattr_rename(vfs_handle_struct *handle,
 	return ret;
 }
 
-static NTSTATUS walk_xattr_streams(connection_struct *conn, files_struct *fsp,
+static NTSTATUS walk_xattr_streams(vfs_handle_struct *handle, files_struct *fsp,
 				   const char *fname,
 				   bool (*fn)(struct ea_struct *ea,
 					      void *private_data),
@@ -659,9 +669,12 @@ static NTSTATUS walk_xattr_streams(connection_struct *conn, files_struct *fsp,
 	NTSTATUS status;
 	char **names;
 	size_t i, num_names;
-	size_t prefix_len = strlen(SAMBA_XATTR_DOSSTREAM_PREFIX);
+	struct streams_xattr_config *config;
 
-	status = get_ea_names_from_file(talloc_tos(), conn, fsp, fname,
+	SMB_VFS_HANDLE_GET_DATA(handle, config, struct streams_xattr_config,
+				return NT_STATUS_UNSUCCESSFUL);
+
+	status = get_ea_names_from_file(talloc_tos(), handle->conn, fsp, fname,
 					&names, &num_names);
 	if (!NT_STATUS_IS_OK(status)) {
 		return status;
@@ -670,20 +683,24 @@ static NTSTATUS walk_xattr_streams(connection_struct *conn, files_struct *fsp,
 	for (i=0; i<num_names; i++) {
 		struct ea_struct ea;
 
-		if (strncmp(names[i], SAMBA_XATTR_DOSSTREAM_PREFIX,
-			    prefix_len) != 0) {
+		if (strncmp(names[i], config->prefix,
+			    config->prefix_len) != 0) {
+			continue;
+		}
+		if (samba_private_attr_name(names[i])) {
 			continue;
 		}
 
-		status = get_ea_value(names, conn, fsp, fname, names[i], &ea);
+		status = get_ea_value(names, handle->conn, fsp, fname, names[i], &ea);
 		if (!NT_STATUS_IS_OK(status)) {
 			DEBUG(10, ("Could not get ea %s for file %s: %s\n",
 				   names[i], fname, nt_errstr(status)));
 			continue;
 		}
 
-		ea.name = talloc_asprintf(ea.value.data, ":%s",
-					  names[i] + prefix_len);
+		ea.name = talloc_asprintf(ea.value.data, ":%s%s",
+					  names[i] + config->prefix_len,
+					  config->store_stream_type ? "" : ":$DATA");
 		if (ea.name == NULL) {
 			DEBUG(0, ("talloc failed\n"));
 			continue;
@@ -803,7 +820,7 @@ static NTSTATUS streams_xattr_streaminfo(vfs_handle_struct *handle,
 		 */
 		status = NT_STATUS_OK;
 	} else {
-		status = walk_xattr_streams(handle->conn, fsp, fname,
+		status = walk_xattr_streams(handle, fsp, fname,
 				    collect_one_stream, &state);
 	}
 
@@ -829,6 +846,47 @@ static uint32_t streams_xattr_fs_capabilities(struct vfs_handle_struct *handle,
 	return SMB_VFS_NEXT_FS_CAPABILITIES(handle, p_ts_res) | FILE_NAMED_STREAMS;
 }
 
+static int streams_xattr_connect(vfs_handle_struct *handle, const char *service, const char *user)
+{
+	struct streams_xattr_config *config;
+	const char *default_prefix = SAMBA_XATTR_DOSSTREAM_PREFIX;
+	const char **prefix;
+
+	config = talloc_zero(handle->conn, struct streams_xattr_config);
+	if (!config) {
+		DEBUG(1, ("talloc_zero() failed\n"));
+		errno = ENOMEM;
+		return -1;
+	}
+
+	prefix = lp_parm_string_list(SNUM(handle->conn),
+				     "streams_xattr", "prefix",
+				     &default_prefix);
+	if (prefix) {
+		config->prefix = talloc_strdup(config, *prefix);
+	} else {
+		config->prefix = talloc_strdup(config, "");
+	}
+	if (!config->prefix) {
+		DEBUG(1, ("talloc_strdup() failed\n"));
+		errno = ENOMEM;
+		return -1;
+	}
+	config->prefix_len = strlen(config->prefix);
+	DEBUG(10, ("streams_xattr using stream prefix: %s\n", config->prefix));
+
+	config->store_stream_type = lp_parm_bool(SNUM(handle->conn),
+						 "streams_xattr",
+						 "store_stream_type",
+						 true);
+
+	SMB_VFS_HANDLE_SET_DATA(handle, config,
+				NULL, struct stream_xattr_config,
+				return -1);
+
+	return 0;
+}
+
 static ssize_t streams_xattr_pwrite(vfs_handle_struct *handle,
 				    files_struct *fsp, const void *data,
 				    size_t n, off_t offset)
@@ -1031,6 +1089,7 @@ static int streams_xattr_fallocate(struct vfs_handle_struct *handle,
 
 static struct vfs_fn_pointers vfs_streams_xattr_fns = {
 	.fs_capabilities_fn = streams_xattr_fs_capabilities,
+	.connect_fn = streams_xattr_connect,
 	.open_fn = streams_xattr_open,
 	.stat_fn = streams_xattr_stat,
 	.fstat_fn = streams_xattr_fstat,
-- 
1.8.5.3

-------------- next part --------------
>From 26603c44dff9a1c32ecae7ef7b816c9796975ef6 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <rb at sernet.de>
Date: Mon, 26 May 2014 16:12:17 +0200
Subject: [PATCH 3/3] man vfs_streams_xattr: new options "prefix" and 
 "store_stream_type"

Add documentation for the two new options "streams_xattr:prefix" and
"streams_xattr:store_stream_type".

Signed-off-by: Ralph Boehme <rb at sernet.de>
---
 docs-xml/manpages/vfs_streams_xattr.8.xml | 35 +++++++++++++++++++++++++++++--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/docs-xml/manpages/vfs_streams_xattr.8.xml b/docs-xml/manpages/vfs_streams_xattr.8.xml
index 5aa5aa8..93cfb98 100644
--- a/docs-xml/manpages/vfs_streams_xattr.8.xml
+++ b/docs-xml/manpages/vfs_streams_xattr.8.xml
@@ -33,8 +33,9 @@
 	alternate data streams in the file system. As a normal posix file
 	system does not support the concept of multiple data streams per file,
 	the streams_xattr module stores the data in posix extended attributes
-	(xattrs). The name of these attributes is
-	user.DosStream."ADS-NAME".</para>
+	(xattrs). The name of these attributes by default is
+	user.DosStream."ADS-NAME". The prefix "user.DosStream." can be changed
+    with the module option <command>streams_xattr:prefix</command>.</para>
 
 	<para>The file system that is shared with this module enabled must
 	support xattrs.</para>
@@ -46,6 +47,36 @@
 </refsect1>
 
 <refsect1>
+	<title>OPTIONS</title>
+
+	<variablelist>
+
+		<varlistentry>
+		  <term>streams_xattr:prefix = STRING</term>
+		  <listitem>
+		    <para>
+              Name prefix used when storing an ADS in an xattr, defaults to
+              <command>user.DosStream.</command>.
+		    </para>
+		  </listitem>
+		</varlistentry>
+
+		<varlistentry>
+		  <term>streams_xattr:store_stream_type = [yes|no]</term>
+		  <listitem>
+		    <para>
+              Whether the xattr names for Alternate Data Streams of type
+              "$DATA" are suffixed by the stream type string ":$DATA".
+              The default is <command>yes</command>.
+		    </para>
+		  </listitem>
+		</varlistentry>
+
+	</variablelist>
+
+</refsect1>
+
+<refsect1>
 	<title>EXAMPLES</title>
 
 <programlisting>
-- 
1.8.5.3



More information about the samba-technical mailing list