vfs_streams_xattr: stream name prefix and type suffix

Ralph Böhme rb at sernet.de
Tue May 27 06:46:23 MDT 2014


Hi

On Tue, May 27, 2014 at 11:06:00AM +0200, Volker Lendecke wrote:
> On Mon, May 26, 2014 at 06:14:23PM +0200, Ralph Böhme wrote:
> > 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.
> 
> Some minor nit-picks, sorry for the second round...

appreciate the review!

> > +	if (stype) {
> 
> Can we make this (stype != NULL) ?

Done.

> 
> >  		/* 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) {
> 
> This ==true discussion was meant as a joke :-)
> 
> I'd much rather see "if (config->store_stream_type) {"

Done.

> > +		/* Append an explicit stream type if one wasn't specified. */
> > +		*xattr_name = talloc_asprintf(ctx, "%s%s", *xattr_name, ":$DATA");
> 
> There's a few lines beyond 80 chars. Can you fix those?

Done.

> > +	prefix = lp_parm_string_list(SNUM(handle->conn),
> > +				     "streams_xattr", "prefix",
> > +				     &default_prefix);
> 
> Why lp_parm_string_list here? Wouldn't lp_parm_const_string
> do here?

Patch changed to use lp_parm_const_string().

Btw, what's the rationlae for returning the stream size one byte
smaller then the underling xattr? We don't do that in
vfs_streams_depot.

Doesn't make sense to me and effectively truncates the xattr by one
byte.

Thanks!
-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 57c675936ff49200193fda2884cbd38898598d6d 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 | 122 +++++++++++++++++++++++++++---------
 1 file changed, 92 insertions(+), 30 deletions(-)

diff --git a/source3/modules/vfs_streams_xattr.c b/source3/modules/vfs_streams_xattr.c
index 5e9bd3e..d0e4c1b 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,41 @@ 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 != NULL) {
 		/* 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) {
+		/*
+		 * 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 +159,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,8 +285,8 @@ 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,
-					&xattr_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);
 		return -1;
@@ -322,8 +336,8 @@ 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,
-					&xattr_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);
 		return -1;
@@ -386,8 +400,8 @@ static int streams_xattr_open(vfs_handle_struct *handle,
 		return ret;
 	}
 
-	status = streams_xattr_get_name(talloc_tos(), smb_fname->stream_name,
-					&xattr_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);
 		goto fail;
@@ -541,8 +555,8 @@ static int streams_xattr_unlink(vfs_handle_struct *handle,
 		return ret;
 	}
 
-	status = streams_xattr_get_name(talloc_tos(), smb_fname->stream_name,
-					&xattr_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);
 		goto fail;
@@ -597,14 +611,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 +664,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 +673,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;
+
+	SMB_VFS_HANDLE_GET_DATA(handle, config, struct streams_xattr_config,
+				return NT_STATUS_UNSUCCESSFUL);
 
-	status = get_ea_names_from_file(talloc_tos(), conn, fsp, fname,
+	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 +687,26 @@ 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 +826,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 +852,44 @@ 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_const_string(SNUM(handle->conn),
+				      "streams_xattr", "prefix",
+				      default_prefix);
+	config->prefix = talloc_strdup(config, prefix);
+	if (config->prefix == NULL) {
+		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 +1092,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 1b4cab5a935d0d2771713de065f7099443f97b7b 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