I chose to divide along the lines of Weierstrass vs other curve shapes
(currently just Montgomery), mainly because it's the first thing that
came to mind.
It happened to reveal an issue in the logic for when (deterministic)
ECDSA and ECJPAKE are built-in, which this commit is also fixing.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
Separate the fits-in-buffer check (*data_length <= data_size) from the
we-think-it's-a-sensible-size check (*data_length <=
MBEDTLS_X509_MAX_DN_NAME_SIZE).
This requires using an intermediate buffer for the DER data, since its
maximum sensible size has to be larger than the maximum sensible size for
the payload, due to the overhead of the ASN.1 tag+length.
Remove test cases focusing on the DER length since the implementation no
longer has a threshold for it.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Fix the expected output in some test cases.
Add a few more test cases to exercise both a payload length around 256 bytes
and a DER length around 256 bytes, since both are placed in a 256-byte
buffer (value of MBEDTLS_X509_MAX_DN_NAME_SIZE).
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Due to differing validations amongst X.509 library functions, there are
inputs that mbedtls_x509_string_to_names() accepts, but it produces output
that some library functions can't parse. Accept this for now. Do call the
functions, even when we don't care about their return code: we're ok with
returning errors, but not with e.g. a buffer overflow.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
In particular, "X509 String to Names: long hexstring (DER=258 bytes, too long)"
causes a buffer overflow in parse_attribute_value_der_encoded().
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Before this PR, the tests passed without this, but they shouldn't have.
Accelerators need to declare what curves they support, and if no curve
is declared as accelerated, the built-in implementations of algs and key
types need to be enabled.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
Those components were introduced in #7103, resolving #6622: Some PSA
ECC size macros are too small when the largest accelerated curve is
larger than the largest built-in curve.
At that point, it was not possible yet to omit all built-in curves,
so we made these components that had only one (small) curve built-in and
all the others accelerated.
Now that it's possible to disable all ECC built-ins, and we have tests
doing that, we don't need that kind of fiddling any more.
Note: these component disabled RSA in order to make sure max key size
macros were not taken from RSA. We have test components with all of ECC
accelerated and RSA disabled
(component_test_psa_crypto_config_accel_ecc_no_bignum and
component_test_psa_crypto_config_accel_ecc_ffdh_no_bignum), making the
"all curves except one" components really redundant.
Note: removing them was one of the items in #7757.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
Conflicts:
* `include/mbedtls/build_info.h`: a new fragment to auto-enable
`MBEDTLS_CIPHER_PADDING_PKCS7` was added in
c9f4040f7f in `development-restricted`.
In `development`, this section of the file has moved to
`include/mbedtls/config_adjust_legacy_crypto.h`.
* `library/bignum.c`: function name change in `development-restricted` vs
comment change in development. The comment change in `development` is not
really relevant, so just take the line from `development-restricted`.
Components that accelerate an algorithm that uses hashing internally
(such as deterministic ECDSA and RSA-PSS) need the hash algorithms
available in libtestdriver1.
Previously, the omission of SHA-3 in
tests/include/test/drivers/crypto_config_test_driver_extension.h meant
it was enabled in libtestdriver1 when not requesting its acceleration,
and disabled when requesting it. Adding it in a previous commit fixed
the components that asked it accelerated, but broke the component that
didn't ask for it but still needed it.
Fix those components by explicitly requesting SHA-3 as we already do for
the other hash algorithms that are require for the same reason.
Note: this broke test_suite_psa_crypto_storage_format.v0 which is
apparently the only place exercising signatures with SHA-3.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
Switch pkparse to use new mbedtls_pkcs12_pbe_ext function
and deprecate mbedtls_pkcs12_pbe function.
Signed-off-by: Waleed Elmelegy <waleed.elmelegy@arm.com>
Switch pkparse to use new mbedtls_pkcs5_pbes2_ext function
and deprecate mbedtls_pkcs5_pbes2 function.
Signed-off-by: Waleed Elmelegy <waleed.elmelegy@arm.com>
Test mbedtls_ssl_decrypt_buf() with a null cipher (the only type of stream
cipher we support). Test the good case (to make sure the test code
constructs the input correctly), test with an invalid MAC, and test with a
shortened input.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
test_suite_ssl is huge and needs splitting.
Create a new test suite focused on mbedtls_ssl_decrypt_buf(), which is a
complicated function that needs more thorough testing with malformed inputs.
At this point, we are only doing negative testing with CBC-non-ETM test
suites. This needs to grow.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
The official spelling of the trade mark changed from all-lowercase "mbed"
to normal proper noun capitalization "Mbed" a few years ago. We've been
using the new spelling in new text but still have the old spelling in a
lot of text. This commit updates most occurrences of "mbed TLS":
```
sed -i -e 's/mbed TLS/Mbed TLS/g' $(git ls-files ':!ChangeLog' ':!tests/data_files/**' ':!tests/suites/*.data' ':!programs/x509/*' ':!configs/tfm*')
```
Justification for the omissions:
* `ChangeLog`: historical text.
* `test/data_files/**`, `tests/suites/*.data`, `programs/x509/*`: many
occurrences are significant names in certificates and such. Changing
the spelling would invalidate many signatures and tests.
* `configs/tfm*`: this is an imported file. We'll follow the upstream
updates.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Return back to modifying input parameters in pkcs12_parse_pbe_params
to avoid change in behaviour.
Signed-off-by: Waleed Elmelegy <waleed.elmelegy@arm.com>
Move the initialisation of the pkcs7 object to before the first possible
test failure, otherwise failure in those tests could result in an
uninitialised pointer being free'd. Found by coverity.
Signed-off-by: Paul Elliott <paul.elliott@arm.com>
These should be shared between ref and accel, for meaningful coverage
comparison.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
Builds, but 20 test cases failing in test_suite_psa_crypto, to be
addressed in future commits.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
Add new mbedtls_pkcs12_pbe_ext function to replace
old mbedtls_pkcs12_pbe function that have security
issues.
Signed-off-by: Waleed Elmelegy <waleed.elmelegy@arm.com>
In make builds, when GEN_FILES is false (empty), don't try to re-generate
configuration-independent source files, regardless of whether they seem
out of date. This is useful, for example, if you have a source tree where
`make generated_files` has already run and file timestamps reflect the
time the files were copied or extracted, and you are now in an environment
that lacks some of the necessary tools to re-generate the files.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
* Prevent pkcs12_pbe encryption when PKCS7 padding has been
disabled since this not part of the specs.
* Allow decryption when PKCS7 padding is disabled for legacy
reasons, However, invalid padding is not checked.
* Document new behaviour, known limitations and possible
security concerns.
* Add tests to check these scenarios. Test data has been
generated by the below code using OpenSSL as a reference:
#include <openssl/pkcs12.h>
#include <openssl/evp.h>
#include <openssl/des.h>
#include <openssl/asn1.h>
#include "crypto/asn1.h"
#include <string.h>
int main()
{
char pass[] = "\xBB\xBB\xBB\xBB\xBB\xBB\xBB\xBB\xBB";
unsigned char salt[] = "\xCC\xCC\xCC\xCC\xCC\xCC\xCC\xCC\xCC";
unsigned char plaintext[] = "\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA";
unsigned char *ciphertext = NULL;
int iter = 10;
X509_ALGOR *alg = X509_ALGOR_new();
int ciphertext_len = 0;
int alg_nid = NID_pbe_WithSHA1And3_Key_TripleDES_CBC;
alg->parameter = ASN1_TYPE_new();
struct asn1_object_st * aobj;
PKCS5_pbe_set0_algor(alg, alg_nid, iter,
salt, sizeof(salt)-1);
aobj = alg->algorithm;
printf("\"30%.2X", 2 + aobj->length + alg->parameter->value.asn1_string->length);
printf("06%.2X", aobj->length);
for (int i = 0; i < aobj->length; i++) {
printf("%.2X", aobj->data[i]);
}
for (int i = 0; i < alg->parameter->value.asn1_string->length; i++) {
printf("%.2X", alg->parameter->value.asn1_string->data[i]);
}
printf("\":\"");
for (int i = 0; i < sizeof(pass)-1; i++) {
printf("%.2X", pass[i] & 0xFF);
}
printf("\":\"");
for (int i = 0; i < sizeof(plaintext)-1; i++) {
printf("%.2X", plaintext[i]);
}
printf("\":");
printf("0");
printf(":\"");
unsigned char * res = PKCS12_pbe_crypt(alg, pass, sizeof(pass)-1, plaintext, sizeof(plaintext)-1, &ciphertext, &ciphertext_len, 1);
if (res == NULL)
printf("Encryption failed!\n");
for (int i = 0; i < ciphertext_len; i++) {
printf("%.2X", res[i]);
}
printf("\"\n");
return 0;
}
Signed-off-by: Waleed Elmelegy <waleed.elmelegy@arm.com>
#
Conflicts:
* `include/psa/crypto_sizes.h`: the addition of the `u` suffix in this branch
conflicts with the rework of the calculation of `PSA_HASH_MAX_SIZE` and
`PSA_HMAC_MAX_HASH_BLOCK_SIZE` in `development`. Use the new definitions
from `development`, and add the `u` suffix to the relevant constants.
Remove unnecessary "../library" prefix from test suite includes. This
makes the tests repo-agnostic between the mbedtls and psa-crypto repos.
Signed-off-by: David Horstmann <david.horstmann@arm.com>
This enables compatibility with older versions of CMake that do not have
the abbreviated switch.
Signed-off-by: David Horstmann <david.horstmann@arm.com>
Use CMake's -t option to build only the crypto target. Parameterize the
crypto target to have the right name depending on whether this is Mbed
TLS or PSA Crypto.
Signed-off-by: David Horstmann <david.horstmann@arm.com>
Either remove exclusive references to Mbed TLS or accompany them with
references to "PSA Crypto".
Signed-off-by: David Horstmann <david.horstmann@arm.com>
They aren't used anywhere.
Keep the command line options of all.sh to avoid breaking any wrapper
scripts that people might have.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
None of the tests actually need GNUTLS_LEGACY (3.3.8): GNUTLS (3.4.10)
works.
None of the tests actually need OPENSSL_LEGACY (1.0.1j): OPENSSL (1.0.2g)
works.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Use CMake to build the library out-of-source (rather than make)
in tests/scripts/test_psa_compliance.py and add a script argument for
the out-of-source build directory.
Signed-off-by: David Horstmann <david.horstmann@arm.com>
Introduce changes needed to run all.sh in the psa-crypto repo. Where
behaviour must differ, detect that we are in the psa-crypto repo by
checking for the 'core' directory.
Signed-off-by: David Horstmann <david.horstmann@arm.com>
The test for outputing a hexstring representation is actually
testing dn_gets, and is tested in test_suite_x509parse.
Signed-off-by: Agathiyan Bragadeesh <agathiyan.bragadeesh2@arm.com>
Due to change in handling non-ascii characters, existing tests had to be
updated to handle the new implementation. New tests and certificates
are added to test the escaping functionality in edge cases.
Signed-off-by: Agathiyan Bragadeesh <agathiyan.bragadeesh2@arm.com>
Converts none ascii to escaped hexpairs in mbedtls_x509_dn_gets and
interprets hexpairs in mbedtls_x509_string_to_names.
Signed-off-by: Agathiyan Bragadeesh <agathiyan.bragadeesh2@arm.com>
This escapes special characters according to RFC 4514 in
mbedtls_x509_dn_gets and de-escapes in mbedtls_x509_string_to_names.
This commit does not handle hexpairs.
Signed-off-by: Agathiyan Bragadeesh <agathiyan.bragadeesh2@arm.com>
* Don't break string literals in the allow list definition
* Comment each test that belongs to the allow list is there.
Signed-off-by: Tomás González <tomasagustin.gonzalezorlando@arm.com>
* Turn the warnings produced when finding non-executed tests that
are not in the allow list into errors.
Signed-off-by: Tomás González <tomasagustin.gonzalezorlando@arm.com>
Do explain why we don't test a smaller buffer in addition to testing the
nominal size and a larger buffer.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Some basic test coverage for now:
* Nominal operation.
* Larger output buffer.
* Clone an operation and use it after the original operation stops.
Generate test data automatically. For the time being, only do that for
hashes that Python supports natively. Supporting all algorithms is future
work.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Since 3.0.0, mbedtls_config.h (formerly config.h) no longer needs to include
config_psa.h or check_config.h: build_info.h takes care of that.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
"Fix PBKDF2 with empty salt on platforms where malloc(0)=NULL" took care of
making an empty salt work. But it didn't fix the case of an empty salt
segment followed by a non-empty salt segment, which still invoked memcpy
with a potentially null pointer as the source. This commit fixes that case,
and also simplifies the logic in the function a little.
Test data obtained with:
```
pip3 install cryptodome
python3 -c 'import sys; from Crypto.Hash import SHA256; from Crypto.Protocol.KDF import PBKDF2; cost = int(sys.argv[1], 0); salt = bytes.fromhex(sys.argv[2]); password = bytes.fromhex(sys.argv[3]); n = int(sys.argv[4], 0); print(PBKDF2(password=password, salt=salt, dkLen=n, count=cost, hmac_hash_module=SHA256).hex())' 1 "" "706173737764" 64
```
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
The hardware module name otherName SAN contains 2 OIDs:
OtherName ::= SEQUENCE {
type-id OBJECT IDENTIFIER,
value [0] EXPLICIT ANY DEFINED BY type-id }
HardwareModuleName ::= SEQUENCE {
hwType OBJECT IDENTIFIER,
hwSerialNum OCTET STRING }
The first, type-id, is the one that identifies the otherName as a
HardwareModuleName. The second, hwType, identifies the type of hardware.
This change fixes 2 issues:
1. We were erroneously trying to identify HardwareModuleNames by looking
at hwType, not type-id.
2. We accidentally inverted the check so that we were checking that
hwType did NOT match HardwareModuleName.
This fix ensures that type-id is correctly checked to make sure that it
matches the OID for HardwareModuleName.
Signed-off-by: David Horstmann <david.horstmann@arm.com>
