* [PATCH 2/3] pem/cert/tls: Use l_secure_memeq for verifying padding
2021-02-10 23:36 [PATCH 1/3] util: Add a constant time version of l_memeq Andrew Zaborowski
@ 2021-02-10 23:36 ` Andrew Zaborowski
2021-02-10 23:36 ` [PATCH 3/3] pem/cert: Ensure RFC8018/RFC1423 padding isn't 0 Andrew Zaborowski
2021-02-10 23:47 ` [PATCH 1/3] util: Add a constant time version of l_memeq Denis Kenzior
2 siblings, 0 replies; 4+ messages in thread
From: Andrew Zaborowski @ 2021-02-10 23:36 UTC (permalink / raw)
To: ell
[-- Attachment #1: Type: text/plain, Size: 3378 bytes --]
Instead of open-coding for loops for verifying padding, use
l_secure_memeq. This simplifies the code and makes the intent of
the logic easier to understand.
---
ell/cert.c | 24 +++++++++++-------------
ell/pem.c | 12 ++++++------
ell/tls-record.c | 7 +++----
3 files changed, 20 insertions(+), 23 deletions(-)
diff --git a/ell/cert.c b/ell/cert.c
index c26051f..3f2db1e 100644
--- a/ell/cert.c
+++ b/ell/cert.c
@@ -599,7 +599,7 @@ struct l_key *cert_key_from_pkcs8_encrypted_private_key_info(const uint8_t *der,
size_t key_info_len, alg_id_len, data_len, tmp_len;
struct l_cipher *alg;
uint8_t *decrypted;
- int i;
+ uint8_t pad;
struct l_key *pkey;
bool r;
bool is_block;
@@ -639,24 +639,22 @@ struct l_key *cert_key_from_pkcs8_encrypted_private_key_info(const uint8_t *der,
}
decrypted_len = data_len;
+ pad = decrypted[data_len - 1];
- if (is_block) {
- /*
- * For block ciphers strip padding as defined in RFC8018
- * (for PKCS#5 v1) or RFC1423 / RFC5652 (for v2).
- */
+ /*
+ * For block ciphers strip padding as defined in RFC8018
+ * (for PKCS#5 v1) or RFC1423 / RFC5652 (for v2).
+ */
+ if (is_block && pad) {
pkey = NULL;
- if (decrypted[data_len - 1] >= data_len ||
- decrypted[data_len - 1] > 16)
+ if (pad > data_len || pad > 16)
goto cleanup;
- for (i = 1; i < decrypted[data_len - 1]; i++)
- if (decrypted[data_len - 1 - i] !=
- decrypted[data_len - 1])
- goto cleanup;
+ if (!l_secure_memeq(decrypted + data_len - pad, pad - 1U, pad))
+ goto cleanup;
- decrypted_len -= decrypted[data_len - 1];
+ decrypted_len -= pad;
}
pkey = cert_key_from_pkcs8_private_key_info(decrypted, decrypted_len);
diff --git a/ell/pem.c b/ell/pem.c
index 67d2af1..d33cf68 100644
--- a/ell/pem.c
+++ b/ell/pem.c
@@ -707,8 +707,8 @@ struct l_key *pem_load_private_key(uint8_t *content, size_t len, char *label,
if (dekalgid) {
struct l_cipher *alg;
bool r;
- int i;
size_t block_len;
+ uint8_t pad;
if (encrypted)
*encrypted = true;
@@ -731,14 +731,14 @@ struct l_key *pem_load_private_key(uint8_t *content, size_t len, char *label,
goto err;
/* Remove padding like in RFC1423 Section 1.1 */
- if (content[len - 1] > block_len)
+ pad = content[len - 1];
+ if (pad > block_len)
goto err;
- for (i = 1; i < content[len - 1]; i++)
- if (content[len - 1 - i] != content[len - 1])
- goto err;
+ if (!l_secure_memeq(content + len - pad, pad - 1U, pad))
+ goto err;
- len -= content[len - 1];
+ len -= pad;
}
pkey = cert_key_from_pkcs1_rsa_private_key(content, len);
diff --git a/ell/tls-record.c b/ell/tls-record.c
index 505263d..cdbd0b8 100644
--- a/ell/tls-record.c
+++ b/ell/tls-record.c
@@ -522,10 +522,9 @@ static bool tls_handle_ciphertext(struct l_tls *tls)
tls->mac_length[0];
l_put_be16(compressed_len, compressed + 11);
- for (i = 0; i < padding_len; i++)
- if (compressed[13 + cipher_output_len - 1 -
- padding_len + i] != padding_len)
- error = 1;
+ error |= !l_secure_memeq(compressed + 13 + cipher_output_len -
+ 1 - padding_len, padding_len,
+ padding_len);
/* Calculate the MAC if needed */
tls_write_mac(tls, compressed + 8, 5 + compressed_len,
--
2.27.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 3/3] pem/cert: Ensure RFC8018/RFC1423 padding isn't 0
2021-02-10 23:36 [PATCH 1/3] util: Add a constant time version of l_memeq Andrew Zaborowski
2021-02-10 23:36 ` [PATCH 2/3] pem/cert/tls: Use l_secure_memeq for verifying padding Andrew Zaborowski
@ 2021-02-10 23:36 ` Andrew Zaborowski
2021-02-10 23:47 ` [PATCH 1/3] util: Add a constant time version of l_memeq Denis Kenzior
2 siblings, 0 replies; 4+ messages in thread
From: Andrew Zaborowski @ 2021-02-10 23:36 UTC (permalink / raw)
To: ell
[-- Attachment #1: Type: text/plain, Size: 1515 bytes --]
---
ell/cert.c | 8 ++++----
ell/pem.c | 2 +-
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/ell/cert.c b/ell/cert.c
index 3f2db1e..1413563 100644
--- a/ell/cert.c
+++ b/ell/cert.c
@@ -599,7 +599,6 @@ struct l_key *cert_key_from_pkcs8_encrypted_private_key_info(const uint8_t *der,
size_t key_info_len, alg_id_len, data_len, tmp_len;
struct l_cipher *alg;
uint8_t *decrypted;
- uint8_t pad;
struct l_key *pkey;
bool r;
bool is_block;
@@ -639,16 +638,17 @@ struct l_key *cert_key_from_pkcs8_encrypted_private_key_info(const uint8_t *der,
}
decrypted_len = data_len;
- pad = decrypted[data_len - 1];
/*
* For block ciphers strip padding as defined in RFC8018
* (for PKCS#5 v1) or RFC1423 / RFC5652 (for v2).
*/
- if (is_block && pad) {
+ if (is_block) {
+ uint8_t pad = decrypted[data_len - 1];
+
pkey = NULL;
- if (pad > data_len || pad > 16)
+ if (pad > data_len || pad > 16 || pad == 0)
goto cleanup;
if (!l_secure_memeq(decrypted + data_len - pad, pad - 1U, pad))
diff --git a/ell/pem.c b/ell/pem.c
index d33cf68..aec59ef 100644
--- a/ell/pem.c
+++ b/ell/pem.c
@@ -732,7 +732,7 @@ struct l_key *pem_load_private_key(uint8_t *content, size_t len, char *label,
/* Remove padding like in RFC1423 Section 1.1 */
pad = content[len - 1];
- if (pad > block_len)
+ if (pad > block_len || pad == 0)
goto err;
if (!l_secure_memeq(content + len - pad, pad - 1U, pad))
--
2.27.0
^ permalink raw reply related [flat|nested] 4+ messages in thread