[PATCH] make fetch_ldap_pw robust

[Jeremy Allison]

On Fri, Apr 21, 2017 at 04:09:54PM +0200, vl--- via samba-technical wrote:
> Hi!
> 
> Review appreciated!

Oh, I really like the storehex command. We've needed that
for a while - thanks !

> From 6452825815e52ab18522d0a4f3c3ba676405ec88 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Fri, 21 Apr 2017 13:05:12 +0200
> Subject: [PATCH 1/2] secrets: Protect against a non-0-terminated ldap password
> 
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
>  source3/passdb/secrets.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/source3/passdb/secrets.c b/source3/passdb/secrets.c
> index 4372c63..0ddee99 100644
> --- a/source3/passdb/secrets.c
> +++ b/source3/passdb/secrets.c
> @@ -316,6 +316,13 @@ bool fetch_ldap_pw(char **dn, char** pw)
>  	*pw=(char *)secrets_fetch(key, &size);
>  	SAFE_FREE(key);
>  
> +	if ((size != 0) && ((*pw)[size-1] != '\0')) {
> +		DBG_ERR("Non 0-terminated password for dn %s\n", *dn);
> +		SAFE_FREE(*pw);
> +		SAFE_FREE(*dn);
> +		return false;
> +	}
> +
>  	if (!size) {
>  		/* Upgrade 2.2 style entry */
>  		char *p;
> -- 
> 1.9.1
> 
> 
> From 3e5caed57f2c5dc89f20aee6d605cf0409760500 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Fri, 21 Apr 2017 14:10:33 +0200
> Subject: [PATCH 2/2] tdbtool: Add "storehex" command
> 
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
>  lib/tdb/man/tdbtool.8.xml | 10 ++++++
>  lib/tdb/tools/tdbtool.c   | 87 +++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 97 insertions(+)
> 
> diff --git a/lib/tdb/man/tdbtool.8.xml b/lib/tdb/man/tdbtool.8.xml
> index 9a9b95e..045cbde 100644
> --- a/lib/tdb/man/tdbtool.8.xml
> +++ b/lib/tdb/man/tdbtool.8.xml
> @@ -160,6 +160,16 @@
>  		</varlistentry>
>  
>  		<varlistentry>
> +		<term><option>storehex</option>
> +		<replaceable>KEY</replaceable>
> +		<replaceable>DATA</replaceable>
> +		</term>
> +		<listitem><para>Store (replace) a record in the
> +		current database where key and data are in hex format.
> +		</para></listitem>
> +		</varlistentry>
> +
> +		<varlistentry>
>  		<term><option>show</option>
>  		<replaceable>KEY</replaceable>
>  		</term>
> diff --git a/lib/tdb/tools/tdbtool.c b/lib/tdb/tools/tdbtool.c
> index beb3af1..e3535b9 100644
> --- a/lib/tdb/tools/tdbtool.c
> +++ b/lib/tdb/tools/tdbtool.c
> @@ -48,6 +48,7 @@ enum commands {
>  	CMD_DUMP,
>  	CMD_INSERT,
>  	CMD_MOVE,
> +	CMD_STOREHEX,
>  	CMD_STORE,
>  	CMD_SHOW,
>  	CMD_KEYS,
> @@ -83,6 +84,7 @@ COMMAND_TABLE cmd_table[] = {
>  	{"dump",	CMD_DUMP},
>  	{"insert",	CMD_INSERT},
>  	{"move",	CMD_MOVE},
> +	{"storehex",	CMD_STOREHEX},
>  	{"store",	CMD_STORE},
>  	{"show",	CMD_SHOW},
>  	{"keys",	CMD_KEYS},
> @@ -229,6 +231,7 @@ static void help(void)
>  "  info                 : print summary info about the database\n"
>  "  insert    key  data  : insert a record\n"
>  "  move      key  file  : move a record to a destination tdb\n"
> +"  storehex  key  data  : store a record (replace), key/value in hex format\n"
>  "  store     key  data  : store a record (replace)\n"
>  "  show      key        : show a record by key\n"
>  "  delete    key        : delete a record by key\n"
> @@ -346,6 +349,86 @@ static void store_tdb(char *keyname, size_t keylen, char* data, size_t datalen)
>  	}
>  }
>  
> +static bool hexchar(char c, uint8_t *v)
> +{
> +	if ((c >= '0') && (c <= '9')) {
> +		*v = (c - '0');
> +		return true;
> +	}
> +	if ((c >= 'A') && (c <= 'F')) {
> +		*v = (c - 'A' + 10);
> +		return true;
> +	}
> +	if ((c >= 'a') && (c <= 'f')) {
> +		*v = (c - 'a' + 10);
> +		return true;
> +	}
> +	return false;
> +}
> +
> +static bool parse_hex(const char *src, size_t srclen, uint8_t *dst)
> +{
> +	size_t i=0;
> +
> +	if ((srclen % 2) != 0) {
> +		return false;
> +	}
> +
> +	while (i<srclen) {
> +		bool ok;
> +		uint8_t hi,lo;
> +
> +		ok = (hexchar(src[i++], &hi) && hexchar(src[i++], &lo));
> +		if (!ok) {
> +			return false;
> +		}
> +		*dst = (hi<<4)|lo;
> +		dst += 1;
> +	}
> +
> +	return true;
> +}
> +
> +static void store_hex_tdb(char *keystr, size_t keylen,
> +			  char *datastr, size_t datalen)
> +{
> +	if ((keystr == NULL) || (keylen == 0)) {
> +		terror("need key");
> +		return;
> +	}
> +	if ((datastr == NULL) || (datalen == 0)) {
> +		terror("need data");
> +		return;
> +	}
> +
> +	{
> +		uint8_t keybuf[keylen/2];
> +		TDB_DATA key = { .dptr = keybuf, .dsize = sizeof(keybuf) };
> +		uint8_t databuf[datalen/2];
> +		TDB_DATA data = { .dptr = databuf, .dsize = sizeof(databuf) };
> +		bool ok;
> +
> +		ok = parse_hex(keystr, keylen, keybuf);
> +		if (!ok) {
> +			terror("need hex key");
> +			return;
> +		}
> +		ok = parse_hex(datastr, datalen, databuf);
> +		if (!ok) {
> +			terror("need hex data");
> +			return;
> +		}
> +
> +		printf("storing key/data:\n");
> +		print_data((char *)key.dptr, key.dsize);
> +		print_data((char *)data.dptr, data.dsize);
> +
> +		if (tdb_store(tdb, key, data, TDB_REPLACE) != 0) {
> +			terror("store failed");
> +		}
> +	}
> +}
> +
>  static void show_tdb(char *keyname, size_t keylen)
>  {
>  	TDB_DATA key, dbuf;
> @@ -693,6 +776,10 @@ static int do_command(void)
>  			bIterate = 0;
>  			store_tdb(arg1,arg1len,arg2,arg2len);
>  			return 0;
> +		case CMD_STOREHEX:
> +			bIterate = 0;
> +			store_hex_tdb(arg1,arg1len,arg2,arg2len);
> +			return 0;
>  		case CMD_SHOW:
>  			bIterate = 0;
>  			show_tdb(arg1, arg1len);
> -- 
> 1.9.1
>