Skip to content

Simplify or specify public key format for DH and DSA #14

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 9 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
194 changes: 132 additions & 62 deletions include/psa/crypto.h
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,96 @@ psa_status_t psa_get_key_information(psa_key_handle_t handle,
psa_key_type_t *type,
size_t *bits);

/**
* \brief Set domain parameters for a key.
*
* Some key types require additional domain parameters to be set before import
* or generation of the key. The domain parameters can be set with this
* function or, for key generation, through the \c extra parameter of
* psa_generate_key().
*
* The format for the required domain parameters varies by the key type.
* - For DSA public keys (#PSA_KEY_TYPE_DSA_PUBLIC_KEY),
* the `Dss-Parms` format as defined by RFC 3279 §2.3.2.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

“the DER representation of the Dss-Parms format”

* ```
* Dss-Parms ::= SEQUENCE {
* p INTEGER,
* q INTEGER,
* g INTEGER
* }
* ```
* - For Diffie-Hellman key exchange keys (#PSA_KEY_TYPE_DH_PUBLIC_KEY), the
* `DomainParameters` format as defined by RFC 3279 §2.3.3.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

“the DER representation of the DomainParameters format”

* ```
* DomainParameters ::= SEQUENCE {
* p INTEGER, -- odd prime, p=jq +1
* g INTEGER, -- generator, g
* q INTEGER, -- factor of p-1
* j INTEGER OPTIONAL, -- subgroup factor
* validationParms ValidationParms OPTIONAL
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We try to avoid optional stuff in PSA. Do we allow, mandate or reject j and validationParms? If it's allowed, do we guarantee that the implementation will check it?

My gut feeling:

  • Allow them because it's convenient to accept whatever has been package, and it's very easy to skip.
  • Allow an implementation to ignore them altogether, because verifying them can be costly and the API is meant to be suitable for microcontrollers.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds very good for usability.

Considering it from the security perspective the checks a specific implementation makes have significant impact and I think that we should be very explicit about it. How about allowing the validationParams to be present and allowing the implementations to ignore them, but adding a new function for parameter validation?

By the way can't we just drop custom parameter FFDH from 1.0 and use named groups instead. We could reintroduce custom parameters if and when customers ask for it.
(I know I have asked this before, but I don't remember what was the conclusion.)

I dislike custom parameter FFDH, because it can be backdoored and no matter how hard you validate your parameters, you won't notice:
https://tls.mbed.org/tech-updates/blog/dh-backdoors

(Of course you can argue that named groups have to be bigger because of logjam, but I wouldn't advise to use keys that short with custom parameters either.)

* }
* ValidationParms ::= SEQUENCE {
* seed BIT STRING,
* pgenCounter INTEGER
* }
* ```
*
* \param handle Handle to the key to set domain parameters for.
* \param[in] data Buffer containing the key domain parameters. The content
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documentation for handle parameter is missing.

* of this buffer is interpreted according to \p type. of
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing \p type parameter in the documentation and in the prototype.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something's missing before “of psa_export_key() or …”

* psa_export_key() or psa_export_public_key() for the
* chosen type.
* \param data_length Size of the \p data buffer in bytes.
*
* \retval #PSA_SUCCESS
* \retval #PSA_ERROR_INVALID_HANDLE
* \retval #PSA_ERROR_OCCUPIED_SLOT
* There is already a key in the specified slot.
* \retval #PSA_ERROR_INVALID_ARGUMENT
* \retval #PSA_ERROR_COMMUNICATION_FAILURE
* \retval #PSA_ERROR_HARDWARE_FAILURE
* \retval #PSA_ERROR_TAMPERING_DETECTED
* \retval #PSA_ERROR_BAD_STATE
* The library has not been previously initialized by psa_crypto_init().
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not relevant for this function (you won't get a handle in the first place)

* It is implementation-dependent whether a failure to initialize
* results in this error code.
*/
psa_status_t psa_set_key_domain_parameters(psa_key_handle_t handle,
const uint8_t *data,
size_t data_length);

/**
* \brief Get domain parameters for a key.
*
* Get the domain parameters for a key with this function, if any. The format
* of the domain parameters written to \p data is specified in the
* documentation for psa_set_key_domain_parameters().
*
* \param handle Handle to the key to get domain parameters from.
* \param[out] data On success, the key domain parameters.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documentation for handle parameter is missing.

* \param data_size Size of the \p data buffer in bytes.
* \param[out] data_length On success, the number of bytes
* that make up the key domain parameters data.
*
* \retval #PSA_SUCCESS
* \retval #PSA_ERROR_INVALID_HANDLE
* \retval #PSA_ERROR_EMPTY_SLOT
* There is no key in the specified slot.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not an error: you can read domain parameters that have been set even before importing a key. The possible errors here are that no domain parameters have been set, or that the slot contains a key which does not have any domain parameters.

* \retval #PSA_ERROR_INVALID_ARGUMENT
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+ NOT_SUPPORTED if the implementation doesn't have code to format domain parameters

* \retval #PSA_ERROR_NOT_SUPPORTED
* \retval #PSA_ERROR_COMMUNICATION_FAILURE
* \retval #PSA_ERROR_HARDWARE_FAILURE
* \retval #PSA_ERROR_TAMPERING_DETECTED
* \retval #PSA_ERROR_BAD_STATE
* The library has not been previously initialized by psa_crypto_init().
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not relevant for this function (you won't get a handle in the first place)

* It is implementation-dependent whether a failure to initialize
* results in this error code.
*/
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a macro in crypto_sizes.h to determine the buffer size from the key type and bit-size.

psa_status_t psa_get_key_domain_parameters(psa_key_handle_t handle,
uint8_t *data,
size_t data_size,
size_t *data_length);

/**
* \brief Export a key in binary format.
*
Expand Down Expand Up @@ -404,19 +494,10 @@ psa_status_t psa_get_key_information(psa_key_handle_t handle,
* 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, whose structure is described in ASN.1 as follows:
* ```
* DSAPrivateKey ::= SEQUENCE {
* version INTEGER, -- must be 0
* prime INTEGER, -- p
* subprime INTEGER, -- q
* generator INTEGER, -- g
* public INTEGER, -- y
* private INTEGER, -- x
* }
* ```
* - For DSA private keys (#PSA_KEY_TYPE_DSA_KEYPAIR), the format is the
* representation of the private key `x` as a big-endian byte string. The
* length of the byte string is the private key size in bytes (leading zeroes
* are not stripped).
* - For elliptic curve key pairs (key types for which
* #PSA_KEY_TYPE_IS_ECC_KEYPAIR is true), the format is
* a representation of the private value as a `ceiling(m/8)`-byte string
Expand All @@ -428,6 +509,10 @@ psa_status_t psa_get_key_information(psa_key_handle_t handle,
* and `PSA_ECC_CURVE_BRAINPOOL_PXXX`).
* This is the content of the `privateKey` field of the `ECPrivateKey`
* format defined by RFC 5915.
* - For Diffie-Hellman key exchange key pairs (#PSA_KEY_TYPE_DH_KEYPAIR), the
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

“key pairs” → “private keys” for consistency

* format is the representation of the private key `x` as a big-endian byte
* string. The length of the byte string is the private key size in bytes
* (leading zeroes are not stripped).
* - 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().
*
Expand Down Expand Up @@ -467,71 +552,42 @@ psa_status_t psa_export_key(psa_key_handle_t handle,
* The output of this function can be passed to psa_import_key() to
* create an object that is equivalent to the public key.
*
* 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 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 }
* This specification supports a single format for each key type.
* Implementations may support other formats as long as the standard
* format is supported. Implementations that support other formats
* should ensure that the formats are clearly unambiguous so as to
* minimize the risk that an invalid input is accidentally interpreted
* according to a different format.
*
* For standard key types, the output format is as follows:
* - For RSA public keys (#PSA_KEY_TYPE_RSA_PUBLIC_KEY), the DER encoding of
* the representation defined by RFC 3279 §2.3.1 as `RSAPublicKey`.
* ```
* 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
* the format is defined by RFC 3279 §2.3.5 as
* `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` OID as specified in
* RFC 5480 §2.1.1.1 or other applicable standards.
* ```
* 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 ::= ...
* -- first 8 bits: 0x04;
* -- then x_P as a `ceiling(m/8)`-byte string, big endian;
* -- then y_P as a `ceiling(m/8)`-byte string, big endian;
* -- where `m` is the bit size associated with the curve,
* -- i.e. the bit size of `q` for a curve over `F_q`.
*
* EcpkParameters ::= CHOICE { -- other choices are not allowed
* namedCurve OBJECT IDENTIFIER }
* ```
* - For DSA public keys (#PSA_KEY_TYPE_DSA_PUBLIC_KEY), the format is the
* representation of the public key `y = g^x mod p` as a big-endian byte
* string. The length of the byte string is the length of the base prime `p`
* in bytes.
* - For Diffie-Hellman key exchange public keys (#PSA_KEY_TYPE_DH_PUBLIC_KEY),
* the format is the representation of the public key `y = g^x mod p` as a
* big-endian byte string. The length of the byte string is the length of the
* base prime `p` in bytes.
*
* \param handle Handle to the key to export.
* \param[out] data Buffer where the key data is to be written.
Expand Down Expand Up @@ -2159,7 +2215,9 @@ psa_status_t psa_key_derivation(psa_crypto_generator_t *generator,
* in the same format that psa_import_key()
* accepts. The standard formats for public
* keys are documented in the documentation
* of psa_export_public_key().
* of psa_export_public_key(). For EC keys, it
* must also be of the same group as the private
* key.
* \param peer_key_length Size of \p peer_key in bytes.
* \param alg The key agreement algorithm to compute
* (\c PSA_ALG_XXX value such that
Expand Down Expand Up @@ -2261,6 +2319,18 @@ typedef struct {
* specifying the public exponent. The
* default public exponent used when \p extra
* is \c NULL is 65537.
* - For an DSA key (\p type is
* #PSA_KEY_TYPE_DSA_KEYPAIR), \p extra is an
* optional structure specifying the key domain
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It isn't a structure, it's a buffer that will be parsed.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find this description unclear: it looks like you can pass extra, use set_key_domain_parameters, or do neither. In fact you need to do at least one, and the last one applies.

For a DSA key (\p type is #PSA_KEY_TYPE_DSA_KEYPAIR) or a finite-field Diffie-Hellman key (p type is #PSA_KEY_TYPE_DH_KEYPAIR), extra is a pointer to a buffer containing domain parameters in DER representation. The format of buffer is the same as the input to psa_set_key_domain_parameters(). If the application has previously called psa_set_key_domain_parameters() on \p handle, it may pass an empty buffer (`extra_length = 0`), in which case the key generation will use the previously set domain parameters.

* parameters. The key domain parameters can also be
* provided by psa_set_key_domain_parameters(),
* which documents the format of the structure.
* - For a DH key (\p type is
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic is exactly the same for DSA and DH, so you can combine their documentation.

* #PSA_KEY_TYPE_DH_KEYPAIR), the \p extra is an
* optional structure specifying the key domain
* parameters. The key domain parameters can also be
* provided by psa_set_key_domain_parameters(),
* which documents the format of the structure.
* \param extra_size Size of the buffer that \p extra
* points to, in bytes. Note that if \p extra is
* \c NULL then \p extra_size must be zero.
Expand Down
9 changes: 9 additions & 0 deletions include/psa/crypto_values.h
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,15 @@
#define PSA_ECC_CURVE_CURVE25519 ((psa_ecc_curve_t) 0x001d)
#define PSA_ECC_CURVE_CURVE448 ((psa_ecc_curve_t) 0x001e)

/** Diffie-Hellman key exchange public key. */
#define PSA_KEY_TYPE_DH_PUBLIC_KEY ((psa_key_type_t)0x60040000)
/** Diffie-Hellman key exchange key pair (private and public key). */
#define PSA_KEY_TYPE_DH_KEYPAIR ((psa_key_type_t)0x70040000)
/** Whether a key type is a Diffie-Hellman key exchange key (pair or
* public-only). */
#define PSA_KEY_TYPE_IS_DH(type) \
(PSA_KEY_TYPE_PUBLIC_KEY_OF_KEYPAIR(type) == PSA_KEY_TYPE_DH_PUBLIC_KEY)

/** The block size of a block cipher.
*
* \param type A cipher key type (value of type #psa_key_type_t).
Expand Down
Loading