[PATCH 0/3] Support any-salt for padding pss verification
This patchset does add support for any salt length in function padding_pss_verify which currently supports only max salt-length option. The fix is preceded by two changes to enhance memory consumption and const-correctness in the area of the patch.
- Hermann Gioja -
SESA644425 (3): lib: rsa: Fix const-correctness of rsassa_pss functions lib: rsa: Leverage existing data buffer instead of systematic copy lib: rsa: Update function padding_pss_verify (any-salt)
include/image.h | 2 +- include/u-boot/rsa.h | 4 +-- lib/rsa/rsa-verify.c | 64 +++++++++++++++++++++++--------------------- 3 files changed, 36 insertions(+), 34 deletions(-)
base-commit: 6d3c46ed0e230d999c3f20f7fd4f3a88c03b14ca
Prior to introduction of modifications in rsassa_pss functions related to padding verification, doing a pass to update const-correctness in targeted functions to comply with coding-rules and avoid const-cast
Signed-off-by: SESA644425 gioja.hermann@non.se.com --- Despite checkpath.pl recommendation, it is not possible to use u8 instead of uint8_t. Proceeding with u8 breaks build with error: unknown type name u8 include/image.h | 2 +- include/u-boot/rsa.h | 4 ++-- lib/rsa/rsa-verify.c | 14 +++++++------- 3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/include/image.h b/include/image.h index 97e5f2eb24..bea93118f8 100644 --- a/include/image.h +++ b/include/image.h @@ -1291,7 +1291,7 @@ ll_entry_declare(struct crypto_algo, __name, cryptos) struct padding_algo { const char *name; int (*verify)(struct image_sign_info *info, - uint8_t *pad, int pad_len, + const uint8_t *pad, int pad_len, const uint8_t *hash, int hash_len); };
diff --git a/include/u-boot/rsa.h b/include/u-boot/rsa.h index b9634e38d9..085363eb1e 100644 --- a/include/u-boot/rsa.h +++ b/include/u-boot/rsa.h @@ -101,11 +101,11 @@ int rsa_verify_with_pkey(struct image_sign_info *info, const void *hash, uint8_t *sig, uint sig_len);
int padding_pkcs_15_verify(struct image_sign_info *info, - uint8_t *msg, int msg_len, + const uint8_t *msg, int msg_len, const uint8_t *hash, int hash_len);
int padding_pss_verify(struct image_sign_info *info, - uint8_t *msg, int msg_len, + const uint8_t *msg, int msg_len, const uint8_t *hash, int hash_len);
#define RSA_DEFAULT_PADDING_NAME "pkcs-1.5" diff --git a/lib/rsa/rsa-verify.c b/lib/rsa/rsa-verify.c index 112664059c..c2c7248f90 100644 --- a/lib/rsa/rsa-verify.c +++ b/lib/rsa/rsa-verify.c @@ -73,7 +73,7 @@ static int rsa_verify_padding(const uint8_t *msg, const int pad_len, }
int padding_pkcs_15_verify(struct image_sign_info *info, - uint8_t *msg, int msg_len, + const uint8_t *msg, int msg_len, const uint8_t *hash, int hash_len) { struct checksum_algo *checksum = info->checksum; @@ -125,7 +125,7 @@ static void u32_i2osp(uint32_t val, uint8_t *buf) * Return: 0 if the octet string was correctly generated, others on error */ static int mask_generation_function1(struct checksum_algo *checksum, - uint8_t *seed, int seed_len, + const uint8_t *seed, int seed_len, uint8_t *output, int output_len) { struct image_region region[2]; @@ -176,9 +176,9 @@ out: }
static int compute_hash_prime(struct checksum_algo *checksum, - uint8_t *pad, int pad_len, - uint8_t *hash, int hash_len, - uint8_t *salt, int salt_len, + const uint8_t *pad, int pad_len, + const uint8_t *hash, int hash_len, + const uint8_t *salt, int salt_len, uint8_t *hprime) { struct image_region region[3]; @@ -215,7 +215,7 @@ out: * @hash_len: Length of the hash */ int padding_pss_verify(struct image_sign_info *info, - uint8_t *msg, int msg_len, + const uint8_t *msg, int msg_len, const uint8_t *hash, int hash_len) { uint8_t *masked_db = NULL; @@ -287,7 +287,7 @@ int padding_pss_verify(struct image_sign_info *info,
/* step 12 & 13 */ compute_hash_prime(checksum, pad_zero, 8, - (uint8_t *)hash, hash_len, + hash, hash_len, salt, salt_len, hprime);
/* step 14 */
On Wed, 9 Mar 2022 at 02:28, SESA644425 giojahermann@gmail.com wrote:
Prior to introduction of modifications in rsassa_pss functions related to padding verification, doing a pass to update const-correctness in targeted functions to comply with coding-rules and avoid const-cast
Signed-off-by: SESA644425 gioja.hermann@non.se.com
Please use your real name.
Despite checkpath.pl recommendation, it is not possible to use u8 instead of uint8_t. Proceeding with u8 breaks build with error: unknown type name u8 include/image.h | 2 +- include/u-boot/rsa.h | 4 ++-- lib/rsa/rsa-verify.c | 14 +++++++------- 3 files changed, 10 insertions(+), 10 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
On Wed, Mar 09, 2022 at 01:27:15AM -0800, SESA644425 wrote:
Prior to introduction of modifications in rsassa_pss functions related to padding verification, doing a pass to update const-correctness in targeted functions to comply with coding-rules and avoid const-cast
Signed-off-by: SESA644425 gioja.hermann@non.se.com Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
Prior to introduction of modifications in rsassa_pss functions related to padding verification, doing a pass to reduce memory consumption of function by replacing memory copies of parts of const buffer by pointers to the original buffer (masked_db and h are subparts of msg buffer which is declared const, salt is a subpart of db which is a working buffer, unmodified after being filled). New pointers scope is limited to the function where they are declared (not returned to caller by any mean), zeroing risk of memory fault related to the change.
Signed-off-by: SESA644425 gioja.hermann@non.se.com --- Despite checkpath.pl recommendation, it is not possible to use u8 instead of uint8_t. Proceeding with u8 breaks build with error: unknown type name u8 lib/rsa/rsa-verify.c | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-)
diff --git a/lib/rsa/rsa-verify.c b/lib/rsa/rsa-verify.c index c2c7248f90..de71234cfb 100644 --- a/lib/rsa/rsa-verify.c +++ b/lib/rsa/rsa-verify.c @@ -208,6 +208,10 @@ out: * saltlen:-1 "set the salt length to the digest length" is currently * not supported. * + * msg is a concatenation of : masked_db + h + 0xbc + * Once unmasked, db is a concatenation of : [0x00]* + 0x01 + salt + * Length of 0-padding at begin of db depends on salt length. + * * @info: Specifies key and FIT information * @msg: byte array of message, len equal to msg_len * @msg_len: Message length @@ -218,27 +222,25 @@ int padding_pss_verify(struct image_sign_info *info, const uint8_t *msg, int msg_len, const uint8_t *hash, int hash_len) { - uint8_t *masked_db = NULL; - int masked_db_len = msg_len - hash_len - 1; - uint8_t *h = NULL, *hprime = NULL; - int h_len = hash_len; + const uint8_t *masked_db = NULL; uint8_t *db_mask = NULL; - int db_mask_len = masked_db_len; - uint8_t *db = NULL, *salt = NULL; - int db_len = masked_db_len, salt_len = msg_len - hash_len - 2; + uint8_t *db = NULL; + int db_len = msg_len - hash_len - 1; + const uint8_t *h = NULL; + uint8_t *hprime = NULL; + int h_len = hash_len; + uint8_t *salt = NULL; + int salt_len = msg_len - hash_len - 2; uint8_t pad_zero[8] = { 0 }; int ret, i, leftmost_bits = 1; uint8_t leftmost_mask; struct checksum_algo *checksum = info->checksum;
/* first, allocate everything */ - masked_db = malloc(masked_db_len); - h = malloc(h_len); - db_mask = malloc(db_mask_len); + db_mask = malloc(db_len); db = malloc(db_len); - salt = malloc(salt_len); hprime = malloc(hash_len); - if (!masked_db || !h || !db_mask || !db || !salt || !hprime) { + if (!db_mask || !db || !hprime) { printf("%s: can't allocate some buffer\n", __func__); ret = -ENOMEM; goto out; @@ -252,8 +254,8 @@ int padding_pss_verify(struct image_sign_info *info, }
/* step 5 */ - memcpy(masked_db, msg, masked_db_len); - memcpy(h, msg + masked_db_len, h_len); + masked_db = &msg[0]; + h = &msg[db_len];
/* step 6 */ leftmost_mask = (0xff >> (8 - leftmost_bits)) << (8 - leftmost_bits); @@ -265,7 +267,7 @@ int padding_pss_verify(struct image_sign_info *info, }
/* step 7 */ - mask_generation_function1(checksum, h, h_len, db_mask, db_mask_len); + mask_generation_function1(checksum, h, h_len, db_mask, db_len);
/* step 8 */ for (i = 0; i < db_len; i++) @@ -283,7 +285,7 @@ int padding_pss_verify(struct image_sign_info *info, }
/* step 11 */ - memcpy(salt, &db[1], salt_len); + salt = &db[1];
/* step 12 & 13 */ compute_hash_prime(checksum, pad_zero, 8, @@ -295,11 +297,8 @@ int padding_pss_verify(struct image_sign_info *info,
out: free(hprime); - free(salt); free(db); free(db_mask); - free(h); - free(masked_db);
return ret; }
Hi,
On Wed, 9 Mar 2022 at 02:28, SESA644425 giojahermann@gmail.com wrote:
Prior to introduction of modifications in rsassa_pss functions related to padding verification, doing a pass to reduce memory consumption of function by replacing memory copies of parts of const buffer by pointers to the original buffer (masked_db and h are subparts of msg buffer which is declared const, salt is a subpart of db which is a working buffer, unmodified after being filled). New pointers scope is limited to the function where they are declared (not returned to caller by any mean), zeroing risk of memory fault related to the change.
Signed-off-by: SESA644425 gioja.hermann@non.se.com
Despite checkpath.pl recommendation, it is not possible to use u8 instead of uint8_t. Proceeding with u8 breaks build with error: unknown type name u8
Yes any code compiled by host tools must use uint8_t/
lib/rsa/rsa-verify.c | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
On Wed, Mar 09, 2022 at 01:27:16AM -0800, SESA644425 wrote:
Prior to introduction of modifications in rsassa_pss functions related to padding verification, doing a pass to reduce memory consumption of function by replacing memory copies of parts of const buffer by pointers to the original buffer (masked_db and h are subparts of msg buffer which is declared const, salt is a subpart of db which is a working buffer, unmodified after being filled). New pointers scope is limited to the function where they are declared (not returned to caller by any mean), zeroing risk of memory fault related to the change.
Signed-off-by: SESA644425 gioja.hermann@non.se.com Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
Modify function to support any salt length instead of max length only. Function now detects salt length by parsing the content of db buffer. Note that it works with (but is not limited to) zero-length, digest-length and max-length
Signed-off-by: SESA644425 gioja.hermann@non.se.com --- Despite checkpath.pl recommendation, it is not possible to use u8 instead of uint8_t. Proceeding with u8 breaks build with error: unknown type name u8 lib/rsa/rsa-verify.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/lib/rsa/rsa-verify.c b/lib/rsa/rsa-verify.c index de71234cfb..1d95cfbdee 100644 --- a/lib/rsa/rsa-verify.c +++ b/lib/rsa/rsa-verify.c @@ -204,9 +204,7 @@ out: /* * padding_pss_verify() - verify the pss padding of a signature * - * Only works with a rsa_pss_saltlen:-2 (default value) right now - * saltlen:-1 "set the salt length to the digest length" is currently - * not supported. + * Works with any salt length * * msg is a concatenation of : masked_db + h + 0xbc * Once unmasked, db is a concatenation of : [0x00]* + 0x01 + salt @@ -229,8 +227,8 @@ int padding_pss_verify(struct image_sign_info *info, const uint8_t *h = NULL; uint8_t *hprime = NULL; int h_len = hash_len; - uint8_t *salt = NULL; - int salt_len = msg_len - hash_len - 2; + uint8_t *db_nopad = NULL, *salt = NULL; + int db_padlen, salt_len; uint8_t pad_zero[8] = { 0 }; int ret, i, leftmost_bits = 1; uint8_t leftmost_mask; @@ -277,15 +275,20 @@ int padding_pss_verify(struct image_sign_info *info, db[0] &= 0xff >> leftmost_bits;
/* step 10 */ - if (db[0] != 0x01) { + db_padlen = 0; + while (db[db_padlen] == 0x00 && db_padlen < (db_len - 1)) + db_padlen++; + db_nopad = &db[db_padlen]; + if (db_nopad[0] != 0x01) { printf("%s: invalid pss padding ", __func__); - printf("(leftmost byte of db isn't 0x01)\n"); + printf("(leftmost byte of db after 0-padding isn't 0x01)\n"); ret = EINVAL; goto out; }
/* step 11 */ - salt = &db[1]; + salt_len = db_len - db_padlen - 1; + salt = &db_nopad[1];
/* step 12 & 13 */ compute_hash_prime(checksum, pad_zero, 8,
On Wed, 9 Mar 2022 at 02:28, SESA644425 giojahermann@gmail.com wrote:
Modify function to support any salt length instead of max length only. Function now detects salt length by parsing the content of db buffer. Note that it works with (but is not limited to) zero-length, digest-length and max-length
Signed-off-by: SESA644425 gioja.hermann@non.se.com
Despite checkpath.pl recommendation, it is not possible to use u8 instead of uint8_t. Proceeding with u8 breaks build with error: unknown type name u8 lib/rsa/rsa-verify.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Should we add a test for these cases to test_vboot.py ?
On Wed, Mar 09, 2022 at 01:27:17AM -0800, SESA644425 wrote:
Modify function to support any salt length instead of max length only. Function now detects salt length by parsing the content of db buffer. Note that it works with (but is not limited to) zero-length, digest-length and max-length
Signed-off-by: SESA644425 gioja.hermann@non.se.com Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
participants (3)
-
SESA644425 -
Simon Glass -
Tom Rini