r9338: fixed the winreg IDL to be correct for the EnumKey and EnumValue

calls. The previous IDL was just a workaround for the limitations of
our older rpc infrastructure. Now that Jelmer has added much improved
string support using the charset keyword we can correctly implemenent
the unusual winreg string buffers.

Jelmer, note the little comment I put on winreg_StringBuf() about why
I couldn't use [value()] for the length field.

This also fixes EnumKey() and EnumValue() to use NTTIME fields for the
last_changed_time. I don't know why we were using a pair of uint32's,
as it is just a NTTIME.
(This used to be commit 8354b016122cc4f3cff042b3ada1de07e1614eb7)

5 files changed, 219 insertions, 112 deletions

diff --git a/source4/lib/registry/reg_backend_rpc.c b/source4/lib/registry/reg_backend_rpc.c
index 73d2d54687..5734b96770 100644
--- a/source4/lib/registry/reg_backend_rpc.c
+++ b/source4/lib/registry/reg_backend_rpc.c
@@ -177,8 +177,8 @@ static WERROR rpc_get_value_by_index(TALLOC_CTX *mem_ctx, struct registry_key *p
 	struct winreg_EnumValue r;
 	uint32_t type, len1, zero = 0;
 	NTSTATUS status;
-	uint8_t buf8;
-	uint16_t buf16;
+	struct winreg_StringBuf name;
+	uint8_t u8;
 	
 	if(mykeydata->num_values == -1) {
 		error = rpc_query_key(parent);
@@ -187,17 +187,18 @@ static WERROR rpc_get_value_by_index(TALLOC_CTX *mem_ctx, struct registry_key *p
 
 	len1 = mykeydata->max_valdatalen;
 	
+	name.length = 0;
+	name.size   = mykeydata->max_valnamelen * 2;
+	name.name   = "";
+
 	r.in.handle = &mykeydata->pol;
 	r.in.enum_index = n;
-	r.in.name_in.length = 0;
-	r.in.name_in.size = mykeydata->max_valnamelen * 2;
-	r.in.name_in.name = &buf16;
+	r.in.name = &name;
 	r.in.type = &type;
-	r.in.value = &buf8;
+	r.in.value = &u8;
 	r.in.length = &zero;
 	r.in.size = &len1;
-	r.out.type = &type;
-
+	r.out.name = &name;
 	
 	status = dcerpc_winreg_EnumValue((struct dcerpc_pipe *)parent->hive->backend_data, mem_ctx, &r);
 	if(NT_STATUS_IS_ERR(status)) {
@@ -208,7 +209,7 @@ static WERROR rpc_get_value_by_index(TALLOC_CTX *mem_ctx, struct registry_key *p
 	if(NT_STATUS_IS_OK(status) && 
 	   W_ERROR_IS_OK(r.out.result) && r.out.length) {
 		*value = talloc(mem_ctx, struct registry_value);
-		(*value)->name = talloc_strdup(mem_ctx, r.out.name_out.name);
+		(*value)->name = talloc_strdup(mem_ctx, r.out.name->name);
 		(*value)->data_type = type;
 		(*value)->data_len = *r.out.length;
 		(*value)->data_blk = talloc_memdup(mem_ctx, r.out.value, *r.out.length);
@@ -221,27 +222,29 @@ static WERROR rpc_get_value_by_index(TALLOC_CTX *mem_ctx, struct registry_key *p
 static WERROR rpc_get_subkey_by_index(TALLOC_CTX *mem_ctx, struct registry_key *parent, int n, struct registry_key **subkey) 
 {
 	struct winreg_EnumKey r;
-	struct winreg_EnumKeyNameRequest keyname;
-	struct winreg_String classname;
-	struct winreg_Time tm;
 	struct rpc_key_data *mykeydata = parent->backend_data;
 	NTSTATUS status;
+	struct winreg_StringBuf namebuf, classbuf;
+	NTTIME change_time = 0;
 
-	r.in.handle = &mykeydata->pol;
-	keyname.unknown = 0x0000020a;
-	init_winreg_String(&keyname.key_name, NULL);
-	init_winreg_String(&classname, NULL);
-	r.in.in_name = &keyname;
-	r.in.class = &classname;
-	tm.low = tm.high = 0x7fffffff;
-	r.in.last_changed_time = &tm;
+	namebuf.length = 0;
+	namebuf.size   = 1024;
+	namebuf.name   = NULL;
+	classbuf.length = 0;
+	classbuf.size   = 0;
+	classbuf.name   = NULL;
 
+	r.in.handle = &mykeydata->pol;
 	r.in.enum_index = n;
-	r.in.unknown = r.out.unknown = 0x0414;
-	r.in.key_name_len = r.out.key_name_len = 0;
+	r.in.name = &namebuf;
+	r.in.class = &classbuf;
+	r.in.last_changed_time = &change_time;
+	r.out.name = &namebuf;
+
 	status = dcerpc_winreg_EnumKey((struct dcerpc_pipe *)parent->hive->backend_data, mem_ctx, &r);
 	if(NT_STATUS_IS_OK(status) && W_ERROR_IS_OK(r.out.result)) {
-			return rpc_open_key(mem_ctx, parent, talloc_strdup(mem_ctx, r.out.out_name->name), subkey);
+		char *name = talloc_strdup(mem_ctx, r.out.name->name);
+		return rpc_open_key(mem_ctx, parent, name, subkey);
 	}
 
 	return r.out.result;
diff --git a/source4/librpc/idl/winreg.idl b/source4/librpc/idl/winreg.idl
index ad77138478..3e29bfcc1c 100644
--- a/source4/librpc/idl/winreg.idl
+++ b/source4/librpc/idl/winreg.idl
@@ -98,53 +98,33 @@
 	} winreg_Time;
 
 	typedef struct {
-		uint32 unknown;
-		winreg_String key_name;
-	} winreg_EnumKeyNameRequest;
-
-	typedef struct {
-		uint32 unknown1;
-		uint32 unknown2;
-		lstring name;
-	} winreg_EnumKeyNameResponse;
+		/* we can't use value(strlen_m(name)*2) here as it
+		   doesn't propogate to the length_is() property
+		   below. Jelmer, can this be fixed? */
+		uint16 length; 
+		uint16 size;
+		[size_is(size/2),length_is(length/2),charset(UTF16)] uint16 *name;
+	} winreg_StringBuf;
 
-	/******************/
-	/* Function: 0x09 */
 	WERROR winreg_EnumKey(
-		[in,ref]    policy_handle *handle,
-		[in] uint32 enum_index,
-		[in,out]    uint16 key_name_len,
-		[in,out]    uint16 unknown,
-		[in]        winreg_EnumKeyNameRequest *in_name,
-		[out]       winreg_EnumKeyNameResponse *out_name,
-		[in,out]    winreg_String *class,
-		[in,out]    winreg_Time *last_changed_time
+		[in,ref]        policy_handle    *handle,
+		[in]            uint32           enum_index,
+		[in,out,ref]    winreg_StringBuf *name,
+		[in,out,unique] winreg_StringBuf *class,
+		[in,out,unique] NTTIME           *last_changed_time
 	);
 
 	/******************/
 	/* Function: 0x0a */
 
-	/* 
-	   this is equivalent IDL to a winreg_String, but we need to
-	   have absolute control over the length/size fields as the
-	   server looks at those to see what size buffer we have, so
-	   we can't use the automatic unistr handing in pidl.
-	*/
-	typedef struct {
-		uint16 length;
-		uint16 size;
-		[size_is(size/2),length_is(length/2)] uint16 *name;
-	} winreg_EnumValueString;
-
 	WERROR winreg_EnumValue(
-		[in,ref] policy_handle *handle,
-		[in]     uint32 enum_index,
-		[in]     winreg_EnumValueString name_in,
-		[out]    winreg_String name_out,
-		[in,out] uint32 *type,
-		[in,out,size_is(*size),length_is(*length)] uint8 *value,
-		[in,out] uint32 *size,
-		[in,out] uint32 *length
+		[in,ref]        policy_handle *handle,
+		[in]            uint32 enum_index,
+		[in,out,ref]    winreg_StringBuf *name,
+		[in,out,unique] uint32 *type,
+		[in,out,unique,size_is(*size),length_is(*length)] uint8 *value,
+		[in,out,unique] uint32 *size,
+		[in,out,unique] uint32 *length
 	);
 
 	/******************/
diff --git a/source4/rpc_server/winreg/rpc_winreg.c b/source4/rpc_server/winreg/rpc_winreg.c
index 050df29727..431323d1b6 100644
--- a/source4/rpc_server/winreg/rpc_winreg.c
+++ b/source4/rpc_server/winreg/rpc_winreg.c
@@ -166,11 +166,13 @@ static WERROR winreg_EnumKey(struct dcesrv_call_state *dce_call, TALLOC_CTX *mem
 	r->out.result = reg_key_get_subkey_by_index(mem_ctx, (struct registry_key *)h->data, r->in.enum_index, &key);
 
 	if (W_ERROR_IS_OK(r->out.result)) {
-		r->out.key_name_len = strlen(key->name);
-		r->out.out_name = talloc_zero(mem_ctx, struct winreg_EnumKeyNameResponse);
-		r->out.out_name->name = key->name;
-		r->out.class = talloc_zero(mem_ctx, struct winreg_String);
-		r->out.last_changed_time = talloc_zero(mem_ctx, struct winreg_Time);
+		if (2*strlen_m(key->name) > r->in.name->size) {
+			return WERR_MORE_DATA;
+		}
+		r->out.name->length = 2*strlen_m(key->name);
+		r->out.name->name = key->name;
+		r->out.class = talloc_zero(mem_ctx, struct winreg_StringBuf);
+		r->out.last_changed_time = &key->last_mod;
 	}
 	
 	return r->out.result;
@@ -196,14 +198,38 @@ static WERROR winreg_EnumValue(struct dcesrv_call_state *dce_call, TALLOC_CTX *m
 	if (!W_ERROR_IS_OK(result)) {
 		return result;
 	}
+
+	/* the client can optionally pass a NULL for type, meaning they don't
+	   want that back */
+	if (r->in.type != NULL) {
+		r->out.type = talloc(mem_ctx, uint32_t);
+		*r->out.type = value->data_type;
+	}
+
+	/* check the client has enough room for the value */
+	if (r->in.size != NULL && 
+	    value->data_len > *r->in.size) {
+		return WERR_MORE_DATA;
+	}
 	
-	r->out.type = talloc(mem_ctx, uint32_t);
-	*r->out.type = value->data_type;
-	r->out.name_out.name = value->name;
-	r->out.value = value->data_blk;
-	r->out.size = talloc(mem_ctx, uint32_t);
-	r->out.length = r->out.size;
-	*r->out.size = value->data_len;
+	/* and enough room for the name */
+	if (r->in.name->size < 2*strlen_m(value->name)) {
+		return WERR_MORE_DATA;		
+	}
+
+	r->out.name->name = value->name;
+	r->out.name->length = 2*strlen_m(value->name);
+	r->out.name->size = 2*strlen_m(value->name);
+
+	if (r->in.value) {
+		r->out.value = value->data_blk;
+	}
+
+	if (r->in.size) {
+		r->out.size = talloc(mem_ctx, uint32_t);
+		*r->out.size = value->data_len;
+		r->out.length = r->out.size;
+	}
 	
 	return WERR_OK;
 }
diff --git a/source4/scripting/libjs/winreg.js b/source4/scripting/libjs/winreg.js
index 703b8da2a7..5323e91ae5 100644
--- a/source4/scripting/libjs/winreg.js
+++ b/source4/scripting/libjs/winreg.js
@@ -4,6 +4,18 @@
 	released under the GNU GPL v2 or later
 */	
 
+libinclude("base.js");
+
+/*
+  close a handle
+*/
+function winreg_close(reg, handle)
+{
+	var io = irpcObj();
+	io.input.handle = handle;
+	reg.winreg_CloseKey(io);
+}
+
 
 /*
   open a hive
@@ -76,6 +88,9 @@ function winreg_open_path(reg, path)
 	io.input.unknown = 0;
 	io.input.access_mask = reg.SEC_FLAG_MAXIMUM_ALLOWED;
 	var status = reg.winreg_OpenKey(io);
+
+	winreg_close(reg, handle);
+
 	if (!status.is_ok) {
 		return undefined;
 	}
@@ -100,38 +115,116 @@ function winreg_enum_path(reg, path)
 		return new Array("HKLM", "HKU");
 	}
 	
-	handle = winreg_open_path(reg, path);
+	var handle = winreg_open_path(reg, path);
 	if (handle == undefined) {
 		return undefined;
 	}
 
 	var io = irpcObj();
-	var wtime = new Object();
-	wtime.low  = 2147483647;
-	wtime.high = 2147483647;
-	var keyname = new Object();
-	keyname.unknown  = 522;
-	keyname.key_name = NULL;
-
 	io.input.handle            = handle;
-	io.input.key_name_len      = 0;
-	io.input.unknown           = 1044;
-	io.input.in_name           = keyname;
-	io.input.class             = "";
-	io.input.last_changed_time = wtime;
-	
+	io.input.name = new Object();
+	io.input.name.length = 0;
+	io.input.name.size   = 32;
+	io.input.name.name   = NULL;
+	io.input.class = new Object();
+	io.input.class.length = 0;
+	io.input.class.size   = 1024;
+	io.input.class.name   = NULL;
+	io.input.last_changed_time = 0;
+
 	var idx = 0;
 	for (idx=0;idx >= 0;idx++) {
-		io.input.enum_index        = idx;
+		io.input.enum_index = idx;
 		var status = reg.winreg_EnumKey(io);
-		if (!status.is_ok) return;
+		if (!status.is_ok) {
+			winreg_close(reg, handle);
+			return;
+		}
+		var out = io.output;
+		if (out.result == "WERR_MORE_DATA") {
+			io.input.name.size = io.input.name.size * 2;
+			idx--;
+			if (io.input.name.size > 32000) {
+				winreg_close(reg, handle);
+				return undefined;
+			}
+			continue;
+		}
+		if (out.result != "WERR_OK") {
+			winreg_close(reg, handle);
+			return list;
+		}
+		list[list.length] = out.name.name;
+		list.length++;
+	}
+
+	winreg_close(reg, handle);
+	return list;
+}
+
+
+/*
+	return a list of values for a winreg server given a path
+	usage:
+	   list = winreg_enum_values(reg, path);
+
+	each returned list element is an object containing a name, a
+	type and a value
+*/
+function winreg_enum_values(reg, path)
+{
+	var list = new Object();
+	list.length = 0;
+
+	var handle = winreg_open_path(reg, path);
+	if (handle == undefined) {
+		return undefined;
+	}
+
+	var io = irpcObj();
+	io.input.handle      = handle;
+	io.input.name        = new Object();
+	io.input.name.length = 0;
+	io.input.name.size   = 128;
+	io.input.name.name   = "";
+	io.input.type        = 0;
+	io.input.value       = new Array(0);
+	io.input.size        = 1024;
+	io.input.length      = 0;
+
+	var idx;
+	for (idx=0;idx >= 0;idx++) {
+		io.input.enum_index = idx;
+		var status = reg.winreg_EnumValue(io);
+		if (!status.is_ok) {
+			winreg_close(reg, handle);
+			return;
+		}
 		var out = io.output;
+		if (out.result == "WERR_MORE_DATA") {
+			io.input.size = io.input.size * 2;
+			io.input.name.size = io.input.name.size * 2;
+			idx--;
+			/* limit blobs to 1M */
+			if (io.input.size > 1000000) {
+				winreg_close(reg, handle);
+				return undefined;
+			}
+			continue;
+		}
 		if (out.result != "WERR_OK") {
+			winreg_close(reg, handle);
 			return list;
 		}
-		list[list.length] = out.out_name.name;
+		var el   = new Object();
+		el.name  = out.name.name;
+		el.type  = out.type;
+		el.value = out.value;
+		el.size  = out.size;
+		list[list.length] = el;
 		list.length++;
 	}
 
+	winreg_close(reg, handle);
 	return list;
 }
diff --git a/source4/torture/rpc/winreg.c b/source4/torture/rpc/winreg.c
index 969bba0691..7d40147a8d 100644
--- a/source4/torture/rpc/winreg.c
+++ b/source4/torture/rpc/winreg.c
@@ -223,7 +223,7 @@ static BOOL test_OpenKey(struct dcerpc_pipe *p, TALLOC_CTX *mem_ctx,
 	r.in.handle = hive_handle;
 	init_winreg_String(&r.in.keyname, keyname);
 	r.in.unknown = 0x00000000;
-	r.in.access_mask = 0x02000000;
+	r.in.access_mask = SEC_FLAG_MAXIMUM_ALLOWED;
 	r.out.handle = key_handle;
 
 	status = dcerpc_winreg_OpenKey(p, mem_ctx, &r);
@@ -301,35 +301,37 @@ static BOOL test_EnumKey(struct dcerpc_pipe *p, TALLOC_CTX *mem_ctx,
 			 struct policy_handle *handle, int depth)
 {
 	struct winreg_EnumKey r;
-	struct winreg_EnumKeyNameRequest keyname;
-	struct winreg_String classname;
-	struct winreg_Time tm;
+	struct winreg_StringBuf class, name;
 	NTSTATUS status;
+	NTTIME t = 0;
 
 	printf("Testing EnumKey\n\n");
 
+	class.length = 0;
+	class.size   = 0;
+	class.name   = NULL;
+
 	r.in.handle = handle;
 	r.in.enum_index = 0;
-	r.in.key_name_len = r.out.key_name_len = 0;
-	r.in.unknown = r.out.unknown = 0x0414;
-	keyname.unknown = 0x0000020a;
-	init_winreg_String(&keyname.key_name, NULL);
-	init_winreg_String(&classname, NULL);
-	r.in.in_name = &keyname;
-	r.in.class = &classname;
-	tm.low = tm.high = 0x7fffffff;
-	r.in.last_changed_time = &tm;
+	r.in.name = &name;
+	r.in.class = &class;
+	r.out.name = &name;
+	r.in.last_changed_time = &t;
 
 	do {
+		name.length = 0;
+		name.size   = 1024;
+		name.name   = NULL;
+
 		status = dcerpc_winreg_EnumKey(p, mem_ctx, &r);
 
 		if (NT_STATUS_IS_OK(status) && W_ERROR_IS_OK(r.out.result)) {
 			struct policy_handle key_handle;
 
-			printf("EnumKey: %d: %s\n", r.in.enum_index, r.out.out_name->name);
+			printf("EnumKey: %d: %s\n", r.in.enum_index, r.out.name->name);
 
 			if (!test_OpenKey(
-				    p, mem_ctx, handle, r.out.out_name->name,
+				    p, mem_ctx, handle, r.out.name->name,
 				    &key_handle)) {
 			} else {
 				test_key(p, mem_ctx, &key_handle, depth + 1);
@@ -433,15 +435,18 @@ static BOOL test_EnumValue(struct dcerpc_pipe *p, TALLOC_CTX *mem_ctx,
 	uint32_t size = max_valbufsize, zero = 0;
 	BOOL ret = True;
 	uint8_t buf8;
-	uint16_t buf16;
+	struct winreg_StringBuf name;
 
 	printf("testing EnumValue\n");
 
+	name.length = 0;
+	name.size   = 1024;
+	name.name   = "";
+
 	r.in.handle = handle;
 	r.in.enum_index = 0;
-	r.in.name_in.length = 0;
-	r.in.name_in.size = 0x200;
-	r.in.name_in.name = &buf16;
+	r.in.name = &name;
+	r.out.name = &name;
 	r.in.type = &type;
 	r.in.value = &buf8;
 	r.in.length = &zero;
@@ -455,8 +460,8 @@ static BOOL test_EnumValue(struct dcerpc_pipe *p, TALLOC_CTX *mem_ctx,
 		}
 
 		if (W_ERROR_IS_OK(r.out.result)) {
-			ret &= test_QueryValue(p, mem_ctx, handle, r.out.name_out.name);
-			ret &= test_QueryMultipleValues(p, mem_ctx, handle, r.out.name_out.name);
+			ret &= test_QueryValue(p, mem_ctx, handle, r.out.name->name);
+			ret &= test_QueryMultipleValues(p, mem_ctx, handle, r.out.name->name);
 		}
 
 		r.in.enum_index++;