From 9aa8090d069eb0b36769f33544faf0e7e429e844 Mon Sep 17 00:00:00 2001 From: Michael Brown Date: Fri, 24 Apr 2015 14:34:32 +0100 Subject: [PATCH] [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 --- src/core/base16.c | 79 ++++++++++------------------------- src/core/settings.c | 38 +++-------------- src/crypto/x509.c | 3 +- src/drivers/net/ecm.c | 2 +- src/drivers/net/netfront.c | 2 +- src/include/ipxe/base16.h | 33 +++++++++++++-- src/interface/efi/efi_debug.c | 2 +- src/net/infiniband/ib_srp.c | 2 +- src/net/pccrc.c | 2 +- src/net/tcp/httpcore.c | 14 ++++--- src/net/tcp/iscsi.c | 14 ++++--- src/tests/base16_test.c | 46 ++++++++++++-------- 12 files changed, 111 insertions(+), 126 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 #include #include +#include #include /** @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; +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; - /* Encode each byte */ - for ( ; remaining-- ; encoded_bytes += 2 ) { - sprintf ( encoded_bytes, "%02x", *(raw_bytes++) ); + 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 @@ -2005,32 +2005,6 @@ const struct setting_type setting_type_uint16 __setting_type = 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) * @@ -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