pem: do not parse ASN1 data after decryption (removes ASN1 dependency)

Now that we have padding verification after decryption and since
this can be used to validate the password as well there is no
need to parse ASN1 content any more, so we can simplify/remove
that dependency.

Signed-off-by: Valerio Setti <valerio.setti@nordicsemi.no>
This commit is contained in:
Valerio Setti 2024-02-16 14:40:42 +01:00
parent 8aff4ef274
commit 4cc6522a85
6 changed files with 8 additions and 42 deletions

View file

@ -1,4 +1,3 @@
Bugfix Bugfix
* mbedtls_pem_read_buffer() now performs a check on the padding data of * mbedtls_pem_read_buffer() now performs a check on the padding data of
decrypted keys and it rejects invalid ones. It also parses and validates decrypted keys and it rejects invalid ones.
the main ASN.1 SEQUENCE header.

View file

@ -198,7 +198,7 @@ typedef struct mbedtls_asn1_named_data {
mbedtls_asn1_named_data; mbedtls_asn1_named_data;
#if defined(MBEDTLS_ASN1_PARSE_C) || defined(MBEDTLS_X509_CREATE_C) || \ #if defined(MBEDTLS_ASN1_PARSE_C) || defined(MBEDTLS_X509_CREATE_C) || \
defined(MBEDTLS_PSA_UTIL_HAVE_ECDSA) || defined(MBEDTLS_PEM_PARSE_C) defined(MBEDTLS_PSA_UTIL_HAVE_ECDSA)
/** /**
* \brief Get the length of an ASN.1 element. * \brief Get the length of an ASN.1 element.
* Updates the pointer to immediately behind the length. * Updates the pointer to immediately behind the length.
@ -245,8 +245,7 @@ int mbedtls_asn1_get_len(unsigned char **p,
int mbedtls_asn1_get_tag(unsigned char **p, int mbedtls_asn1_get_tag(unsigned char **p,
const unsigned char *end, const unsigned char *end,
size_t *len, int tag); size_t *len, int tag);
#endif /* MBEDTLS_ASN1_PARSE_C || MBEDTLS_X509_CREATE_C || #endif /* MBEDTLS_ASN1_PARSE_C || MBEDTLS_X509_CREATE_C || MBEDTLS_PSA_UTIL_HAVE_ECDSA */
MBEDTLS_PSA_UTIL_HAVE_ECDSA || MBEDTLS_PEM_PARSE_C */
#if defined(MBEDTLS_ASN1_PARSE_C) #if defined(MBEDTLS_ASN1_PARSE_C)
/** /**

View file

@ -8,7 +8,7 @@
#include "common.h" #include "common.h"
#if defined(MBEDTLS_ASN1_PARSE_C) || defined(MBEDTLS_X509_CREATE_C) || \ #if defined(MBEDTLS_ASN1_PARSE_C) || defined(MBEDTLS_X509_CREATE_C) || \
defined(MBEDTLS_PSA_UTIL_HAVE_ECDSA) || defined(MBEDTLS_PEM_PARSE_C) defined(MBEDTLS_PSA_UTIL_HAVE_ECDSA)
#include "mbedtls/asn1.h" #include "mbedtls/asn1.h"
#include "mbedtls/platform_util.h" #include "mbedtls/platform_util.h"
@ -74,8 +74,7 @@ int mbedtls_asn1_get_tag(unsigned char **p,
return mbedtls_asn1_get_len(p, end, len); return mbedtls_asn1_get_len(p, end, len);
} }
#endif /* MBEDTLS_ASN1_PARSE_C || MBEDTLS_X509_CREATE_C || #endif /* MBEDTLS_ASN1_PARSE_C || MBEDTLS_X509_CREATE_C || MBEDTLS_PSA_UTIL_HAVE_ECDSA */
MBEDTLS_PSA_UTIL_HAVE_ECDSA || MBEDTLS_PEM_PARSE_C */
#if defined(MBEDTLS_ASN1_PARSE_C) #if defined(MBEDTLS_ASN1_PARSE_C)
int mbedtls_asn1_get_bool(unsigned char **p, int mbedtls_asn1_get_bool(unsigned char **p,

View file

@ -17,7 +17,6 @@
#include "mbedtls/cipher.h" #include "mbedtls/cipher.h"
#include "mbedtls/platform_util.h" #include "mbedtls/platform_util.h"
#include "mbedtls/error.h" #include "mbedtls/error.h"
#include "mbedtls/asn1.h"
#include <string.h> #include <string.h>
@ -466,28 +465,6 @@ int mbedtls_pem_read_buffer(mbedtls_pem_context *ctx, const char *header, const
mbedtls_zeroize_and_free(buf, len); mbedtls_zeroize_and_free(buf, len);
return ret; return ret;
} }
/*
* In RFC1421 PEM is used as container for DER (ASN.1) content so we
* can use ASN.1 functions to parse the main SEQUENCE tag and to get its
* length.
*/
unsigned char *p = buf;
size_t sequence_len;
ret = mbedtls_asn1_get_tag(&p, buf + len, &sequence_len,
MBEDTLS_ASN1_SEQUENCE | MBEDTLS_ASN1_CONSTRUCTED);
if (ret != 0) {
mbedtls_free(buf);
return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PEM_INVALID_DATA, ret);
}
/* Add also the sequence block (tag + len) to the total amount of valid data. */
sequence_len += (p - buf);
/* Ensure that the reported SEQUENCE length matches the data len (i.e. no
* trailing garbage data). */
if (len != sequence_len) {
return MBEDTLS_ERR_PEM_BAD_INPUT_DATA;
}
#else #else
mbedtls_zeroize_and_free(buf, len); mbedtls_zeroize_and_free(buf, len);
return MBEDTLS_ERR_PEM_FEATURE_UNAVAILABLE; return MBEDTLS_ERR_PEM_FEATURE_UNAVAILABLE;

View file

@ -61,21 +61,14 @@ PEM read (valid EC key encoded with AES-128-CBC)
depends_on:MBEDTLS_MD_CAN_MD5:MBEDTLS_AES_C:MBEDTLS_CIPHER_MODE_CBC depends_on:MBEDTLS_MD_CAN_MD5:MBEDTLS_AES_C:MBEDTLS_CIPHER_MODE_CBC
mbedtls_pem_read_buffer:"-----BEGIN EC PRIVATE KEY-----":"-----END EC PRIVATE KEY-----":"-----BEGIN EC PRIVATE KEY-----\nProc-Type\: 4,ENCRYPTED\nDEK-Info\: AES-128-CBC,151F851B6A7F3FBDAA5B7173117D0127\n\nLw+0OM+0Bwcl+ls/vxQbLrVshGc7bsNPvvtj2sJeMFFEq3V1mj/IO++0KK/CDhMH\nh6CZPsmgVOeM5uFpqYaq0fJbUduN2eDMWszWRm0SFkY=\n-----END EC PRIVATE KEY-----":"pwdpwd":0:"3041020101040f00d8023c809afd45e426d1a4dbe0ffa00706052b81040004a1220320000400da1ecfa53d528237625e119e2e0500d2eb671724f16deb6a63749516b7" mbedtls_pem_read_buffer:"-----BEGIN EC PRIVATE KEY-----":"-----END EC PRIVATE KEY-----":"-----BEGIN EC PRIVATE KEY-----\nProc-Type\: 4,ENCRYPTED\nDEK-Info\: AES-128-CBC,151F851B6A7F3FBDAA5B7173117D0127\n\nLw+0OM+0Bwcl+ls/vxQbLrVshGc7bsNPvvtj2sJeMFFEq3V1mj/IO++0KK/CDhMH\nh6CZPsmgVOeM5uFpqYaq0fJbUduN2eDMWszWRm0SFkY=\n-----END EC PRIVATE KEY-----":"pwdpwd":0:"3041020101040f00d8023c809afd45e426d1a4dbe0ffa00706052b81040004a1220320000400da1ecfa53d528237625e119e2e0500d2eb671724f16deb6a63749516b7"
# The text "hello world" (which is clearly not a valid ASN.1 SEQUENCE) is encoded # The text "hello world" together with some invalid padding data is encoded
# with AES-128-CBC to prove that ASN.1 parsing after decoding fails. # with AES-128-CBC in order to test padding validation.
# Since PBKDF1 isn't supported in OpenSSL, here's the steps: # Since PBKDF1 isn't supported in OpenSSL, here's the steps:
# 1. generate the key (password="password"; IV=0x3132333435363738 in hex or "12345678" as string) # 1. generate the key (password="password"; IV=0x3132333435363738 in hex or "12345678" as string)
# echo -n "password12345678" | openssl md5 # echo -n "password12345678" | openssl md5
# 2. encode data # 2. encode data
# echo -n "hello world" | openssl aes-128-cbc -e -base64 -p -K "bbb0ddff1b944b3cc68eaaeb7ac20099" -iv "3132333435363738"
PEM read (Invalid SEQUENCE encoded with AES-128-CBC)
depends_on:MBEDTLS_MD_CAN_MD5:MBEDTLS_AES_C:MBEDTLS_CIPHER_MODE_CBC
mbedtls_pem_read_buffer:"-----BEGIN EC PRIVATE KEY-----":"-----END EC PRIVATE KEY-----":"-----BEGIN EC PRIVATE KEY-----\nProc-Type\: 4,ENCRYPTED\nDEK-Info\: AES-128-CBC,31323334353637380000000000000000\n\nDfRGkwS+VjvR0IYsjZwW6Q==\n-----END EC PRIVATE KEY-----":"password":MBEDTLS_ERR_PEM_INVALID_DATA + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG:""
# Same as above, but with invalid padding data.
# Generated with:
# echo -n -e "\x68\x65\x6c\x6c\x6f\x20\x77\x6f\x72\x6c\x64\x01\x02\x03\x04\x05" | openssl aes-128-cbc -e -base64 -p -K "bbb0ddff1b944b3cc68eaaeb7ac20099" -iv "3132333435363738" -nopad # echo -n -e "\x68\x65\x6c\x6c\x6f\x20\x77\x6f\x72\x6c\x64\x01\x02\x03\x04\x05" | openssl aes-128-cbc -e -base64 -p -K "bbb0ddff1b944b3cc68eaaeb7ac20099" -iv "3132333435363738" -nopad
PEM read (Invalid padding data for AES-128-CBC) PEM read (AES-128-CBC, invalid padding data)
depends_on:MBEDTLS_MD_CAN_MD5:MBEDTLS_AES_C:MBEDTLS_CIPHER_MODE_CBC depends_on:MBEDTLS_MD_CAN_MD5:MBEDTLS_AES_C:MBEDTLS_CIPHER_MODE_CBC
mbedtls_pem_read_buffer:"-----BEGIN EC PRIVATE KEY-----":"-----END EC PRIVATE KEY-----":"-----BEGIN EC PRIVATE KEY-----\nProc-Type\: 4,ENCRYPTED\nDEK-Info\: AES-128-CBC,31323334353637380000000000000000\n\n333hxynfxEdXrSHQfIabxQ==\n-----END EC PRIVATE KEY-----":"password":MBEDTLS_ERR_PEM_BAD_INPUT_DATA:"" mbedtls_pem_read_buffer:"-----BEGIN EC PRIVATE KEY-----":"-----END EC PRIVATE KEY-----":"-----BEGIN EC PRIVATE KEY-----\nProc-Type\: 4,ENCRYPTED\nDEK-Info\: AES-128-CBC,31323334353637380000000000000000\n\n333hxynfxEdXrSHQfIabxQ==\n-----END EC PRIVATE KEY-----":"password":MBEDTLS_ERR_PEM_BAD_INPUT_DATA:""

View file

@ -3,7 +3,6 @@
#include "mbedtls/pem.h" #include "mbedtls/pem.h"
#include "mbedtls/des.h" #include "mbedtls/des.h"
#include "mbedtls/aes.h" #include "mbedtls/aes.h"
#include "mbedtls/asn1.h"
/* END_HEADER */ /* END_HEADER */
/* BEGIN_CASE depends_on:MBEDTLS_PEM_WRITE_C */ /* BEGIN_CASE depends_on:MBEDTLS_PEM_WRITE_C */