From c32f0304db583e166a71370e744251b8d7445cad Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 10 Aug 2018 16:02:11 +0200 Subject: [PATCH 01/14] Fix bad key type constant that worked by accident --- tests/suites/test_suite_psa_crypto.function | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index 04c1c7982c..310df38f44 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -1128,7 +1128,7 @@ exit: void key_lifetime( int lifetime_arg ) { int key_slot = 1; - psa_key_type_t key_type = PSA_ALG_CBC_BASE; + psa_key_type_t key_type = PSA_KEY_TYPE_RAW_DATA; unsigned char key[32] = {0}; psa_key_lifetime_t lifetime_set = lifetime_arg; psa_key_lifetime_t lifetime_get; From 78b3bb670da616b206fb1d9be1a28674deea95ff Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 10 Aug 2018 16:03:41 +0200 Subject: [PATCH 02/14] Change the bitwise encoding of key type categories There were only 5 categories (now 4). Reduce the category mask from 7 bits to 3. Combine unformatted, not-necessarily-uniform keys (HMAC, derivation) with raw data. Reintroduce a KEY_TYPE_IS_UNSTRUCTURED macro (which used to exist under the name KEY_TYPE_IS_RAW_DATA macro) for key types that don't have any structure, including both should-be-uniform keys (such as block cipher and stream cipher keys) and not-necessarily-uniform keys (such as HMAC keys and secrets for key derivation). --- include/psa/crypto.h | 82 +++++++++++++++++++++++++++++--------------- library/psa_crypto.c | 4 +-- 2 files changed, 56 insertions(+), 30 deletions(-) diff --git a/include/psa/crypto.h b/include/psa/crypto.h index 6d31322834..55c0c04132 100644 --- a/include/psa/crypto.h +++ b/include/psa/crypto.h @@ -360,17 +360,19 @@ typedef uint32_t psa_key_type_t; */ #define PSA_KEY_TYPE_VENDOR_FLAG ((psa_key_type_t)0x80000000) -#define PSA_KEY_TYPE_CATEGORY_MASK ((psa_key_type_t)0x7e000000) +#define PSA_KEY_TYPE_CATEGORY_MASK ((psa_key_type_t)0x70000000) +#define PSA_KEY_TYPE_CATEGORY_SYMMETRIC ((psa_key_type_t)0x40000000) +#define PSA_KEY_TYPE_CATEGORY_RAW ((psa_key_type_t)0x50000000) +#define PSA_KEY_TYPE_CATEGORY_PUBLIC_KEY ((psa_key_type_t)0x60000000) +#define PSA_KEY_TYPE_CATEGORY_KEY_PAIR ((psa_key_type_t)0x70000000) + +#define PSA_KEY_TYPE_CATEGORY_FLAG_PAIR ((psa_key_type_t)0x10000000) /** Raw data. * * A "key" of this type cannot be used for any cryptographic operation. * Applications may use this type to store arbitrary data in the keystore. */ -#define PSA_KEY_TYPE_RAW_DATA ((psa_key_type_t)0x02000000) - -#define PSA_KEY_TYPE_CATEGORY_SYMMETRIC ((psa_key_type_t)0x04000000) -#define PSA_KEY_TYPE_CATEGORY_ASYMMETRIC ((psa_key_type_t)0x06000000) -#define PSA_KEY_TYPE_PAIR_FLAG ((psa_key_type_t)0x01000000) +#define PSA_KEY_TYPE_RAW_DATA ((psa_key_type_t)0x50000001) /** HMAC key. * @@ -380,21 +382,21 @@ typedef uint32_t psa_key_type_t; * HMAC keys should generally have the same size as the underlying hash. * This size can be calculated with #PSA_HASH_SIZE(\c alg) where * \c alg is the HMAC algorithm or the underlying hash algorithm. */ -#define PSA_KEY_TYPE_HMAC ((psa_key_type_t)0x02000001) +#define PSA_KEY_TYPE_HMAC ((psa_key_type_t)0x51000000) /** A secret for key derivation. * * The key policy determines which key derivation algorithm the key * can be used for. */ -#define PSA_KEY_TYPE_DERIVE ((psa_key_type_t)0x02000101) +#define PSA_KEY_TYPE_DERIVE ((psa_key_type_t)0x52000000) /** Key for an cipher, AEAD or MAC algorithm based on the AES block cipher. * * The size of the key can be 16 bytes (AES-128), 24 bytes (AES-192) or * 32 bytes (AES-256). */ -#define PSA_KEY_TYPE_AES ((psa_key_type_t)0x04000001) +#define PSA_KEY_TYPE_AES ((psa_key_type_t)0x40000001) /** Key for a cipher or MAC algorithm based on DES or 3DES (Triple-DES). * @@ -405,30 +407,30 @@ typedef uint32_t psa_key_type_t; * deprecated and should only be used to decrypt legacy data. 3-key 3DES * is weak and deprecated and should only be used in legacy protocols. */ -#define PSA_KEY_TYPE_DES ((psa_key_type_t)0x04000002) +#define PSA_KEY_TYPE_DES ((psa_key_type_t)0x40000002) /** Key for an cipher, AEAD or MAC algorithm based on the * Camellia block cipher. */ -#define PSA_KEY_TYPE_CAMELLIA ((psa_key_type_t)0x04000003) +#define PSA_KEY_TYPE_CAMELLIA ((psa_key_type_t)0x40000003) /** Key for the RC4 stream cipher. * * Note that RC4 is weak and deprecated and should only be used in * legacy protocols. */ -#define PSA_KEY_TYPE_ARC4 ((psa_key_type_t)0x04000004) +#define PSA_KEY_TYPE_ARC4 ((psa_key_type_t)0x40000004) /** RSA public key. */ -#define PSA_KEY_TYPE_RSA_PUBLIC_KEY ((psa_key_type_t)0x06010000) +#define PSA_KEY_TYPE_RSA_PUBLIC_KEY ((psa_key_type_t)0x60010000) /** RSA key pair (private and public key). */ -#define PSA_KEY_TYPE_RSA_KEYPAIR ((psa_key_type_t)0x07010000) +#define PSA_KEY_TYPE_RSA_KEYPAIR ((psa_key_type_t)0x70010000) /** DSA public key. */ -#define PSA_KEY_TYPE_DSA_PUBLIC_KEY ((psa_key_type_t)0x06020000) +#define PSA_KEY_TYPE_DSA_PUBLIC_KEY ((psa_key_type_t)0x60020000) /** DSA key pair (private and public key). */ -#define PSA_KEY_TYPE_DSA_KEYPAIR ((psa_key_type_t)0x07020000) +#define PSA_KEY_TYPE_DSA_KEYPAIR ((psa_key_type_t)0x70020000) -#define PSA_KEY_TYPE_ECC_PUBLIC_KEY_BASE ((psa_key_type_t)0x06030000) -#define PSA_KEY_TYPE_ECC_KEYPAIR_BASE ((psa_key_type_t)0x07030000) +#define PSA_KEY_TYPE_ECC_PUBLIC_KEY_BASE ((psa_key_type_t)0x60030000) +#define PSA_KEY_TYPE_ECC_KEYPAIR_BASE ((psa_key_type_t)0x70030000) #define PSA_KEY_TYPE_ECC_CURVE_MASK ((psa_key_type_t)0x0000ffff) /** Elliptic curve key pair. */ #define PSA_KEY_TYPE_ECC_KEYPAIR(curve) \ @@ -441,24 +443,50 @@ typedef uint32_t psa_key_type_t; #define PSA_KEY_TYPE_IS_VENDOR_DEFINED(type) \ (((type) & PSA_KEY_TYPE_VENDOR_FLAG) != 0) +/** Whether a key type is an unstructured array of bytes. + * + * This encompasses both symmetric keys and non-key data. + */ +#define PSA_KEY_TYPE_IS_UNSTRUCTURED(type) \ + (((type) & PSA_KEY_TYPE_CATEGORY_MASK & ~(psa_key_type_t)0x10000000) == \ + PSA_KEY_TYPE_CATEGORY_SYMMETRIC) + /** Whether a key type is asymmetric: either a key pair or a public key. */ #define PSA_KEY_TYPE_IS_ASYMMETRIC(type) \ - (((type) & PSA_KEY_TYPE_CATEGORY_MASK) == PSA_KEY_TYPE_CATEGORY_ASYMMETRIC) + (((type) & PSA_KEY_TYPE_CATEGORY_MASK \ + & ~PSA_KEY_TYPE_CATEGORY_FLAG_PAIR) == \ + PSA_KEY_TYPE_CATEGORY_PUBLIC_KEY) /** Whether a key type is the public part of a key pair. */ #define PSA_KEY_TYPE_IS_PUBLIC_KEY(type) \ - (((type) & (PSA_KEY_TYPE_CATEGORY_MASK | PSA_KEY_TYPE_PAIR_FLAG)) == \ - PSA_KEY_TYPE_CATEGORY_ASYMMETRIC) + (((type) & PSA_KEY_TYPE_CATEGORY_MASK) == PSA_KEY_TYPE_CATEGORY_PUBLIC_KEY) /** Whether a key type is a key pair containing a private part and a public * part. */ #define PSA_KEY_TYPE_IS_KEYPAIR(type) \ - (((type) & (PSA_KEY_TYPE_CATEGORY_MASK | PSA_KEY_TYPE_PAIR_FLAG)) == \ - (PSA_KEY_TYPE_CATEGORY_ASYMMETRIC | PSA_KEY_TYPE_PAIR_FLAG)) -/** The key pair type corresponding to a public key type. */ + (((type) & PSA_KEY_TYPE_CATEGORY_MASK) == PSA_KEY_TYPE_CATEGORY_KEY_PAIR) +/** The key pair type corresponding to a public key type. + * + * You may also pass a key pair type as \p type, it will be left unchanged. + * + * \param type A public key type or key pair type. + * + * \return The corresponding key pair type. + * If \p type is not a public key or a key pair, + * the return value is undefined. + */ #define PSA_KEY_TYPE_KEYPAIR_OF_PUBLIC_KEY(type) \ - ((type) | PSA_KEY_TYPE_PAIR_FLAG) -/** The public key type corresponding to a key pair type. */ + ((type) | PSA_KEY_TYPE_CATEGORY_FLAG_PAIR) +/** The public key type corresponding to a key pair type. + * + * You may also pass a key pair type as \p type, it will be left unchanged. + * + * \param type A public key type or key pair type. + * + * \return The corresponding public key type. + * If \p type is not a public key or a key pair, + * the return value is undefined. + */ #define PSA_KEY_TYPE_PUBLIC_KEY_OF_KEYPAIR(type) \ - ((type) & ~PSA_KEY_TYPE_PAIR_FLAG) + ((type) & ~PSA_KEY_TYPE_CATEGORY_FLAG_PAIR) /** Whether a key type is an RSA key (pair or public-only). */ #define PSA_KEY_TYPE_IS_RSA(type) \ (PSA_KEY_TYPE_PUBLIC_KEY_OF_KEYPAIR(type) == PSA_KEY_TYPE_RSA_PUBLIC_KEY) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 316acbe64e..dfbb6800f5 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -135,9 +135,7 @@ typedef struct static int key_type_is_raw_bytes( psa_key_type_t type ) { - psa_key_type_t category = type & PSA_KEY_TYPE_CATEGORY_MASK; - return( category == PSA_KEY_TYPE_RAW_DATA || - category == PSA_KEY_TYPE_CATEGORY_SYMMETRIC ); + return( PSA_KEY_TYPE_IS_UNSTRUCTURED( type ) ); } typedef struct From e8779747947f0fd3ae5187d3cbe746b77ddd33f7 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 10 Aug 2018 16:10:56 +0200 Subject: [PATCH 03/14] Move key type feature test macros to a more logical place --- include/psa/crypto.h | 104 +++++++++++++++++++++---------------------- 1 file changed, 52 insertions(+), 52 deletions(-) diff --git a/include/psa/crypto.h b/include/psa/crypto.h index 55c0c04132..8a76a2151e 100644 --- a/include/psa/crypto.h +++ b/include/psa/crypto.h @@ -368,6 +368,58 @@ typedef uint32_t psa_key_type_t; #define PSA_KEY_TYPE_CATEGORY_FLAG_PAIR ((psa_key_type_t)0x10000000) +/** Whether a key type is vendor-defined. */ +#define PSA_KEY_TYPE_IS_VENDOR_DEFINED(type) \ + (((type) & PSA_KEY_TYPE_VENDOR_FLAG) != 0) + +/** Whether a key type is an unstructured array of bytes. + * + * This encompasses both symmetric keys and non-key data. + */ +#define PSA_KEY_TYPE_IS_UNSTRUCTURED(type) \ + (((type) & PSA_KEY_TYPE_CATEGORY_MASK & ~(psa_key_type_t)0x10000000) == \ + PSA_KEY_TYPE_CATEGORY_SYMMETRIC) + +/** Whether a key type is asymmetric: either a key pair or a public key. */ +#define PSA_KEY_TYPE_IS_ASYMMETRIC(type) \ + (((type) & PSA_KEY_TYPE_CATEGORY_MASK \ + & ~PSA_KEY_TYPE_CATEGORY_FLAG_PAIR) == \ + PSA_KEY_TYPE_CATEGORY_PUBLIC_KEY) +/** Whether a key type is the public part of a key pair. */ +#define PSA_KEY_TYPE_IS_PUBLIC_KEY(type) \ + (((type) & PSA_KEY_TYPE_CATEGORY_MASK) == PSA_KEY_TYPE_CATEGORY_PUBLIC_KEY) +/** Whether a key type is a key pair containing a private part and a public + * part. */ +#define PSA_KEY_TYPE_IS_KEYPAIR(type) \ + (((type) & PSA_KEY_TYPE_CATEGORY_MASK) == PSA_KEY_TYPE_CATEGORY_KEY_PAIR) +/** The key pair type corresponding to a public key type. + * + * You may also pass a key pair type as \p type, it will be left unchanged. + * + * \param type A public key type or key pair type. + * + * \return The corresponding key pair type. + * If \p type is not a public key or a key pair, + * the return value is undefined. + */ +#define PSA_KEY_TYPE_KEYPAIR_OF_PUBLIC_KEY(type) \ + ((type) | PSA_KEY_TYPE_CATEGORY_FLAG_PAIR) +/** The public key type corresponding to a key pair type. + * + * You may also pass a key pair type as \p type, it will be left unchanged. + * + * \param type A public key type or key pair type. + * + * \return The corresponding public key type. + * If \p type is not a public key or a key pair, + * the return value is undefined. + */ +#define PSA_KEY_TYPE_PUBLIC_KEY_OF_KEYPAIR(type) \ + ((type) & ~PSA_KEY_TYPE_CATEGORY_FLAG_PAIR) +/** Whether a key type is an RSA key (pair or public-only). */ +#define PSA_KEY_TYPE_IS_RSA(type) \ + (PSA_KEY_TYPE_PUBLIC_KEY_OF_KEYPAIR(type) == PSA_KEY_TYPE_RSA_PUBLIC_KEY) + /** Raw data. * * A "key" of this type cannot be used for any cryptographic operation. @@ -439,58 +491,6 @@ typedef uint32_t psa_key_type_t; #define PSA_KEY_TYPE_ECC_PUBLIC_KEY(curve) \ (PSA_KEY_TYPE_ECC_PUBLIC_KEY_BASE | (curve)) -/** Whether a key type is vendor-defined. */ -#define PSA_KEY_TYPE_IS_VENDOR_DEFINED(type) \ - (((type) & PSA_KEY_TYPE_VENDOR_FLAG) != 0) - -/** Whether a key type is an unstructured array of bytes. - * - * This encompasses both symmetric keys and non-key data. - */ -#define PSA_KEY_TYPE_IS_UNSTRUCTURED(type) \ - (((type) & PSA_KEY_TYPE_CATEGORY_MASK & ~(psa_key_type_t)0x10000000) == \ - PSA_KEY_TYPE_CATEGORY_SYMMETRIC) - -/** Whether a key type is asymmetric: either a key pair or a public key. */ -#define PSA_KEY_TYPE_IS_ASYMMETRIC(type) \ - (((type) & PSA_KEY_TYPE_CATEGORY_MASK \ - & ~PSA_KEY_TYPE_CATEGORY_FLAG_PAIR) == \ - PSA_KEY_TYPE_CATEGORY_PUBLIC_KEY) -/** Whether a key type is the public part of a key pair. */ -#define PSA_KEY_TYPE_IS_PUBLIC_KEY(type) \ - (((type) & PSA_KEY_TYPE_CATEGORY_MASK) == PSA_KEY_TYPE_CATEGORY_PUBLIC_KEY) -/** Whether a key type is a key pair containing a private part and a public - * part. */ -#define PSA_KEY_TYPE_IS_KEYPAIR(type) \ - (((type) & PSA_KEY_TYPE_CATEGORY_MASK) == PSA_KEY_TYPE_CATEGORY_KEY_PAIR) -/** The key pair type corresponding to a public key type. - * - * You may also pass a key pair type as \p type, it will be left unchanged. - * - * \param type A public key type or key pair type. - * - * \return The corresponding key pair type. - * If \p type is not a public key or a key pair, - * the return value is undefined. - */ -#define PSA_KEY_TYPE_KEYPAIR_OF_PUBLIC_KEY(type) \ - ((type) | PSA_KEY_TYPE_CATEGORY_FLAG_PAIR) -/** The public key type corresponding to a key pair type. - * - * You may also pass a key pair type as \p type, it will be left unchanged. - * - * \param type A public key type or key pair type. - * - * \return The corresponding public key type. - * If \p type is not a public key or a key pair, - * the return value is undefined. - */ -#define PSA_KEY_TYPE_PUBLIC_KEY_OF_KEYPAIR(type) \ - ((type) & ~PSA_KEY_TYPE_CATEGORY_FLAG_PAIR) -/** Whether a key type is an RSA key (pair or public-only). */ -#define PSA_KEY_TYPE_IS_RSA(type) \ - (PSA_KEY_TYPE_PUBLIC_KEY_OF_KEYPAIR(type) == PSA_KEY_TYPE_RSA_PUBLIC_KEY) - /** Whether a key type is an elliptic curve key (pair or public-only). */ #define PSA_KEY_TYPE_IS_ECC(type) \ ((PSA_KEY_TYPE_PUBLIC_KEY_OF_KEYPAIR(type) & \ From 4e1e9beb56f711b01812b50a9c3282445d9ac578 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 10 Aug 2018 18:57:40 +0200 Subject: [PATCH 04/14] Define the encoding of ECC and DSA keys --- include/psa/crypto.h | 114 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 106 insertions(+), 8 deletions(-) diff --git a/include/psa/crypto.h b/include/psa/crypto.h index 8a76a2151e..837f737c31 100644 --- a/include/psa/crypto.h +++ b/include/psa/crypto.h @@ -1187,10 +1187,50 @@ psa_status_t psa_get_key_information(psa_key_slot_t key, * - For Triple-DES, the format is the concatenation of the * two or three DES keys. * - For RSA key pairs (#PSA_KEY_TYPE_RSA_KEYPAIR), the format - * is the non-encrypted DER representation defined by PKCS\#1 (RFC 8017) - * as RSAPrivateKey. - * - For RSA public keys (#PSA_KEY_TYPE_RSA_PUBLIC_KEY), the format - * is the DER representation defined by RFC 5280 as SubjectPublicKeyInfo. + * is the non-encrypted DER encoding of the representation defined by + * PKCS\#1 (RFC 8017) as `RSAPrivateKey`, version 0. + * ``` + * RSAPrivateKey ::= SEQUENCE { + * version Version, -- 0 + * modulus INTEGER, -- n + * publicExponent INTEGER, -- e + * privateExponent INTEGER, -- d + * prime1 INTEGER, -- p + * prime2 INTEGER, -- q + * exponent1 INTEGER, -- d mod (p-1) + * exponent2 INTEGER, -- d mod (q-1) + * coefficient INTEGER, -- (inverse of q) mod p + * } + * ``` + * - For DSA private keys (#PSA_KEY_TYPE_DSA_KEYPAIR), the format + * is the non-encrypted DER encoding of the representation used by + * OpenSSL and OpenSSH, which the following ASN.1 structure: + * ``` + * DSAPrivateKey ::= SEQUENCE { + * version Version, -- 0 + * prime INTEGER, -- p + * subprime INTEGER, -- q + * generator INTEGER, -- g + * public INTEGER, -- y + * private INTEGER, -- x + * } + * ``` + * - For elliptic curve key pairs (key types for which + * #PSA_KEY_TYPE_IS_ECC_KEYPAIR is true), the format is the + * non-encrypted DER encoding of the representation defined by RFC 5915 as + * `ECPrivateKey`, version 1. + * ``` + * ECPrivateKey ::= SEQUENCE { + * version INTEGER, -- must be 1 + * privateKey OCTET STRING, + * -- `ceiling(log_{256}(n))`-byte string, big endian, + * -- where n is the order of the curve. + * parameters ECParameters {{ NamedCurve }}, -- mandatory + * publicKey BIT STRING -- mandatory + * } + * ``` + * - For public keys (key types for which #PSA_KEY_TYPE_IS_PUBLIC_KEY is + * true), the format is the same as for psa_export_public_key(). * * \param key Slot whose content is to be exported. This must * be an occupied key slot. @@ -1218,11 +1258,69 @@ psa_status_t psa_export_key(psa_key_slot_t key, * The output of this function can be passed to psa_import_key() to * create an object that is equivalent to the public key. * - * For standard key types, the output format is as follows: + * The format is the DER representation defined by RFC 5280 as + * `SubjectPublicKeyInfo`, with the `subjectPublicKey` format + * specified below. + * ``` + * SubjectPublicKeyInfo ::= SEQUENCE { + * algorithm AlgorithmIdentifier, + * subjectPublicKey BIT STRING } + * AlgorithmIdentifier ::= SEQUENCE { + * algorithm OBJECT IDENTIFIER, + * parameters ANY DEFINED BY algorithm OPTIONAL } + * ``` * - * - For RSA keys (#PSA_KEY_TYPE_RSA_KEYPAIR or #PSA_KEY_TYPE_RSA_PUBLIC_KEY), - * the format is the DER representation of the public key defined by RFC 5280 - * as SubjectPublicKeyInfo. + * - For RSA public keys (#PSA_KEY_TYPE_RSA_PUBLIC_KEY), + * the `subjectPublicKey` format is defined by RFC 3279 §2.3.1 as + * `RSAPublicKey`, + * with the OID `rsaEncryption`, + * and with the parameters `NULL`. + * ``` + * pkcs-1 OBJECT IDENTIFIER ::= { iso(1) member-body(2) us(840) + * rsadsi(113549) pkcs(1) 1 } + * rsaEncryption OBJECT IDENTIFIER ::= { pkcs-1 1 } + * + * RSAPublicKey ::= SEQUENCE { + * modulus INTEGER, -- n + * publicExponent INTEGER } -- e + * ``` + * - For DSA public keys (#PSA_KEY_TYPE_DSA_PUBLIC_KEY), + * the `subjectPublicKey` format is defined by RFC 3279 §2.3.2 as + * `DSAPublicKey`, + * with the OID `id-dsa`, + * and with the parameters `DSS-Parms`. + * ``` + * id-dsa OBJECT IDENTIFIER ::= { + * iso(1) member-body(2) us(840) x9-57(10040) x9cm(4) 1 } + * + * Dss-Parms ::= SEQUENCE { + * p INTEGER, + * q INTEGER, + * g INTEGER } + * DSAPublicKey ::= INTEGER -- public key, Y + * ``` + * - For elliptic curve public keys (key types for which + * #PSA_KEY_TYPE_IS_ECC_PUBLIC_KEY is true), + * the `subjectPublicKey` format is defined by RFC 3279 §2.3.5 as + * `ECPoint`, which is an OCTET STRING containing the uncompressed + * representation defined by SEC1 §2.3.3. + * The OID is `id-ecPublicKey`, + * and the parameters must be given as a `namedCurve`. + * ``` + * ansi-X9-62 OBJECT IDENTIFIER ::= + * { iso(1) member-body(2) us(840) 10045 } + * id-public-key-type OBJECT IDENTIFIER ::= { ansi-X9.62 2 } + * id-ecPublicKey OBJECT IDENTIFIER ::= { id-publicKeyType 1 } + * + * ECPoint ::= OCTET STRING + * -- first byte: 0x04; + * -- then x_P as a `ceiling(log_{256}(n))`-byte string, big endian; + * -- then y_P as a `ceiling(log_{256}(n))`-byte string, big endian, + * -- where n is the order of the curve. + * + * EcpkParameters ::= CHOICE { -- other choices are not allowed + * namedCurve OBJECT IDENTIFIER } + * ``` * * \param key Slot whose content is to be exported. This must * be an occupied key slot. From 1be949b8461474f113c6fe2ef249cec8be6455d8 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 10 Aug 2018 19:06:59 +0200 Subject: [PATCH 05/14] New macro PSA_KEY_EXPORT_MAX_SIZE Sufficient buffer size for psa_export_key() and psa_export_public_key(). --- include/psa/crypto.h | 14 +++ include/psa/crypto_sizes.h | 209 +++++++++++++++++++++++++++++++++++++ 2 files changed, 223 insertions(+) diff --git a/include/psa/crypto.h b/include/psa/crypto.h index 837f737c31..87f2d60b70 100644 --- a/include/psa/crypto.h +++ b/include/psa/crypto.h @@ -1243,6 +1243,12 @@ psa_status_t psa_get_key_information(psa_key_slot_t key, * \retval #PSA_ERROR_EMPTY_SLOT * \retval #PSA_ERROR_NOT_PERMITTED * \retval #PSA_ERROR_NOT_SUPPORTED + * \retval #PSA_ERROR_BUFFER_TOO_SMALL + * The size of the \p data buffer is too small. You can determine a + * sufficient buffer size by calling + * #PSA_KEY_EXPORT_MAX_SIZE(\c type, \c bits) + * where \c type is the key type + * and \c bits is the key size in bits. * \retval #PSA_ERROR_COMMUNICATION_FAILURE * \retval #PSA_ERROR_HARDWARE_FAILURE * \retval #PSA_ERROR_TAMPERING_DETECTED @@ -1332,6 +1338,14 @@ psa_status_t psa_export_key(psa_key_slot_t key, * \retval #PSA_SUCCESS * \retval #PSA_ERROR_EMPTY_SLOT * \retval #PSA_ERROR_INVALID_ARGUMENT + * The key is neither a public key nor a key pair. + * \retval #PSA_ERROR_NOT_SUPPORTED + * \retval #PSA_ERROR_BUFFER_TOO_SMALL + * The size of the \p data buffer is too small. You can determine a + * sufficient buffer size by calling + * #PSA_KEY_EXPORT_MAX_SIZE(#PSA_KEY_TYPE_PUBLIC_KEY_OF_KEYPAIR(\c type), \c bits) + * where \c type is the key type + * and \c bits is the key size in bits. * \retval #PSA_ERROR_COMMUNICATION_FAILURE * \retval #PSA_ERROR_HARDWARE_FAILURE * \retval #PSA_ERROR_TAMPERING_DETECTED diff --git a/include/psa/crypto_sizes.h b/include/psa/crypto_sizes.h index ab5b17e194..bc8edc6129 100644 --- a/include/psa/crypto_sizes.h +++ b/include/psa/crypto_sizes.h @@ -305,4 +305,213 @@ PSA_BITS_TO_BYTES(key_bits) - PSA_RSA_MINIMUM_PADDING_SIZE(alg) : \ 0) +/* Maximum size of the ASN.1 encoding of an INTEGER with the specified + * number of bits. + * + * This definition assumes that bits <= 2^19 - 9 so that the length field + * is at most 3 bytes. The length of the encoding is the length of the + * bit string padded to a whole number of bytes plus: + * - 1 type byte; + * - 1 to 3 length bytes; + * - 0 to 1 bytes of leading 0 due to the sign bit. + */ +#define PSA_KEY_EXPORT_ASN1_INTEGER_MAX_SIZE(bits) \ + ((bits) / 8 + 5) + +/* Maximum size of the export encoding of an RSA public key. + * Assumes that the public exponent is less than 2^32. + * + * SubjectPublicKeyInfo ::= SEQUENCE { + * algorithm AlgorithmIdentifier, + * subjectPublicKey BIT STRING } -- contains RSAPublicKey + * AlgorithmIdentifier ::= SEQUENCE { + * algorithm OBJECT IDENTIFIER, + * parameters NULL } + * RSAPublicKey ::= SEQUENCE { + * modulus INTEGER, -- n + * publicExponent INTEGER } -- e + * + * - 3 * 4 bytes of SEQUENCE overhead; + * - 1 + 1 + 9 bytes of algorithm (RSA OID); + * - 2 bytes of NULL; + * - 4 bytes of BIT STRING overhead; + * - n : INTEGER; + * - 7 bytes for the public exponent. + */ +#define PSA_KEY_EXPORT_RSA_PUBLIC_KEY_MAX_SIZE(key_bits) \ + (PSA_KEY_EXPORT_ASN1_INTEGER_MAX_SIZE(key_bits) + 36) + +/* Maximum size of the export encoding of an RSA key pair. + * Assumes thatthe public exponent is less than 2^32 and that the size + * difference between the two primes is at most 1 bit. + * + * RSAPrivateKey ::= SEQUENCE { + * version Version, -- 0 + * modulus INTEGER, -- N-bit + * publicExponent INTEGER, -- 32-bit + * privateExponent INTEGER, -- N-bit + * prime1 INTEGER, -- N/2-bit + * prime2 INTEGER, -- N/2-bit + * exponent1 INTEGER, -- N/2-bit + * exponent2 INTEGER, -- N/2-bit + * coefficient INTEGER, -- N/2-bit + * } + * + * - 4 bytes of SEQUENCE overhead; + * - 3 bytes of version; + * - 7 half-size INTEGERs plus 2 full-size INTEGERs, + * overapproximated as 9 half-size INTEGERS; + * - 7 bytes for the public exponent. + */ +#define PSA_KEY_EXPORT_RSA_KEYPAIR_MAX_SIZE(key_bits) \ + (9 * PSA_KEY_EXPORT_ASN1_INTEGER_MAX_SIZE((key_bits) / 2 + 1) + 14) + +/* Maximum size of the export encoding of a DSA public key. + * + * SubjectPublicKeyInfo ::= SEQUENCE { + * algorithm AlgorithmIdentifier, + * subjectPublicKey BIT STRING } -- contains DSAPublicKey + * AlgorithmIdentifier ::= SEQUENCE { + * algorithm OBJECT IDENTIFIER, + * parameters Dss-Parms } -- SEQUENCE of 3 INTEGERs + * DSAPublicKey ::= INTEGER -- public key, Y + * + * - 3 * 4 bytes of SEQUENCE overhead; + * - 1 + 1 + 7 bytes of algorithm (DSA OID); + * - 4 bytes of BIT STRING overhead; + * - 3 full-size INTEGERs (p, g, y); + * - 1 + 1 + 32 bytes for 1 sub-size INTEGER (q <= 256 bits). + */ +#define PSA_KEY_EXPORT_DSA_PUBLIC_KEY_MAX_SIZE(key_bits) \ + (PSA_KEY_EXPORT_ASN1_INTEGER_MAX_SIZE(key_bits) * 3 + 59) + +/* Maximum size of the export encoding of a DSA key pair. + * + * DSAPrivateKey ::= SEQUENCE { + * version Version, -- 0 + * prime INTEGER, -- p + * subprime INTEGER, -- q + * generator INTEGER, -- g + * public INTEGER, -- y + * private INTEGER, -- x + * } + * + * - 4 bytes of SEQUENCE overhead; + * - 3 bytes of version; + * - 3 full-size INTEGERs (p, g, y); + * - 2 * (1 + 1 + 32) bytes for 2 sub-size INTEGERs (q, x <= 256 bits). + */ +#define PSA_KEY_EXPORT_DSA_KEYPAIR_MAX_SIZE(key_bits) \ + (PSA_KEY_EXPORT_ASN1_INTEGER_MAX_SIZE(key_bits) * 3 + 75) + +/* Maximum size of the export encoding of an ECC public key. + * + * SubjectPublicKeyInfo ::= SEQUENCE { + * algorithm AlgorithmIdentifier, + * subjectPublicKey BIT STRING } -- contains ECPoint + * AlgorithmIdentifier ::= SEQUENCE { + * algorithm OBJECT IDENTIFIER, + * parameters OBJECT IDENTIFIER } -- namedCurve + * ECPoint ::= OCTET STRING + * -- first byte: 0x04; + * -- then x_P as a `ceiling(log_{256}(n))`-byte string, big endian; + * -- then y_P as a `ceiling(log_{256}(n))`-byte string, big endian, + * -- where n is the order of the curve. + * + * - 2 * 4 bytes of SEQUENCE overhead; + * - 1 + 1 + 7 bytes of algorithm (id-ecPublicKey OID); + * - 1 + 1 + 12 bytes of namedCurve OID; + * - 4 bytes of BIT STRING overhead; + * - 1 byte + 2 * point size in ECPoint. + */ +#define PSA_KEY_EXPORT_ECC_PUBLIC_KEY_MAX_SIZE(key_bits) \ + (2 * PSA_BITS_TO_BYTES(key_bits) + 36) + +/* Maximum size of the export encoding of an ECC key pair. + * + * ECPrivateKey ::= SEQUENCE { + * version INTEGER, -- must be 1 + * privateKey OCTET STRING, + * -- `ceiling(log_{256}(n))`-byte string, big endian, + * -- where n is the order of the curve. + * parameters ECParameters {{ NamedCurve }}, -- mandatory + * publicKey BIT STRING -- mandatory + * } + * + * - 4 bytes of SEQUENCE overhead; + * - 1 * point size in privateKey + * - 1 + 1 + 12 bytes of namedCurve OID; + * - 4 bytes of BIT STRING overhead; + * - public key as for #PSA_KEY_EXPORT_ECC_PUBLIC_KEY_MAX_SIZE. + */ +#define PSA_KEY_EXPORT_ECC_KEYPAIR_MAX_SIZE(key_bits) \ + (3 * PSA_BITS_TO_BYTES(key_bits) + 56) + +/** Safe output buffer size for psa_export_key() or psa_export_public_key(). + * + * This macro returns a compile-time constant if its arguments are + * compile-time constants. + * + * \warning This function may call its arguments multiple times or + * zero times, so you should not pass arguments that contain + * side effects. + * + * The following code illustrates how to allocate enough memory to export + * a key by querying the key type and size at runtime. + * \code{c} + * psa_key_type_t key_type; + * size_t key_bits; + * psa_status_t status; + * status = psa_get_key_information(key, &key_type, &key_bits); + * if (status != PSA_SUCCESS) handle_error(...); + * size_t buffer_size = PSA_KEY_EXPORT_MAX_SIZE(key_type, key_bits); + * unsigned char *buffer = malloc(buffer_size); + * if (buffer != NULL) handle_error(...); + * size_t buffer_length; + * status = psa_export_key(key, buffer, buffer_size, &buffer_length); + * if (status != PSA_SUCCESS) handle_error(...); + * \endcode + * + * For psa_export_public_key(), calculate the buffer size from the + * public key type. You can use the macro #PSA_KEY_TYPE_PUBLIC_KEY_OF_KEYPAIR + * to convert a key pair type to the corresponding public key type. + * \code{c} + * psa_key_type_t key_type; + * size_t key_bits; + * psa_status_t status; + * status = psa_get_key_information(key, &key_type, &key_bits); + * if (status != PSA_SUCCESS) handle_error(...); + * psa_key_type_t public_key_type = PSA_KEY_TYPE_PUBLIC_KEY_OF_KEYPAIR(key_type); + * size_t buffer_size = PSA_KEY_EXPORT_MAX_SIZE(public_key_type, key_bits); + * unsigned char *buffer = malloc(buffer_size); + * if (buffer != NULL) handle_error(...); + * size_t buffer_length; + * status = psa_export_public_key(key, buffer, buffer_size, &buffer_length); + * if (status != PSA_SUCCESS) handle_error(...); + * \endcode + * + * \param key_type A supported key type. + * \param key_bits The size of the key in bits. + * \param alg The signature algorithm. + * + * \return If the parameters are valid and supported, return + * a buffer size in bytes that guarantees that + * psa_asymmetric_sign() will not fail with + * #PSA_ERROR_BUFFER_TOO_SMALL. + * If the parameters are a valid combination that is not supported + * by the implementation, this macro either shall return either a + * sensible size or 0. + * If the parameters are not valid, the + * return value is unspecified. + */ +#define PSA_KEY_EXPORT_MAX_SIZE(key_type, key_bits) \ + (PSA_KEY_TYPE_IS_UNSTRUCTURED(key_type) ? PSA_BITS_TO_BYTES(key_bits) : \ + (key_type) == PSA_KEY_TYPE_RSA_KEYPAIR ? PSA_KEY_EXPORT_RSA_KEYPAIR_MAX_SIZE(key_bits) : \ + (key_type) == PSA_KEY_TYPE_RSA_PUBLIC_KEY ? PSA_KEY_EXPORT_RSA_PUBLIC_KEY_MAX_SIZE(key_bits) : \ + (key_type) == PSA_KEY_TYPE_DSA_KEYPAIR ? PSA_KEY_EXPORT_DSA_KEYPAIR_MAX_SIZE(key_bits) : \ + (key_type) == PSA_KEY_TYPE_DSA_PUBLIC_KEY ? PSA_KEY_EXPORT_DSA_PUBLIC_KEY_MAX_SIZE(key_bits) : \ + PSA_KEY_TYPE_IS_ECC_KEYPAIR(key_type) ? PSA_KEY_EXPORT_ECC_KEYPAIR_MAX_SIZE(key_bits) : \ + PSA_KEY_TYPE_IS_ECC_PUBLIC_KEY(key_type) ? PSA_KEY_EXPORT_ECC_PUBLIC_KEY_MAX_SIZE(key_bits) : \ + 0) + #endif /* PSA_CRYPTO_SIZES_H */ From d14664a79b40e0497d919bf73ed4c4cd327a3026 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 10 Aug 2018 19:07:32 +0200 Subject: [PATCH 06/14] Move export key sanity check from generate to exercise Move the code to perform sanity checks on the exported key from generate_key to exercise_key. This way the sanity checks can be performed after importing or deriving a key as well. In addition to checking the exported key if its usage allows it, check the exported public key if the key is asymmetric. --- tests/suites/test_suite_psa_crypto.function | 236 +++++++++++++------- 1 file changed, 154 insertions(+), 82 deletions(-) diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index 310df38f44..317475041c 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -393,6 +393,156 @@ exit: return( 0 ); } +int exported_key_sanity_check( psa_key_type_t type, size_t bits, + uint8_t *exported, size_t exported_length ) +{ + if( key_type_is_raw_bytes( type ) ) + TEST_ASSERT( exported_length == ( bits + 7 ) / 8 ); + +#if defined(MBEDTLS_DES_C) + if( type == PSA_KEY_TYPE_DES ) + { + /* Check the parity bits. */ + unsigned i; + for( i = 0; i < bits / 8; i++ ) + { + unsigned bit_count = 0; + unsigned m; + for( m = 1; m <= 0x100; m <<= 1 ) + { + if( exported[i] & m ) + ++bit_count; + } + TEST_ASSERT( bit_count % 2 != 0 ); + } + } +#endif + +#if defined(MBEDTLS_RSA_C) && defined(MBEDTLS_PK_PARSE_C) + if( type == PSA_KEY_TYPE_RSA_KEYPAIR ) + { + /* Sanity check: does this look like the beginning of a PKCS#8 + * RSA key pair? Assumes bits is a multiple of 8. */ + size_t n_bytes = bits / 8 + 1; + size_t n_encoded_bytes; + unsigned char *n_end; + TEST_ASSERT( exported_length >= 7 + ( n_bytes + 3 ) * 9 / 2 ); + TEST_ASSERT( exported[0] == 0x30 ); + TEST_ASSERT( exported[1] == 0x82 ); // assumes >=416-bit key + TEST_ASSERT( exported[4] == 0x02 ); + TEST_ASSERT( exported[5] == 0x01 ); + TEST_ASSERT( exported[6] == 0x00 ); + TEST_ASSERT( exported[7] == 0x02 ); + n_encoded_bytes = exported[8]; + n_end = exported + 9 + n_encoded_bytes; + if( n_encoded_bytes & 0x80 ) + { + n_encoded_bytes = ( n_encoded_bytes & 0x7f ) << 7; + n_encoded_bytes |= exported[9] & 0x7f; + n_end += 1; + } + /* The encoding of n should start with a 0 byte since it should + * have its high bit set. However Mbed TLS is not compliant and + * generates an invalid, but widely tolerated, encoding of + * positive INTEGERs with a bit size that is a multiple of 8 + * with no leading 0 byte. Accept this here. */ + TEST_ASSERT( n_bytes == n_encoded_bytes || + n_bytes == n_encoded_bytes + 1 ); + if( n_bytes == n_encoded_bytes ) + TEST_ASSERT( exported[n_encoded_bytes <= 127 ? 9 : 10] == 0x00 ); + /* Sanity check: e must be 3 */ + TEST_ASSERT( n_end[0] == 0x02 ); + TEST_ASSERT( n_end[1] == 0x03 ); + TEST_ASSERT( n_end[2] == 0x01 ); + TEST_ASSERT( n_end[3] == 0x00 ); + TEST_ASSERT( n_end[4] == 0x01 ); + TEST_ASSERT( n_end[5] == 0x02 ); + } +#endif /* MBEDTLS_RSA_C */ + +#if defined(MBEDTLS_ECP_C) + if( PSA_KEY_TYPE_IS_ECC_KEYPAIR( type ) ) + { + /* Sanity check: does this look like the beginning of a PKCS#8 + * elliptic curve key pair? */ + TEST_ASSERT( exported_length >= bits * 3 / 8 + 10 ); + TEST_ASSERT( exported[0] == 0x30 ); + } +#endif /* MBEDTLS_ECP_C */ + + return( 0 ); + +exit: + return( 1 ); +} + +static int exercise_export_key( psa_key_slot_t slot, + psa_key_usage_t usage ) +{ + psa_key_type_t type; + size_t bits; + uint8_t *exported = NULL; + size_t exported_size = 0; + size_t exported_length = 0; + int ok = 0; + + if( ( usage & PSA_KEY_USAGE_EXPORT ) == 0 ) + { + TEST_ASSERT( psa_export_key( slot, NULL, 0, &exported_length ) == + PSA_ERROR_NOT_PERMITTED ); + return( 1 ); + } + + TEST_ASSERT( psa_get_key_information( slot, &type, &bits ) == PSA_SUCCESS ); + exported_size = PSA_KEY_EXPORT_MAX_SIZE( type, bits ); + exported = mbedtls_calloc( 1, exported_size ); + TEST_ASSERT( exported != NULL ); + + TEST_ASSERT( psa_export_key( slot, + exported, exported_size, + &exported_length ) == PSA_SUCCESS ); + ok = exported_key_sanity_check( type, bits, exported, exported_length ); + +exit: + mbedtls_free( exported ); + return( ok ); +} + +static int exercise_export_public_key( psa_key_slot_t slot ) +{ + psa_key_type_t type; + psa_key_type_t public_type; + size_t bits; + uint8_t *exported = NULL; + size_t exported_size = 0; + size_t exported_length = 0; + int ok = 0; + + TEST_ASSERT( psa_get_key_information( slot, &type, &bits ) == PSA_SUCCESS ); + if( ! PSA_KEY_TYPE_IS_ASYMMETRIC( type ) ) + { + TEST_ASSERT( psa_export_public_key( slot, + NULL, 0, &exported_length ) == + PSA_ERROR_INVALID_ARGUMENT ); + return( 1 ); + } + + public_type = PSA_KEY_TYPE_PUBLIC_KEY_OF_KEYPAIR( type ); + exported_size = PSA_KEY_EXPORT_MAX_SIZE( public_type, bits ); + exported = mbedtls_calloc( 1, exported_size ); + TEST_ASSERT( exported != NULL ); + + TEST_ASSERT( psa_export_public_key( slot, + exported, exported_size, + &exported_length ) == PSA_SUCCESS ); + ok = exported_key_sanity_check( public_type, bits, + exported, exported_length ); + +exit: + mbedtls_free( exported ); + return( ok ); +} + static int exercise_key( psa_key_slot_t slot, psa_key_usage_t usage, psa_algorithm_t alg ) @@ -421,6 +571,10 @@ static int exercise_key( psa_key_slot_t slot, test_fail( message, __LINE__, __FILE__ ); ok = 0; } + + ok = ok && exercise_export_key( slot, usage ); + ok = ok && exercise_export_public_key( slot ); + return( ok ); } @@ -3056,10 +3210,6 @@ void generate_key( int type_arg, psa_status_t expected_status = expected_status_arg; psa_key_type_t got_type; size_t got_bits; - unsigned char exported[616] = {0}; /* enough for a 1024-bit RSA key */ - size_t exported_length; - psa_status_t expected_export_status = - usage & PSA_KEY_USAGE_EXPORT ? PSA_SUCCESS : PSA_ERROR_NOT_PERMITTED; psa_status_t expected_info_status = expected_status == PSA_SUCCESS ? PSA_SUCCESS : PSA_ERROR_EMPTY_SLOT; psa_key_policy_t policy; @@ -3083,84 +3233,6 @@ void generate_key( int type_arg, TEST_ASSERT( got_type == type ); TEST_ASSERT( got_bits == bits ); - /* Export the key */ - TEST_ASSERT( psa_export_key( slot, - exported, sizeof( exported ), - &exported_length ) == expected_export_status ); - if( expected_export_status == PSA_SUCCESS ) - { - if( key_type_is_raw_bytes( type ) ) - TEST_ASSERT( exported_length == ( bits + 7 ) / 8 ); -#if defined(MBEDTLS_DES_C) - if( type == PSA_KEY_TYPE_DES ) - { - /* Check the parity bits. */ - unsigned i; - for( i = 0; i < bits / 8; i++ ) - { - unsigned bit_count = 0; - unsigned m; - for( m = 1; m <= 0x100; m <<= 1 ) - { - if( exported[i] & m ) - ++bit_count; - } - TEST_ASSERT( bit_count % 2 != 0 ); - } - } -#endif -#if defined(MBEDTLS_RSA_C) && defined(MBEDTLS_PK_PARSE_C) - if( type == PSA_KEY_TYPE_RSA_KEYPAIR ) - { - /* Sanity check: does this look like the beginning of a PKCS#8 - * RSA key pair? Assumes bits is a multiple of 8. */ - size_t n_bytes = bits / 8 + 1; - size_t n_encoded_bytes; - unsigned char *n_end; - TEST_ASSERT( exported_length >= 7 + ( n_bytes + 3 ) * 9 / 2 ); - TEST_ASSERT( exported[0] == 0x30 ); - TEST_ASSERT( exported[1] == 0x82 ); // assumes >=416-bit key - TEST_ASSERT( exported[4] == 0x02 ); - TEST_ASSERT( exported[5] == 0x01 ); - TEST_ASSERT( exported[6] == 0x00 ); - TEST_ASSERT( exported[7] == 0x02 ); - n_encoded_bytes = exported[8]; - n_end = exported + 9 + n_encoded_bytes; - if( n_encoded_bytes & 0x80 ) - { - n_encoded_bytes = ( n_encoded_bytes & 0x7f ) << 7; - n_encoded_bytes |= exported[9] & 0x7f; - n_end += 1; - } - /* The encoding of n should start with a 0 byte since it should - * have its high bit set. However Mbed TLS is not compliant and - * generates an invalid, but widely tolerated, encoding of - * positive INTEGERs with a bit size that is a multiple of 8 - * with no leading 0 byte. Accept this here. */ - TEST_ASSERT( n_bytes == n_encoded_bytes || - n_bytes == n_encoded_bytes + 1 ); - if( n_bytes == n_encoded_bytes ) - TEST_ASSERT( exported[n_encoded_bytes <= 127 ? 9 : 10] == 0x00 ); - /* Sanity check: e must be 3 */ - TEST_ASSERT( n_end[0] == 0x02 ); - TEST_ASSERT( n_end[1] == 0x03 ); - TEST_ASSERT( n_end[2] == 0x01 ); - TEST_ASSERT( n_end[3] == 0x00 ); - TEST_ASSERT( n_end[4] == 0x01 ); - TEST_ASSERT( n_end[5] == 0x02 ); - } -#endif /* MBEDTLS_RSA_C */ -#if defined(MBEDTLS_ECP_C) - if( PSA_KEY_TYPE_IS_ECC( type ) ) - { - /* Sanity check: does this look like the beginning of a PKCS#8 - * elliptic curve key pair? */ - TEST_ASSERT( exported_length >= bits * 3 / 8 + 10 ); - TEST_ASSERT( exported[0] == 0x30 ); - } -#endif /* MBEDTLS_ECP_C */ - } - /* Do something with the key according to its type and permitted usage. */ if( ! exercise_key( slot, usage, alg ) ) goto exit; From 4f6c77b0a9f4534b52c67b3416cc34d290ad9b5f Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sat, 11 Aug 2018 01:17:53 +0200 Subject: [PATCH 07/14] fixup format spec --- include/psa/crypto.h | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/include/psa/crypto.h b/include/psa/crypto.h index 87f2d60b70..3f8cb44c94 100644 --- a/include/psa/crypto.h +++ b/include/psa/crypto.h @@ -1191,7 +1191,7 @@ psa_status_t psa_get_key_information(psa_key_slot_t key, * PKCS\#1 (RFC 8017) as `RSAPrivateKey`, version 0. * ``` * RSAPrivateKey ::= SEQUENCE { - * version Version, -- 0 + * version INTEGER, -- must be 0 * modulus INTEGER, -- n * publicExponent INTEGER, -- e * privateExponent INTEGER, -- d @@ -1207,7 +1207,7 @@ psa_status_t psa_get_key_information(psa_key_slot_t key, * OpenSSL and OpenSSH, which the following ASN.1 structure: * ``` * DSAPrivateKey ::= SEQUENCE { - * version Version, -- 0 + * version INTEGER, -- must be 0 * prime INTEGER, -- p * subprime INTEGER, -- q * generator INTEGER, -- g @@ -1218,15 +1218,19 @@ psa_status_t psa_get_key_information(psa_key_slot_t key, * - For elliptic curve key pairs (key types for which * #PSA_KEY_TYPE_IS_ECC_KEYPAIR is true), the format is the * non-encrypted DER encoding of the representation defined by RFC 5915 as - * `ECPrivateKey`, version 1. + * `ECPrivateKey`, version 1. The `ECParameters` field must be a + * `namedCurve` OID as specified in RFC 5480 §2.1.1.1. The public key + * must be present and must be an `ECPoint` in the same format + * (uncompressed variant) an ECC public key of the + * corresponding type exported with psa_export_public_key(). * ``` * ECPrivateKey ::= SEQUENCE { * version INTEGER, -- must be 1 * privateKey OCTET STRING, - * -- `ceiling(log_{256}(n))`-byte string, big endian, + * -- `ceiling(log2(n)/8)`-byte string, big endian, * -- where n is the order of the curve. - * parameters ECParameters {{ NamedCurve }}, -- mandatory - * publicKey BIT STRING -- mandatory + * parameters [0] IMPLICIT ECParameters {{ namedCurve }}, -- mandatory + * publicKey [1] IMPLICIT BIT STRING -- mandatory * } * ``` * - For public keys (key types for which #PSA_KEY_TYPE_IS_PUBLIC_KEY is @@ -1308,20 +1312,21 @@ psa_status_t psa_export_key(psa_key_slot_t key, * - For elliptic curve public keys (key types for which * #PSA_KEY_TYPE_IS_ECC_PUBLIC_KEY is true), * the `subjectPublicKey` format is defined by RFC 3279 §2.3.5 as - * `ECPoint`, which is an OCTET STRING containing the uncompressed + * `ECPoint`, which contains the uncompressed * representation defined by SEC1 §2.3.3. * The OID is `id-ecPublicKey`, - * and the parameters must be given as a `namedCurve`. + * and the parameters must be given as a `namedCurve` OID as specified in + * RFC 5480 §2.1.1.1. * ``` * ansi-X9-62 OBJECT IDENTIFIER ::= * { iso(1) member-body(2) us(840) 10045 } * id-public-key-type OBJECT IDENTIFIER ::= { ansi-X9.62 2 } * id-ecPublicKey OBJECT IDENTIFIER ::= { id-publicKeyType 1 } * - * ECPoint ::= OCTET STRING - * -- first byte: 0x04; - * -- then x_P as a `ceiling(log_{256}(n))`-byte string, big endian; - * -- then y_P as a `ceiling(log_{256}(n))`-byte string, big endian, + * ECPoint ::= ... + * -- first 8 bits: 0x04; + * -- then x_P as an n-bit string, big endian; + * -- then y_P as a n-bit string, big endian, * -- where n is the order of the curve. * * EcpkParameters ::= CHOICE { -- other choices are not allowed From cb6adbb75023bc47cbd4082bccf0cff297f6d752 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sat, 11 Aug 2018 01:18:12 +0200 Subject: [PATCH 08/14] fixup sizes --- include/psa/crypto_sizes.h | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/include/psa/crypto_sizes.h b/include/psa/crypto_sizes.h index bc8edc6129..4df72b0254 100644 --- a/include/psa/crypto_sizes.h +++ b/include/psa/crypto_sizes.h @@ -412,10 +412,10 @@ * AlgorithmIdentifier ::= SEQUENCE { * algorithm OBJECT IDENTIFIER, * parameters OBJECT IDENTIFIER } -- namedCurve - * ECPoint ::= OCTET STRING - * -- first byte: 0x04; - * -- then x_P as a `ceiling(log_{256}(n))`-byte string, big endian; - * -- then y_P as a `ceiling(log_{256}(n))`-byte string, big endian, + * ECPoint ::= ... + * -- first 8 bits: 0x04; + * -- then x_P as an n-bit string, big endian; + * -- then y_P as a n-bit string, big endian, * -- where n is the order of the curve. * * - 2 * 4 bytes of SEQUENCE overhead; @@ -432,10 +432,10 @@ * ECPrivateKey ::= SEQUENCE { * version INTEGER, -- must be 1 * privateKey OCTET STRING, - * -- `ceiling(log_{256}(n))`-byte string, big endian, + * -- `ceiling(log2(n)/8)`-byte string, big endian, * -- where n is the order of the curve. - * parameters ECParameters {{ NamedCurve }}, -- mandatory - * publicKey BIT STRING -- mandatory + * parameters [0] IMPLICIT ECParameters {{ NamedCurve }}, + * publicKey [1] IMPLICIT BIT STRING * } * * - 4 bytes of SEQUENCE overhead; From dd2f95b855f9c570dd1d73260abc01f3000e7868 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sat, 11 Aug 2018 01:22:42 +0200 Subject: [PATCH 09/14] Improve and augment export sanity checks Implement sanity checks of exported public keys, using ASN.1 parsing. Rewrite the sanity checks of key pairs using ASN.1 parsing, so as to check more things with simpler code. --- tests/suites/test_suite_psa_crypto.function | 323 ++++++++++++++++---- 1 file changed, 269 insertions(+), 54 deletions(-) diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index 317475041c..b04f6a390b 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -5,7 +5,10 @@ #include "spm/psa_defs.h" #endif +#include "mbedtls/asn1.h" #include "mbedtls/asn1write.h" +#include "mbedtls/oid.h" + #include "psa/crypto.h" #define ARRAY_LENGTH( array ) ( sizeof( array ) / sizeof( *( array ) ) ) @@ -38,13 +41,6 @@ static int mem_is_zero( void *buffer, size_t size ) return( 1 ); } -static int key_type_is_raw_bytes( psa_key_type_t type ) -{ - psa_key_type_t category = type & PSA_KEY_TYPE_CATEGORY_MASK; - return( category == PSA_KEY_TYPE_RAW_DATA || - category == PSA_KEY_TYPE_CATEGORY_SYMMETRIC ); -} - /* Write the ASN.1 INTEGER with the value 2^(bits-1)+x backwards from *p. */ static int asn1_write_10x( unsigned char **p, unsigned char *start, @@ -393,11 +389,118 @@ exit: return( 0 ); } -int exported_key_sanity_check( psa_key_type_t type, size_t bits, - uint8_t *exported, size_t exported_length ) +typedef struct { - if( key_type_is_raw_bytes( type ) ) + unsigned char length; + unsigned char string[]; +} small_byte_string_t; +#define DECLARE_SMALL_STRING_OF_LITERAL( name, literal ) \ + static const small_byte_string_t name = \ + { sizeof( literal ) - 1, literal } + +#if defined(MBEDTLS_RSA_C) +DECLARE_SMALL_STRING_OF_LITERAL( key_type_oid_rsa, + MBEDTLS_OID_PKCS1_RSA ); +#endif +#if defined(MBEDTLS_ECP_C) +DECLARE_SMALL_STRING_OF_LITERAL( key_type_oid_ecc, + MBEDTLS_OID_EC_ALG_UNRESTRICTED ); +#endif + +static int is_oid_of_key_type( psa_key_type_t type, + const uint8_t *oid, size_t oid_length ) +{ + const small_byte_string_t *expected_oid = +#if defined(MBEDTLS_RSA_C) + PSA_KEY_TYPE_IS_RSA( type ) ? &key_type_oid_rsa : +#endif /* MBEDTLS_RSA_C */ +#if defined(MBEDTLS_ECP_C) + PSA_KEY_TYPE_IS_ECC( type ) ? &key_type_oid_ecc : +#endif /* MBEDTLS_ECP_C */ + NULL; + + if( expected_oid == NULL ) + { + char message[40]; + mbedtls_snprintf( message, sizeof( message ), + "OID not known for key type=0x%08lx", + (unsigned long) type ); + test_fail( message, __LINE__, __FILE__ ); + return( 0 ); + } + + TEST_ASSERT( oid_length == expected_oid->length ); + TEST_ASSERT( memcmp( oid, expected_oid->string, oid_length ) == 0 ); + return( 1 ); + +exit: + return( 0 ); +} + +static int asn1_skip_integer( unsigned char **p, const unsigned char *end, + size_t min_bits, size_t max_bits, + int must_be_odd ) +{ + size_t len; + size_t actual_bits; + unsigned char msb; + TEST_ASSERT( mbedtls_asn1_get_tag( p, end, &len, + MBEDTLS_ASN1_INTEGER ) == 0 ); + /* Tolerate a slight departure from DER encoding: + * - 0 may be represented by an empty string or a 1-byte string. + * - The sign bit may be used as a value bit. */ + if( ( len == 1 && ( *p )[0] == 0 ) || + ( len > 1 && ( *p )[0] == 0 && ( ( *p )[1] & 0x80 ) != 0 ) ) + { + ++( *p ); + --len; + } + if( min_bits == 0 && len == 0 ) + return( 1 ); + msb = ( *p )[0]; + TEST_ASSERT( msb != 0 ); + actual_bits = 8 * ( len - 1 ); + while( msb != 0 ) + { + msb >>= 1; + ++actual_bits; + } + TEST_ASSERT( actual_bits >= min_bits ); + TEST_ASSERT( actual_bits <= max_bits ); + if( must_be_odd ) + TEST_ASSERT( ( ( *p )[len-1] & 1 ) != 0 ); + *p += len; + return( 1 ); +exit: + return( 0 ); +} + +static int asn1_get_implicit_tag( unsigned char **p, const unsigned char *end, + size_t *len, + unsigned char n, unsigned char tag ) +{ + int ret; + ret = mbedtls_asn1_get_tag( p, end, len, + MBEDTLS_ASN1_CONTEXT_SPECIFIC | + MBEDTLS_ASN1_CONSTRUCTED | ( n ) ); + if( ret != 0 ) + return( ret ); + end = *p + *len; + ret = mbedtls_asn1_get_tag( p, end, len, tag ); + if( ret != 0 ) + return( ret ); + if( *p + *len != end ) + return( MBEDTLS_ERR_ASN1_LENGTH_MISMATCH ); + return( 0 ); +} + +static int exported_key_sanity_check( psa_key_type_t type, size_t bits, + uint8_t *exported, size_t exported_length ) +{ + if( PSA_KEY_TYPE_IS_UNSTRUCTURED( type ) ) TEST_ASSERT( exported_length == ( bits + 7 ) / 8 ); + else + TEST_ASSERT( exported_length <= PSA_KEY_EXPORT_MAX_SIZE( type, bits ) ); #if defined(MBEDTLS_DES_C) if( type == PSA_KEY_TYPE_DES ) @@ -416,64 +519,176 @@ int exported_key_sanity_check( psa_key_type_t type, size_t bits, TEST_ASSERT( bit_count % 2 != 0 ); } } + else #endif #if defined(MBEDTLS_RSA_C) && defined(MBEDTLS_PK_PARSE_C) if( type == PSA_KEY_TYPE_RSA_KEYPAIR ) { - /* Sanity check: does this look like the beginning of a PKCS#8 - * RSA key pair? Assumes bits is a multiple of 8. */ - size_t n_bytes = bits / 8 + 1; - size_t n_encoded_bytes; - unsigned char *n_end; - TEST_ASSERT( exported_length >= 7 + ( n_bytes + 3 ) * 9 / 2 ); - TEST_ASSERT( exported[0] == 0x30 ); - TEST_ASSERT( exported[1] == 0x82 ); // assumes >=416-bit key - TEST_ASSERT( exported[4] == 0x02 ); - TEST_ASSERT( exported[5] == 0x01 ); - TEST_ASSERT( exported[6] == 0x00 ); - TEST_ASSERT( exported[7] == 0x02 ); - n_encoded_bytes = exported[8]; - n_end = exported + 9 + n_encoded_bytes; - if( n_encoded_bytes & 0x80 ) - { - n_encoded_bytes = ( n_encoded_bytes & 0x7f ) << 7; - n_encoded_bytes |= exported[9] & 0x7f; - n_end += 1; - } - /* The encoding of n should start with a 0 byte since it should - * have its high bit set. However Mbed TLS is not compliant and - * generates an invalid, but widely tolerated, encoding of - * positive INTEGERs with a bit size that is a multiple of 8 - * with no leading 0 byte. Accept this here. */ - TEST_ASSERT( n_bytes == n_encoded_bytes || - n_bytes == n_encoded_bytes + 1 ); - if( n_bytes == n_encoded_bytes ) - TEST_ASSERT( exported[n_encoded_bytes <= 127 ? 9 : 10] == 0x00 ); - /* Sanity check: e must be 3 */ - TEST_ASSERT( n_end[0] == 0x02 ); - TEST_ASSERT( n_end[1] == 0x03 ); - TEST_ASSERT( n_end[2] == 0x01 ); - TEST_ASSERT( n_end[3] == 0x00 ); - TEST_ASSERT( n_end[4] == 0x01 ); - TEST_ASSERT( n_end[5] == 0x02 ); - } + uint8_t *p = exported; + uint8_t *end = exported + exported_length; + size_t len; + /* RSAPrivateKey ::= SEQUENCE { + * version Version, -- 0 + * modulus INTEGER, -- n + * publicExponent INTEGER, -- e + * privateExponent INTEGER, -- d + * prime1 INTEGER, -- p + * prime2 INTEGER, -- q + * exponent1 INTEGER, -- d mod (p-1) + * exponent2 INTEGER, -- d mod (q-1) + * coefficient INTEGER, -- (inverse of q) mod p + * } + */ + TEST_ASSERT( mbedtls_asn1_get_tag( &p, end, &len, + MBEDTLS_ASN1_SEQUENCE | + MBEDTLS_ASN1_CONSTRUCTED ) == 0 ); + TEST_ASSERT( p + len == end ); + if( ! asn1_skip_integer( &p, end, 0, 0, 0 ) ) + goto exit; + if( ! asn1_skip_integer( &p, end, bits, bits, 1 ) ) + goto exit; + if( ! asn1_skip_integer( &p, end, 2, bits, 1 ) ) + goto exit; + /* Require d to be at least half the size of n. */ + if( ! asn1_skip_integer( &p, end, bits / 2, bits, 1 ) ) + goto exit; + /* Require p and q to be at most half the size of n, rounded up. */ + if( ! asn1_skip_integer( &p, end, bits / 2, bits / 2 + 1, 1 ) ) + goto exit; + if( ! asn1_skip_integer( &p, end, bits / 2, bits / 2 + 1, 1 ) ) + goto exit; + if( ! asn1_skip_integer( &p, end, 1, bits / 2 + 1, 0 ) ) + goto exit; + if( ! asn1_skip_integer( &p, end, 1, bits / 2 + 1, 0 ) ) + goto exit; + if( ! asn1_skip_integer( &p, end, 1, bits / 2 + 1, 0 ) ) + goto exit; + TEST_ASSERT( p == end ); + } + else #endif /* MBEDTLS_RSA_C */ #if defined(MBEDTLS_ECP_C) if( PSA_KEY_TYPE_IS_ECC_KEYPAIR( type ) ) { - /* Sanity check: does this look like the beginning of a PKCS#8 - * elliptic curve key pair? */ - TEST_ASSERT( exported_length >= bits * 3 / 8 + 10 ); - TEST_ASSERT( exported[0] == 0x30 ); - } + uint8_t *p = exported; + uint8_t *end = exported + exported_length; + size_t len; + int version; + /* ECPrivateKey ::= SEQUENCE { + * version INTEGER, -- must be 1 + * privateKey OCTET STRING, + * -- `ceiling(log_{256}(n))`-byte string, big endian, + * -- where n is the order of the curve. + * parameters ECParameters {{ NamedCurve }}, -- mandatory + * publicKey BIT STRING -- mandatory + * } + */ + TEST_ASSERT( mbedtls_asn1_get_tag( &p, end, &len, + MBEDTLS_ASN1_SEQUENCE | + MBEDTLS_ASN1_CONSTRUCTED ) == 0 ); + TEST_ASSERT( p + len == end ); + TEST_ASSERT( mbedtls_asn1_get_int( &p, end, &version ) == 0 ); + TEST_ASSERT( version == 1 ); + TEST_ASSERT( mbedtls_asn1_get_tag( &p, end, &len, + MBEDTLS_ASN1_OCTET_STRING ) == 0 ); + /* Bug in Mbed TLS: the length of the octet string depends on the value */ + // TEST_ASSERT( len == PSA_BITS_TO_BYTES( bits ) ); + p += len; + TEST_ASSERT( asn1_get_implicit_tag( &p, end, &len, 0, + MBEDTLS_ASN1_OID ) == 0 ); + p += len; + TEST_ASSERT( asn1_get_implicit_tag( &p, end, &len, 1, + MBEDTLS_ASN1_BIT_STRING ) == 0 ); + TEST_ASSERT( p + len == end ); + TEST_ASSERT( p[0] == 0 ); /* 0 unused bits in the bit string */ + ++p; + TEST_ASSERT( p + 1 + 2 * PSA_BITS_TO_BYTES( bits ) == end ); + TEST_ASSERT( p[0] == 4 ); + } + else #endif /* MBEDTLS_ECP_C */ - return( 0 ); + if( PSA_KEY_TYPE_IS_PUBLIC_KEY( type ) ) + { + uint8_t *p = exported; + uint8_t *end = exported + exported_length; + size_t len; + mbedtls_asn1_buf alg; + mbedtls_asn1_buf params; + mbedtls_asn1_bitstring bitstring; + /* SubjectPublicKeyInfo ::= SEQUENCE { + * algorithm AlgorithmIdentifier, + * subjectPublicKey BIT STRING } + * AlgorithmIdentifier ::= SEQUENCE { + * algorithm OBJECT IDENTIFIER, + * parameters ANY DEFINED BY algorithm OPTIONAL } + */ + TEST_ASSERT( mbedtls_asn1_get_tag( &p, end, &len, + MBEDTLS_ASN1_SEQUENCE | + MBEDTLS_ASN1_CONSTRUCTED ) == 0 ); + TEST_ASSERT( p + len == end ); + TEST_ASSERT( mbedtls_asn1_get_alg( &p, end, &alg, ¶ms ) == 0 ); + if( ! is_oid_of_key_type( type, alg.p, alg.len ) ) + goto exit; + TEST_ASSERT( mbedtls_asn1_get_bitstring( &p, end, &bitstring ) == 0 ); + TEST_ASSERT( p == end ); + p = bitstring.p; +#if defined(MBEDTLS_RSA_C) + if( type == PSA_KEY_TYPE_RSA_PUBLIC_KEY ) + { + /* RSAPublicKey ::= SEQUENCE { + * modulus INTEGER, -- n + * publicExponent INTEGER } -- e + */ + TEST_ASSERT( bitstring.unused_bits == 0 ); + TEST_ASSERT( mbedtls_asn1_get_tag( &p, end, &len, + MBEDTLS_ASN1_SEQUENCE | + MBEDTLS_ASN1_CONSTRUCTED ) == 0 ); + TEST_ASSERT( p + len == end ); + if( ! asn1_skip_integer( &p, end, bits, bits, 1 ) ) + goto exit; + if( ! asn1_skip_integer( &p, end, 2, bits, 1 ) ) + goto exit; + TEST_ASSERT( p == end ); + } + else +#endif /* MBEDTLS_RSA_C */ +#if defined(MBEDTLS_ECP_C) + if( PSA_KEY_TYPE_IS_ECC_PUBLIC_KEY( type ) ) + { + /* ECPoint ::= ... + * -- first 8 bits: 0x04; + * -- then x_P as an n-bit string, big endian; + * -- then y_P as a n-bit string, big endian, + * -- where n is the order of the curve. + */ + TEST_ASSERT( bitstring.unused_bits == 0 ); + TEST_ASSERT( p + 1 + 2 * PSA_BITS_TO_BYTES( bits ) == end ); + TEST_ASSERT( p[0] == 4 ); + } + else +#endif /* MBEDTLS_ECP_C */ + { + char message[40]; + mbedtls_snprintf( message, sizeof( message ), + "No sanity check for public key type=0x%08lx", + (unsigned long) type ); + test_fail( message, __LINE__, __FILE__ ); + return( 0 ); + } + } + else + + { + /* No sanity checks for other types */ + } + + return( 1 ); exit: - return( 1 ); + return( 0 ); } static int exercise_export_key( psa_key_slot_t slot, From 8f609239d59426d2078508cd620c0982af7c26ad Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sat, 11 Aug 2018 01:24:55 +0200 Subject: [PATCH 10/14] Do export sanity checks in import_export as well This is not useful to validate the implementation when importing canonical input, which is the case for most import/export test cases, but it helps validate the sanity checks themselves. --- tests/suites/test_suite_psa_crypto.function | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index b04f6a390b..ee29ec951c 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -1001,6 +1001,9 @@ void import_export( data_t *data, goto destroy; } + if( ! exercise_export_key( slot, usage_arg ) ) + goto exit; + if( canonical_input ) { TEST_ASSERT( exported_length == data->len ); From ae3d2a2c26cccc2b7c83e335d7d2bd7d5a387211 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 13 Aug 2018 14:14:22 +0200 Subject: [PATCH 11/14] Avoid non-standard C constructs Don't rely on static initialization of a flexible array member, that's a GNU extension. The previous code also triggered a Clang warning "suggest braces around initialization of subobject" (-Wmissing-braces) for `struct {char a[]} = {"foo"}`. --- tests/suites/test_suite_psa_crypto.function | 42 ++++++++------------- 1 file changed, 16 insertions(+), 26 deletions(-) diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index ee29ec951c..d5922b7670 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -389,37 +389,27 @@ exit: return( 0 ); } -typedef struct -{ - unsigned char length; - unsigned char string[]; -} small_byte_string_t; -#define DECLARE_SMALL_STRING_OF_LITERAL( name, literal ) \ - static const small_byte_string_t name = \ - { sizeof( literal ) - 1, literal } - -#if defined(MBEDTLS_RSA_C) -DECLARE_SMALL_STRING_OF_LITERAL( key_type_oid_rsa, - MBEDTLS_OID_PKCS1_RSA ); -#endif -#if defined(MBEDTLS_ECP_C) -DECLARE_SMALL_STRING_OF_LITERAL( key_type_oid_ecc, - MBEDTLS_OID_EC_ALG_UNRESTRICTED ); -#endif - static int is_oid_of_key_type( psa_key_type_t type, const uint8_t *oid, size_t oid_length ) { - const small_byte_string_t *expected_oid = + const uint8_t *expected_oid = NULL; + size_t expected_oid_length = 0; #if defined(MBEDTLS_RSA_C) - PSA_KEY_TYPE_IS_RSA( type ) ? &key_type_oid_rsa : + if( PSA_KEY_TYPE_IS_RSA( type ) ) + { + expected_oid = (uint8_t *) MBEDTLS_OID_PKCS1_RSA; + expected_oid_length = sizeof( MBEDTLS_OID_PKCS1_RSA ) - 1; + } + else #endif /* MBEDTLS_RSA_C */ #if defined(MBEDTLS_ECP_C) - PSA_KEY_TYPE_IS_ECC( type ) ? &key_type_oid_ecc : + if( PSA_KEY_TYPE_IS_ECC( type ) ) + { + expected_oid = (uint8_t *) MBEDTLS_OID_EC_ALG_UNRESTRICTED; + expected_oid_length = sizeof( MBEDTLS_OID_EC_ALG_UNRESTRICTED ) - 1; + } + else #endif /* MBEDTLS_ECP_C */ - NULL; - - if( expected_oid == NULL ) { char message[40]; mbedtls_snprintf( message, sizeof( message ), @@ -429,8 +419,8 @@ static int is_oid_of_key_type( psa_key_type_t type, return( 0 ); } - TEST_ASSERT( oid_length == expected_oid->length ); - TEST_ASSERT( memcmp( oid, expected_oid->string, oid_length ) == 0 ); + TEST_ASSERT( oid_length == expected_oid_length ); + TEST_ASSERT( memcmp( oid, expected_oid, oid_length ) == 0 ); return( 1 ); exit: From 6ef7983208a01a2e2da58c5edabfcbc0b0dcd00c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 13 Aug 2018 14:17:06 +0200 Subject: [PATCH 12/14] Fix copypasta in PSA_KEY_EXPORT_MAX_SIZE documentation --- include/psa/crypto_sizes.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/psa/crypto_sizes.h b/include/psa/crypto_sizes.h index 4df72b0254..c42375beba 100644 --- a/include/psa/crypto_sizes.h +++ b/include/psa/crypto_sizes.h @@ -492,7 +492,6 @@ * * \param key_type A supported key type. * \param key_bits The size of the key in bits. - * \param alg The signature algorithm. * * \return If the parameters are valid and supported, return * a buffer size in bytes that guarantees that From c6290c043e4095768aef2bd4c145c7ae36a124a3 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 13 Aug 2018 17:24:59 +0200 Subject: [PATCH 13/14] Minor documentation improvements --- include/psa/crypto.h | 4 ++-- tests/suites/test_suite_psa_crypto.function | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/include/psa/crypto.h b/include/psa/crypto.h index 3f8cb44c94..c3899bfe7f 100644 --- a/include/psa/crypto.h +++ b/include/psa/crypto.h @@ -1204,7 +1204,7 @@ psa_status_t psa_get_key_information(psa_key_slot_t key, * ``` * - For DSA private keys (#PSA_KEY_TYPE_DSA_KEYPAIR), the format * is the non-encrypted DER encoding of the representation used by - * OpenSSL and OpenSSH, which the following ASN.1 structure: + * OpenSSL and OpenSSH, whose structure is described in ASN.1 as follows: * ``` * DSAPrivateKey ::= SEQUENCE { * version INTEGER, -- must be 0 @@ -1316,7 +1316,7 @@ psa_status_t psa_export_key(psa_key_slot_t key, * representation defined by SEC1 §2.3.3. * The OID is `id-ecPublicKey`, * and the parameters must be given as a `namedCurve` OID as specified in - * RFC 5480 §2.1.1.1. + * RFC 5480 §2.1.1.1 or other applicable standards. * ``` * ansi-X9-62 OBJECT IDENTIFIER ::= * { iso(1) member-body(2) us(840) 10045 } diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index d5922b7670..16227fb6d4 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -589,6 +589,7 @@ static int exported_key_sanity_check( psa_key_type_t type, size_t bits, TEST_ASSERT( asn1_get_implicit_tag( &p, end, &len, 0, MBEDTLS_ASN1_OID ) == 0 ); p += len; + /* publicKey: ECPoint in uncompressed representation (as below) */ TEST_ASSERT( asn1_get_implicit_tag( &p, end, &len, 1, MBEDTLS_ASN1_BIT_STRING ) == 0 ); TEST_ASSERT( p + len == end ); @@ -649,7 +650,7 @@ static int exported_key_sanity_check( psa_key_type_t type, size_t bits, if( PSA_KEY_TYPE_IS_ECC_PUBLIC_KEY( type ) ) { /* ECPoint ::= ... - * -- first 8 bits: 0x04; + * -- first 8 bits: 0x04 (uncompressed representation); * -- then x_P as an n-bit string, big endian; * -- then y_P as a n-bit string, big endian, * -- where n is the order of the curve. From dea46cf8f15c2357dd515039c5e494d766f53595 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 21 Aug 2018 16:12:54 +0200 Subject: [PATCH 14/14] Clarify comment in test In RSAPrivateKey, Version is an INTEGER. The version must be 0. --- tests/suites/test_suite_psa_crypto.function | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index 16227fb6d4..87bf28e238 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -519,7 +519,7 @@ static int exported_key_sanity_check( psa_key_type_t type, size_t bits, uint8_t *end = exported + exported_length; size_t len; /* RSAPrivateKey ::= SEQUENCE { - * version Version, -- 0 + * version INTEGER, -- must be 0 * modulus INTEGER, -- n * publicExponent INTEGER, -- e * privateExponent INTEGER, -- d