[base16] Add buffer size parameter to base16_encode() and base16_decode()

The current API for Base16 (and Base64) encoding requires the caller
to always provide sufficient buffer space.  This prevents the use of
the generic encoding/decoding functionality in some situations, such
as in formatting the hex setting types.

Implement a generic hex_encode() (based on the existing
format_hex_setting()), implement base16_encode() and base16_decode()
in terms of the more generic hex_encode() and hex_decode(), and update
all callers to provide the additional buffer length parameter.

Signed-off-by: Michael Brown <mcb30@ipxe.org>

12 files changed, 112 insertions, 127 deletions

diff --git a/src/core/base16.c b/src/core/base16.c
index f5177c07..f9e0f336 100644
--- a/src/core/base16.c
+++ b/src/core/base16.c
@@ -28,6 +28,7 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
 #include <errno.h>
 #include <assert.h>
 #include <ipxe/string.h>
+#include <ipxe/vsprintf.h>
 #include <ipxe/base16.h>
 
 /** @file
@@ -37,48 +38,42 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
  */
 
 /**
- * Base16-encode data
+ * Encode hexadecimal string (with optional byte separator character)
  *
+ * @v separator		Byte separator character, or 0 for no separator
  * @v raw		Raw data
- * @v len		Length of raw data
- * @v encoded		Buffer for encoded string
- *
- * The buffer must be the correct length for the encoded string.  Use
- * something like
- *
- *     char buf[ base16_encoded_len ( len ) + 1 ];
- *
- * (the +1 is for the terminating NUL) to provide a buffer of the
- * correct size.
+ * @v raw_len		Length of raw data
+ * @v data		Buffer
+ * @v len		Length of buffer
+ * @ret len		Encoded length
  */
-void base16_encode ( const uint8_t *raw, size_t len, char *encoded ) {
-	const uint8_t *raw_bytes = raw;
-	char *encoded_bytes = encoded;
-	size_t remaining = len;
-
-	/* Encode each byte */
-	for ( ; remaining-- ; encoded_bytes += 2 ) {
-		sprintf ( encoded_bytes, "%02x", *(raw_bytes++) );
+size_t hex_encode ( char separator, const void *raw, size_t raw_len,
+		    char *data, size_t len ) {
+	const uint8_t *bytes = raw;
+	const char delimiter[2] = { separator, '\0' };
+	size_t used = 0;
+	unsigned int i;
+
+	if ( len )
+		data[0] = 0; /* Ensure that a terminating NUL exists */
+	for ( i = 0 ; i < raw_len ; i++ ) {
+		used += ssnprintf ( ( data + used ), ( len - used ),
+				    "%s%02x", ( used ? delimiter : "" ),
+				    bytes[i] );
 	}
-
-	/* Ensure terminating NUL exists even if length was zero */
-	*encoded_bytes = '\0';
-
-	DBG ( "Base16-encoded to \"%s\":\n", encoded );
-	DBG_HDA ( 0, raw, len );
-	assert ( strlen ( encoded ) == base16_encoded_len ( len ) );
+	return used;
 }
 
 /**
- * Decode hexadecimal string
+ * Decode hexadecimal string (with optional byte separator character)
  *
- * @v encoded		Encoded string
  * @v separator		Byte separator character, or 0 for no separator
+ * @v encoded		Encoded string
  * @v data		Buffer
  * @v len		Length of buffer
  * @ret len		Length of data, or negative error
  */
-int hex_decode ( const char *encoded, char separator, void *data, size_t len ) {
+int hex_decode ( char separator, const char *encoded, void *data, size_t len ) {
 	uint8_t *out = data;
 	unsigned int count = 0;
 	unsigned int sixteens;
@@ -110,31 +105,3 @@ int hex_decode ( const char *encoded, char separator, void *data, size_t len ) {
 	}
 	return count;
 }
-
-/**
- * Base16-decode data
- *
- * @v encoded		Encoded string
- * @v raw		Raw data
- * @ret len		Length of raw data, or negative error
- *
- * The buffer must be large enough to contain the decoded data.  Use
- * something like
- *
- *     char buf[ base16_decoded_max_len ( encoded ) ];
- *
- * to provide a buffer of the correct size.
- */
-int base16_decode ( const char *encoded, uint8_t *raw ) {
-	int len;
-
-	len = hex_decode ( encoded, 0, raw, -1UL );
-	if ( len < 0 )
-		return len;
-
-	DBG ( "Base16-decoded \"%s\" to:\n", encoded );
-	DBG_HDA ( 0, raw, len );
-	assert ( len <= ( int ) base16_decoded_max_len ( encoded ) );
-
-	return len;
-}
diff --git a/src/core/settings.c b/src/core/settings.c
index bd242f6b..be48ea7a 100644
--- a/src/core/settings.c
+++ b/src/core/settings.c
@@ -2006,32 +2006,6 @@ const struct setting_type setting_type_uint32 __setting_type =
 	SETTING_TYPE_UINT ( SETTING_TYPE_INT32 );
 
 /**
- * Format hex string setting value
- *
- * @v delimiter		Byte delimiter
- * @v raw		Raw setting value
- * @v raw_len		Length of raw setting value
- * @v buf		Buffer to contain formatted value
- * @v len		Length of buffer
- * @ret len		Length of formatted value, or negative error
- */
-static int format_hex_setting ( const char *delimiter, const void *raw,
-				size_t raw_len, char *buf, size_t len ) {
-	const uint8_t *bytes = raw;
-	int used = 0;
-	unsigned int i;
-
-	if ( len )
-		buf[0] = 0; /* Ensure that a terminating NUL exists */
-	for ( i = 0 ; i < raw_len ; i++ ) {
-		used += ssnprintf ( ( buf + used ), ( len - used ),
-				    "%s%02x", ( used ? delimiter : "" ),
-				    bytes[i] );
-	}
-	return used;
-}
-
-/**
  * Parse hex string setting value (using colon delimiter)
  *
  * @v type		Setting type
@@ -2043,7 +2017,7 @@ static int format_hex_setting ( const char *delimiter, const void *raw,
  */
 static int parse_hex_setting ( const struct setting_type *type __unused,
 			       const char *value, void *buf, size_t len ) {
-	return hex_decode ( value, ':', buf, len );
+	return hex_decode ( ':', value, buf, len );
 }
 
 /**
@@ -2059,7 +2033,7 @@ static int parse_hex_setting ( const struct setting_type *type __unused,
 static int format_hex_colon_setting ( const struct setting_type *type __unused,
 				      const void *raw, size_t raw_len,
 				      char *buf, size_t len ) {
-	return format_hex_setting ( ":", raw, raw_len, buf, len );
+	return hex_encode ( ':', raw, raw_len, buf, len );
 }
 
 /**
@@ -2075,7 +2049,7 @@ static int format_hex_colon_setting ( const struct setting_type *type __unused,
 static int parse_hex_hyphen_setting ( const struct setting_type *type __unused,
 				      const char *value, void *buf,
 				      size_t len ) {
-	return hex_decode ( value, '-', buf, len );
+	return hex_decode ( '-', value, buf, len );
 }
 
 /**
@@ -2091,7 +2065,7 @@ static int parse_hex_hyphen_setting ( const struct setting_type *type __unused,
 static int format_hex_hyphen_setting ( const struct setting_type *type __unused,
 				       const void *raw, size_t raw_len,
 				       char *buf, size_t len ) {
-	return format_hex_setting ( "-", raw, raw_len, buf, len );
+	return hex_encode ( '-', raw, raw_len, buf, len );
 }
 
 /**
@@ -2106,7 +2080,7 @@ static int format_hex_hyphen_setting ( const struct setting_type *type __unused,
  */
 static int parse_hex_raw_setting ( const struct setting_type *type __unused,
 				   const char *value, void *buf, size_t len ) {
-	return hex_decode ( value, 0, buf, len );
+	return hex_decode ( 0, value, buf, len );
 }
 
 /**
@@ -2122,7 +2096,7 @@ static int parse_hex_raw_setting ( const struct setting_type *type __unused,
 static int format_hex_raw_setting ( const struct setting_type *type __unused,
 				    const void *raw, size_t raw_len,
 				    char *buf, size_t len ) {
-	return format_hex_setting ( "", raw, raw_len, buf, len );
+	return hex_encode ( 0, raw, raw_len, buf, len );
 }
 
 /** A hex-string setting (colon-delimited) */
diff --git a/src/crypto/x509.c b/src/crypto/x509.c
index 49a1bce7..00eb226e 100644
--- a/src/crypto/x509.c
+++ b/src/crypto/x509.c
@@ -143,7 +143,8 @@ const char * x509_name ( struct x509_certificate *cert ) {
 	} else {
 		/* Certificate has no commonName: use SHA-1 fingerprint */
 		x509_fingerprint ( cert, digest, fingerprint );
-		base16_encode ( fingerprint, sizeof ( fingerprint ), buf );
+		base16_encode ( fingerprint, sizeof ( fingerprint ),
+				buf, sizeof ( buf ) );
 	}
 	return buf;
 }
diff --git a/src/drivers/net/ecm.c b/src/drivers/net/ecm.c
index 33f80e02..8c84ea9e 100644
--- a/src/drivers/net/ecm.c
+++ b/src/drivers/net/ecm.c
@@ -105,7 +105,7 @@ int ecm_fetch_mac ( struct usb_device *usb,
 		return -EINVAL;
 
 	/* Decode MAC address */
-	len = base16_decode ( buf, hw_addr );
+	len = base16_decode ( buf, hw_addr, ETH_ALEN );
 	if ( len < 0 ) {
 		rc = len;
 		return rc;
diff --git a/src/drivers/net/netfront.c b/src/drivers/net/netfront.c
index 6a1e4fc1..2f4bbf2a 100644
--- a/src/drivers/net/netfront.c
+++ b/src/drivers/net/netfront.c
@@ -139,7 +139,7 @@ static int netfront_read_mac ( struct netfront_nic *netfront, void *hw_addr ) {
 		xendev->key, mac );
 
 	/* Decode MAC address */
-	len = hex_decode ( mac, ':', hw_addr, ETH_ALEN );
+	len = hex_decode ( ':', mac, hw_addr, ETH_ALEN );
 	if ( len < 0 ) {
 		rc = len;
 		DBGC ( netfront, "NETFRONT %s could not decode MAC address "
diff --git a/src/include/ipxe/base16.h b/src/include/ipxe/base16.h
index fb20c9d5..8c44da17 100644
--- a/src/include/ipxe/base16.h
+++ b/src/include/ipxe/base16.h
@@ -32,9 +32,36 @@ static inline size_t base16_decoded_max_len ( const char *encoded ) {
 	return ( ( strlen ( encoded ) + 1 ) / 2 );
 }
 
-extern void base16_encode ( const uint8_t *raw, size_t len, char *encoded );
-extern int hex_decode ( const char *string, char separator, void *data,
+extern size_t hex_encode ( char separator, const void *raw, size_t raw_len,
+			   char *data, size_t len );
+extern int hex_decode ( char separator, const char *encoded, void *data,
 			size_t len );
-extern int base16_decode ( const char *encoded, uint8_t *raw );
+
+/**
+ * Base16-encode data
+ *
+ * @v raw		Raw data
+ * @v raw_len		Length of raw data
+ * @v data		Buffer
+ * @v len		Length of buffer
+ * @ret len		Encoded length
+ */
+static inline __attribute__ (( always_inline )) size_t
+base16_encode ( const void *raw, size_t raw_len, char *data, size_t len ) {
+	return hex_encode ( 0, raw, raw_len, data, len );
+}
+
+/**
+ * Base16-decode data
+ *
+ * @v encoded		Encoded string
+ * @v data		Buffer
+ * @v len		Length of buffer
+ * @ret len		Length of data, or negative error
+ */
+static inline __attribute__ (( always_inline )) int
+base16_decode ( const char *encoded, void *data, size_t len ) {
+	return hex_decode ( 0, encoded, data, len );
+}
 
 #endif /* _IPXE_BASE16_H */
diff --git a/src/interface/efi/efi_debug.c b/src/interface/efi/efi_debug.c
index e426c795..47380395 100644
--- a/src/interface/efi/efi_debug.c
+++ b/src/interface/efi/efi_debug.c
@@ -330,7 +330,7 @@ const char * efi_devpath_text ( EFI_DEVICE_PATH_PROTOCOL *path ) {
 		max_len = ( ( sizeof ( text ) - 1 /* NUL */ ) / 2 /* "xx" */ );
 		if ( len > max_len )
 			len = max_len;
-		base16_encode ( start, len, text );
+		base16_encode ( start, len, text, sizeof ( text ) );
 		return text;
 	}
 
diff --git a/src/net/infiniband/ib_srp.c b/src/net/infiniband/ib_srp.c
index 7b2b2b4e..3700184c 100644
--- a/src/net/infiniband/ib_srp.c
+++ b/src/net/infiniband/ib_srp.c
@@ -291,7 +291,7 @@ static int ib_srp_parse_byte_string ( const char *rp_comp, uint8_t *bytes,
 		return -EINVAL_BYTE_STRING_LEN;
 
 	/* Parse byte string */
-	decoded_size = base16_decode ( rp_comp, bytes );
+	decoded_size = base16_decode ( rp_comp, bytes, size );
 	if ( decoded_size < 0 )
 		return decoded_size;
 
diff --git a/src/net/pccrc.c b/src/net/pccrc.c
index 10342207..6ea26b3d 100644
--- a/src/net/pccrc.c
+++ b/src/net/pccrc.c
@@ -63,7 +63,7 @@ peerdist_info_hash_ntoa ( const struct peerdist_info *info, const void *hash ) {
 	assert ( base16_encoded_len ( digestsize ) < sizeof ( buf ) );
 
 	/* Transcribe hash value */
-	base16_encode ( hash, digestsize, buf );
+	base16_encode ( hash, digestsize, buf, sizeof ( buf ) );
 	return buf;
 }
 
diff --git a/src/net/tcp/httpcore.c b/src/net/tcp/httpcore.c
index 42a0f90e..d94ab5f0 100644
--- a/src/net/tcp/httpcore.c
+++ b/src/net/tcp/httpcore.c
@@ -1122,12 +1122,14 @@ static void http_digest_update ( struct md5_context *ctx, const char *string ) {
  *
  * @v ctx		Digest context
  * @v out		Buffer for digest output
+ * @v len		Buffer length
  */
-static void http_digest_final ( struct md5_context *ctx, char *out ) {
+static void http_digest_final ( struct md5_context *ctx, char *out,
+				size_t len ) {
 	uint8_t digest[MD5_DIGEST_SIZE];
 
 	digest_final ( &md5_algorithm, ctx, digest );
-	base16_encode ( digest, sizeof ( digest ), out );
+	base16_encode ( digest, sizeof ( digest ), out, len );
 }
 
 /**
@@ -1172,20 +1174,20 @@ static char * http_digest_auth ( struct http_request *http,
 	http_digest_update ( &ctx, user );
 	http_digest_update ( &ctx, realm );
 	http_digest_update ( &ctx, password );
-	http_digest_final ( &ctx, ha1 );
+	http_digest_final ( &ctx, ha1, sizeof ( ha1 ) );
 	if ( md5sess ) {
 		http_digest_init ( &ctx );
 		http_digest_update ( &ctx, ha1 );
 		http_digest_update ( &ctx, nonce );
 		http_digest_update ( &ctx, cnonce );
-		http_digest_final ( &ctx, ha1 );
+		http_digest_final ( &ctx, ha1, sizeof ( ha1 ) );
 	}
 
 	/* Generate HA2 */
 	http_digest_init ( &ctx );
 	http_digest_update ( &ctx, method );
 	http_digest_update ( &ctx, uri );
-	http_digest_final ( &ctx, ha2 );
+	http_digest_final ( &ctx, ha2, sizeof ( ha2 ) );
 
 	/* Generate response */
 	http_digest_init ( &ctx );
@@ -1197,7 +1199,7 @@ static char * http_digest_auth ( struct http_request *http,
 		http_digest_update ( &ctx, "auth" /* qop */ );
 	}
 	http_digest_update ( &ctx, ha2 );
-	http_digest_final ( &ctx, response );
+	http_digest_final ( &ctx, response, sizeof ( response ) );
 
 	/* Generate the authorisation string */
 	len = asprintf ( &auth, "Authorization: Digest username=\"%s\", "
diff --git a/src/net/tcp/iscsi.c b/src/net/tcp/iscsi.c
index 2e420d9a..e553b214 100644
--- a/src/net/tcp/iscsi.c
+++ b/src/net/tcp/iscsi.c
@@ -709,7 +709,7 @@ static int iscsi_build_login_request_strings ( struct iscsi_session *iscsi,
 		char buf[ base16_encoded_len ( iscsi->chap.response_len ) + 1 ];
 		assert ( iscsi->initiator_username != NULL );
 		base16_encode ( iscsi->chap.response, iscsi->chap.response_len,
-				buf );
+				buf, sizeof ( buf ) );
 		used += ssnprintf ( data + used, len - used,
 				    "CHAP_N=%s%cCHAP_R=0x%s%c",
 				    iscsi->initiator_username, 0, buf, 0 );
@@ -719,7 +719,7 @@ static int iscsi_build_login_request_strings ( struct iscsi_session *iscsi,
 		size_t challenge_len = ( sizeof ( iscsi->chap_challenge ) - 1 );
 		char buf[ base16_encoded_len ( challenge_len ) + 1 ];
 		base16_encode ( ( iscsi->chap_challenge + 1 ), challenge_len,
-				buf );
+				buf, sizeof ( buf ) );
 		used += ssnprintf ( data + used, len - used,
 				    "CHAP_I=%d%cCHAP_C=0x%s%c",
 				    iscsi->chap_challenge[0], 0, buf, 0 );
@@ -833,15 +833,17 @@ static int iscsi_tx_login_request ( struct iscsi_session *iscsi ) {
  *
  * @v encoded		Encoded large binary value
  * @v raw		Raw data
+ * @v len		Length of data buffer
  * @ret len		Length of raw data, or negative error
  */
-static int iscsi_large_binary_decode ( const char *encoded, uint8_t *raw ) {
+static int iscsi_large_binary_decode ( const char *encoded, uint8_t *raw,
+				       size_t len ) {
 
 	/* Check for initial '0x' or '0b' and decode as appropriate */
 	if ( *(encoded++) == '0' ) {
 		switch ( tolower ( *(encoded++) ) ) {
 		case 'x' :
-			return base16_decode ( encoded, raw );
+			return base16_decode ( encoded, raw, len );
 		case 'b' :
 			return base64_decode ( encoded, raw );
 		}
@@ -980,7 +982,7 @@ static int iscsi_handle_chap_c_value ( struct iscsi_session *iscsi,
 	int rc;
 
 	/* Process challenge */
-	len = iscsi_large_binary_decode ( value, buf );
+	len = iscsi_large_binary_decode ( value, buf, sizeof ( buf ) );
 	if ( len < 0 ) {
 		rc = len;
 		DBGC ( iscsi, "iSCSI %p invalid CHAP challenge \"%s\": %s\n",
@@ -1065,7 +1067,7 @@ static int iscsi_handle_chap_r_value ( struct iscsi_session *iscsi,
 	chap_respond ( &iscsi->chap );
 
 	/* Process response */
-	len = iscsi_large_binary_decode ( value, buf );
+	len = iscsi_large_binary_decode ( value, buf, sizeof ( buf ) );
 	if ( len < 0 ) {
 		rc = len;
 		DBGC ( iscsi, "iSCSI %p invalid CHAP response \"%s\": %s\n",
diff --git a/src/tests/base16_test.c b/src/tests/base16_test.c
index 04bf4e6c..46884aef 100644
--- a/src/tests/base16_test.c
+++ b/src/tests/base16_test.c
@@ -77,30 +77,42 @@ BASE16 ( random_test,
  * Report a base16 encoding test result
  *
  * @v test		Base16 test
+ * @v file		Test code file
+ * @v line		Test code line
  */
-#define base16_encode_ok( test ) do {					\
-	size_t len = base16_encoded_len ( (test)->len );		\
-	char buf[ len + 1 /* NUL */ ];					\
-	ok ( len == strlen ( (test)->encoded ) );			\
-	base16_encode ( (test)->data, (test)->len, buf );		\
-	ok ( strcmp ( (test)->encoded, buf ) == 0 );			\
-	} while ( 0 )
+static void base16_encode_okx ( struct base16_test *test, const char *file,
+				unsigned int line ) {
+	size_t len = base16_encoded_len ( test->len );
+	char buf[ len + 1 /* NUL */ ];
+	size_t check_len;
+
+	okx ( len == strlen ( test->encoded ), file, line );
+	check_len = base16_encode ( test->data, test->len, buf, sizeof ( buf ));
+	okx ( check_len == len, file, line );
+	okx ( strcmp ( test->encoded, buf ) == 0, file, line );
+}
+#define base16_encode_ok( test ) base16_encode_okx ( test, __FILE__, __LINE__ )
 
 /**
  * Report a base16 decoding test result
  *
  * @v test		Base16 test
+ * @v file		Test code file
+ * @v line		Test code line
  */
-#define base16_decode_ok( test ) do {					\
-	size_t max_len = base16_decoded_max_len ( (test)->encoded );	\
-	uint8_t buf[max_len];						\
-	int len;							\
-	len = base16_decode ( (test)->encoded, buf );			\
-	ok ( len >= 0 );						\
-	ok ( ( size_t ) len <= max_len );				\
-	ok ( ( size_t ) len == (test)->len );				\
-	ok ( memcmp ( (test)->data, buf, len ) == 0 );			\
-	} while ( 0 )
+static void base16_decode_okx ( struct base16_test *test, const char *file,
+				unsigned int line ) {
+	size_t max_len = base16_decoded_max_len ( test->encoded );
+	uint8_t buf[max_len];
+	int len;
+
+	len = base16_decode ( test->encoded, buf, sizeof ( buf ) );
+	okx ( len >= 0, file, line );
+	okx ( ( size_t ) len <= max_len, file, line );
+	okx ( ( size_t ) len == test->len, file, line );
+	okx ( memcmp ( test->data, buf, len ) == 0, file, line );
+}
+#define base16_decode_ok( test ) base16_decode_okx ( test, __FILE__, __LINE__ )
 
 /**
  * Perform Base16 self-tests