Skip to content
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

Add support for interruptible signature calculation and verification #107

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

athoelke
Copy link
Contributor

@athoelke athoelke commented Oct 12, 2023

Add a new class of 'interruptible operation' to the API. These allow complex calculations to be broken into smaller steps, e.g. for asymmetric signature algorithms, to enable bounded latency operation of the API in constrained execution environments.

Operations for interruptible signature and interruptible verification have been introduced. These pick up the main concepts from the experimental interruptible hash signature and verification API added to mbedtls, but adapt the API to also support message signature functionality.

In the process of adding the API, I have reworked much of the material relating to different algorithm behaviors (hash-and-sign, message-signature and specialized algorithms). There is no quick way to reverse engineer a 'nice' set of commits for the many changes and additions to §10.7, so the PR is presented pretty much 'all in one go'.

If it is easier to read the end result than the diff, Asymmetric signature is on page 256 in this draft PDF: IHI0086-PSA_Certified_Crypto_API-1.3.0-interruptible-signature-draft2.pdf

The previous drafts:
* IHI0086-PSA_Certified_Crypto_API-1.2.0-draft-interruptible-api.pdf.
* IHI0086-PSA_Certified_Crypto_API-1.3.0-interruptible-signature-draft.pdf.

The remaining todos have now been resolved.

Some of the rationale behind the design and naming of the API is written up in #98.

Fixes #23

@athoelke athoelke added enhancement New feature or request API design Related the design of the API Crypto API Issue or PR related to the Cryptography API labels Oct 12, 2023
@athoelke athoelke added this to the Crypto API 1.2 milestone Oct 12, 2023
@athoelke
Copy link
Contributor Author

Note also that the API naming is 'the best idea so far', and is not fixed until we issue the 1.2 specification. If you have suggestions for better/different names or terminology, please comment and let me know.

@athoelke
Copy link
Contributor Author

athoelke commented Nov 7, 2023

Need to also add these functions to the list of APIs that require the SIGN and VERIFY usage flags in doc/crypto/api/keys/policy.rst.

Provide interruptible operations for message and hash signature calculation and verification.

Reworked information in this section of the API relating to the behavior of different types of signature algorithm, consolidating this to the section introduction.
@athoelke
Copy link
Contributor Author

Rebased this PR, now that #177 has been merged.

I have also reworked the introduction of Interruptible operations to the Functionality chapter, following the restructuring that was introduced by the addition of the PAKE operation.

@athoelke
Copy link
Contributor Author

I've also attached a new PDF render (see the main PR description above)

* Align permitted errors with the single-part functions
* Introduce the max_ops APIs
@athoelke
Copy link
Contributor Author

Although we worked hard during API development to avoid having collisions in the first 31 characters of API identifiers (minimum requirement for a C99 implementation), we missed the collisions between psa_sign_interruptible_operation_t and psa_sign_interruptible_operation_init() (33-character common prefix), and psa_verify_interruptible_operation_t and psa_verify_interruptible_operation_init() (35-character common prefix).

However, the 31-significant character requirement is for 'external identifiers', and I am not sure that type names are considered to be 'external' rather than 'internal' (for which a 63 significant character requirement holds)?

@athoelke
Copy link
Contributor Author

The identifier length maybe-problem comes back again, but harder with the additional interruptible operations required for #198. See my comment.

@gilles-peskine-arm
Copy link
Contributor

The exact phrase “external identifier” isn't defined in C99 or C11, but “external name” is: “As discussed in 5.2.4.1, an implementation may limit the number of significant >
characters in an identifier; the limit for an external name (an identifier that>
linkage) may be more restrictive than that for an internal name (a macro name o>
identifier that does not have external linkage)” (N1256 §6.4.2.1). (See also: concurring Stack Overflow answer.) A type name does not have external linkage. Only object and function names can be external identifiers (if not scoped to a file or block. i.e. only if they're implicitly or explicitly extern). 31 characters is only a limit between external identifiers.

@athoelke
Copy link
Contributor Author

athoelke commented May 21, 2024

Revisiting the naming of the API elements, after considering an interruptible API for key agreement use cases (#198). For those functions, there are no expected multi-part operations, so we dispense with interruptible in the operation and function names as there is no overlap between multi-part and interruptible operation names.

However....

It would be ideal if the naming patterns for multi-part and interruptible APIs were distinct, so that it is immediately apparent when reading code which of these types of API patterns is being used. To achieve that, it would be better to begin an interruptible operation with a function suffix like *_start(), as we use *_setup() for multi-part operations.

That would suggest that we rename the above APIs to psa_sign_interruptible_start() and psa_sign_interruptible_start_complete() - and the latter seems to me to be confusing?

So perhaps we could instead do psa_sign_interruptible_setup_start() and psa_sign_interruptible_setup_complete()? - this presents an issue for verification, as psa_verify_interruptible_setup_start() and psa_verify_interruptible_setup_complete() have exactly 31 initial characters in common, and a C99 implementation is permitted to see these as the same external identifier.

Using psa_verify_interruptible_start_setup() and psa_verify_interruptible_complete_setup() result in a name prefix collision with psa_verify_interruptible_complete().

Options

  1. Use *_setup() for both multi-part and interruptible patterns.
  2. Have inconsistencies between the signature and key agreement use case interruptible API patterns.
  3. Live with the odd-sounding psa_sign_interruptible_start_complete() and use the psa_sign_interruptible_start()/psa_sign_interruptible_start_complete() pattern for the signature API.
  4. Find a new name for *_start() that is five or fewer characters, and is less awkward for psa_sign_interruptible_xxx_complete().
  5. Think of a new name for *_setup_complete(), that will be used in a loop until completed. E.g. *_prepare()? - or is the loss of *_complete() an issue for consistency with the 'loop until done' semantics of interruptible operations?
  6. Find a replacement for interruptible in the API, that uses at least one fewer characters (12 or less). E.g. restartable, incremental, stepwise.

Place your votes, thoughts, or other suggestions below...

@gilles-peskine-arm
Copy link
Contributor

psa_sign_interruptible_setup_start() and psa_sign_interruptible_setup_complete()

That's the pattern I'd like: setup-update*-finish for data-driven multistep, start-complete for computation-driven multistep.

I don't have a good idea to offer (yet?), but I would prefer a consistent abbreviation (even if it's non-obvious like intible) to an occasional deviation to the naming pattern when the full name gets too long.

Given the length constraint, we have to consider key derivation. Even though Mbed TLS doesn't need interruptible ECC derivation at the moment, the API should include it.

I guess this is the starting point?

psa_key_derivation_output_key_start(
    const psa_key_attributes_t *attributes,
    psa_key_derivation_operation_t *operation,
    psa_key_derivation_output_interruptible_operation_t *iop);
psa_key_derivation_output_key_complete(
    const psa_key_derivation_output_interruptible_operation_t *iop,
    mbedtls_svc_key_id_t *key);
psa_key_derivation_output_key_abort(
    psa_key_derivation_output_interruptible_operation_t *iop);

@athoelke
Copy link
Contributor Author

athoelke commented May 22, 2024

Given the length constraint, we have to consider key derivation. Even though Mbed TLS doesn't need interruptible ECC derivation at the moment, the API should include it.

I guess this is the starting point?

psa_key_derivation_output_key_start(
    const psa_key_attributes_t *attributes,
    psa_key_derivation_operation_t *operation,
    psa_key_derivation_output_interruptible_operation_t *iop);
psa_key_derivation_output_key_complete(
    const psa_key_derivation_output_interruptible_operation_t *iop,
    mbedtls_svc_key_id_t *key);
psa_key_derivation_output_key_abort(
    psa_key_derivation_output_interruptible_operation_t *iop);

Hmmm. And that concept for key derivation suggests that the pattern for interruptible signature could be entirely different? It is not the operation that is interruptible, it is individual functions. . If the interruptible function is an operation function, then the interruptible state is a bit like an satellite object to the multi-part operation state.

If we follow that idea, then we would end up with a rather different API for sign/verify message - but the sign/verify hash would almost exactly match the beta API in Mbed TLS. (Sorry @paul-elliott-arm - I know you're working on turning this current PR into code right now...).

The simplest addition would be to provide an interruptible version of psa_sign_message() and psa_verify_message(), similar to the current Mbed TLS interruptible sign hash:

psa_sign_message_interruptible_t
psa_sign_message_start(op, key, alg, [inbuf] message)
psa_sign_message_complete(op, [outbuf] sig)
psa_sign_message_abort(op)

However, this does not work for signature algorithms that require complex computation with the key prior to digesting the message, and the Crypto API design does not permit an implementation to refer to application memory buffers outside of the call that supplies the buffer (i.e. message above must be processed or copied by the time psa_sign_message_start() returns).

So, better would be for interruptible sign/verify message to be additional functions (and interruptible state objects) for the suggested multi-part sign/verify-message API from #98. Something like this:

// basic sign message operation
psa_sign_message_operation_t
psa_sign_message_setup(op, key, alg)
psa_sign_message_update(op, [inbuf] msg)
psa_sign_message_finish(op, [outbuf] sig)
psa_sign_message_abort(op)

// interruptible setup for sign message
psa_sign_message_setup_interruptible_t
psa_sign_message_setup_start(op, key, alg, iop)
psa_sign_message_setup_complete(iop)
psa_sign_message_setup_abort(iop)

// interruptible finish for sign message
psa_sign_message_finish_interruptible_t
psa_sign_message_finish_start(op, iop)
psa_sign_message_finish_complete(iop, [outbuf] sig)
psa_sign_message_finish_abort(iop)

// basic verify message operation
psa_verify_message_operation_t
psa_verify_message_setup(op, key, alg, [inbuf] sig)
psa_verify_message_update(op, [inbuf] msg)
psa_verify_message_finish(op)
psa_verify_message_abort(op)

// interruptible setup for verify message
psa_verify_message_setup_interruptible_t
psa_verify_message_setup_start(op, key, alg, [inbuf] sig, iop)
psa_verify_message_setup_complete(iop)
psa_verify_message_setup_abort(iop)

// interruptible finish for verify message
psa_verify_message_finish_interruptible_t
psa_verify_message_finish_start(op, iop)
psa_verify_message_finish_complete(iop)
psa_verify_message_finish_abort(iop)

If this approach is used, then:

  • There is a reusable pattern for all such functions.
  • We could avoid using operation in the interruptible state objects and call them *_interruptible_t instead - which distinguishes them clearly from multi-part state objects.
  • Drop interruptible from function names, without creating name collisions, and preventing concerns arising from the 31 significant character limit

But, there are more state objects, and interruptible message signing involves three state objects, making the application code somewhat more complex.

@paul-elliott-arm
Copy link
Contributor

I actually quite like the simplicity of this approach. Given the various stages for sign/verify message are basically going to be compulsary, do we necessarily have to have seperate state objects?

I presume we would be adding these new message interfaces, but would return PSA_ERROR_NOT_SUPPORTED for now?

@athoelke
Copy link
Contributor Author

I actually quite like the simplicity of this approach. Given the various stages for sign/verify message are basically going to be compulsary, do we necessarily have to have separate state objects?

My assumption is that the footprint of a multi-part sign-message operation object is significantly smaller than that of an interruptible sign-message operation - so having the interruptible state in a separate object is valuable for implementations or application that do not support or use the interruptibile interfaces.

Keeping the interruptible sign-hash separate from the interruptible sign-message is probably valuable for the same reason - interruptible sign-hash does not need to store the hash internal state at any point.

If these assumptions are not, in fact, correct, because the signature computation state is of a similar size to the internal hash state (only one is required at any one time), then we could consider collapsing all of the 'sign' objects back into a single type. But instead of reverting to the API in the current PR content, we include the multi-part sign message operation functionality so that there is no need to distinguish the multi-part operation functions from the interruptible multi-part operation functions.

@athoelke
Copy link
Contributor Author

athoelke commented May 23, 2024

If these assumptions are not, in fact, correct, because the signature computation state is of a similar size to the internal hash state (only one is required at any one time), then we could consider collapsing all of the 'sign' objects back into a single type. But instead of reverting to the API in the current PR content, we include the multi-part sign message operation functionality so that there is no need to distinguish the multi-part operation functions from the interruptible multi-part operation functions.

However, this would again make the interruptible signature API different in pattern to the other interruptible interfaces. How much is it worth being able to code the use of an interruptible version of an API without having to double check the state object name, or function names, because the pattern is always the same?

Once there is a single exception, then a developer can never be sure unless they learn every instance (or consult the docs).

@gilles-peskine-arm
Copy link
Contributor

gilles-peskine-arm commented May 28, 2024

My assumption is that the footprint of a multi-part sign-message operation object is significantly smaller than that of an interruptible sign-message operation

That assumption is very likely false if you're talking total memory consumption: the intermediate state of an ECC calculation is likely to be significantly larger than the intermediate state of a hash calculation. Note that I'm referring to the total memory consumption: in Mbed TLS, mbedtls_ecp_restart_ctx is smaller than e.g. mbedtls_sha256_context, but that's only because it contains several pointers to heap-allocated objects. So there's no strong incentive to avoid needing enough memory for the hash state when doing sign-message.

@gilles-peskine-arm
Copy link
Contributor

I would prefer to have a consistent infix in function names, just so that people who aren't interested in the functionality (which is most users) can ignore it in a function index.

In today's call, we suggested iop as an infix. Rejected proposals:

  • interruptible is too long — as discussed, it tends to take us over the 32-character limit for significance in identifiers.
  • restartable is barely not too long, and is a less descriptive name.
  • No shortening of interruptible (int, inter, ible, …) preserves both its comprehensibility and non-ambiguity.

@gilles-peskine-arm
Copy link
Contributor

Even a short infix like iop takes us over the limit for psa_key_derivation_output_key which is already 29 characters.

@paul-elliott-arm
Copy link
Contributor

so, the sort of agreed parts would look like this:

typedef /* implementation-defined type */ psa_sign_iop_operation_t;
typedef /* implementation-defined type */ psa_verify_iop_operation_t;

#define PSA_SIGN_IOP_OPERATION_INIT \
    /* implementation-defined value */

#define PSA_VERIFY_IOP_OPERATION_INIT \
    /* implementation-defined value */

#define PSA_IOP_MAX_OPS_UNLIMITED UINT32_MAX

psa_status_t psa_sign_iop_abort(psa_sign_iop_operation_t *operation);
psa_status_t psa_sign_iop_complete(psa_sign_iop_operation_t *operation,
                                   uint8_t *signature,
                                   size_t signature_size,
                                   size_t *signature_length);
uint32_t psa_sign_iop_get_num_ops(psa_sign_iop_operation_t *operation);
psa_status_t psa_sign_iop_hash(psa_sign_iop_operation_t *operation,
                               const uint8_t *hash,
                               size_t hash_length);
psa_sign_iop_operation_t psa_sign_iop_operation_init(void);
psa_status_t psa_sign_iop_setup(psa_sign_iop_operation_t *operation,
                                psa_key_id_t key,
                                psa_algorithm_t alg);
psa_status_t psa_sign_iop_setup_complete(psa_sign_iop_operation_t *operation);
psa_status_t psa_sign_iop_update(psa_sign_iop_operation_t *operation,
                                 const uint8_t *input,
                                 size_t input_length);

psa_status_t psa_verify_iop_abort(psa_verify_iop_operation_t *operation);
psa_status_t psa_verify_iop_complete(psa_verify_iop_operation_t *operation);
uint32_t psa_verify_iop_get_num_ops(psa_verify_iop_operation_t *operation);
psa_status_t psa_verify_iop_hash(psa_verify_iop_operation_t *operation,
                                 const uint8_t *hash,
                                 size_t hash_length);
psa_verify_iop_operation_t psa_verify_iop_operation_init(void);
psa_status_t psa_verify_iop_setup(psa_verify_iop_operation_t *operation,
                                  psa_key_id_t key,
                                  psa_algorithm_t alg,
                                  const uint8_t *signature,
                                  size_t signature_length);
psa_status_t psa_verify_iop_setup_complete(psa_verify_iop_operation_t *operation);
psa_status_t psa_verify_iop_update(psa_verify_iop_operation_t *operation,
                                   const uint8_t *input,
                                   size_t input_length);

/* -------------------------------*/

typedef /* implementation-defined */ psa_generate_key_iop_operation_t;

#define PSA_GENERATE_KEY_IOP_OPERATION_INIT /* implementation-defined */
psa_generate_key_iop_operation_t psa_generate_key_iop_operation_init();

psa_status_t psa_generate_key_iop_setup(psa_generate_key_iop_operation_t *op,
                                                  const psa_key_attributes_t * attr);
psa_status_t psa_generate_key_iop_complete(psa_generate_key_iop_operation_t *op,
                                                     psa_key_id_t *key);
psa_status_t psa_generate_key_iop_abort(psa_generate_key_interruptible_operation_t *op);

@paul-elliott-arm
Copy link
Contributor

Use psa_kdf_iop_* for key derivation or psa_derive_key_iop_* for key derivation.

remove operation from structs. Instead of op call parameter iop

Use new key agreement operation (pass in key attributes, key and buffer containing public key, outputs key) not raw.

@paul-elliott-arm
Copy link
Contributor

/* Sign Hash / Message ----------------------------------*/

typedef /* implementation-defined type */ psa_sign_iop_t;
typedef /* implementation-defined type */ psa_verify_iop_t;

#define PSA_SIGN_IOP_INIT \
    /* implementation-defined value */

#define PSA_VERIFY_IOP_INIT \
    /* implementation-defined value */

#define PSA_IOP_MAX_OPS_UNLIMITED UINT32_MAX

psa_status_t psa_sign_iop_abort(psa_sign_iop_t *iop);
psa_status_t psa_sign_iop_complete(psa_sign_iop_t *iop,
                                   uint8_t *signature,
                                   size_t signature_size,
                                   size_t *signature_length);
uint32_t psa_sign_iop_get_num_ops(psa_sign_iop_t *iop);
psa_status_t psa_sign_iop_hash(psa_sign_iop_t *iop,
                               const uint8_t *hash,
                               size_t hash_length);
psa_sign_iop_t psa_sign_iop_operation_init(void);
psa_status_t psa_sign_iop_setup(psa_sign_iop_t *iop,
                                psa_key_id_t key,
                                psa_algorithm_t alg);
psa_status_t psa_sign_iop_setup_complete(psa_sign_iop_t *iop);
psa_status_t psa_sign_iop_update(psa_sign_iop_t *iop,
                                 const uint8_t *input,
                                 size_t input_length);

psa_status_t psa_verify_iop_abort(psa_verify_iop_t *iop);
psa_status_t psa_verify_iop_complete(psa_verify_iop_t *iop);
uint32_t psa_verify_iop_get_num_ops(psa_verify_iop_t *iop);
psa_status_t psa_verify_iop_hash(psa_verify_iop_t *iop,
                                 const uint8_t *hash,
                                 size_t hash_length);
psa_verify_iop_t psa_verify_iop_operation_init(void);
psa_status_t psa_verify_iop_setup(psa_verify_iop_t *iop,
                                  psa_key_id_t key,
                                  psa_algorithm_t alg,
                                  const uint8_t *signature,
                                  size_t signature_length);
psa_status_t psa_verify_iop_setup_complete(psa_verify_iop_t *iop);
psa_status_t psa_verify_iop_update(psa_verify_iop_t *iop,
                                   const uint8_t *input,
                                   size_t input_length);

/* Key Generation ---------------------------------------*/

typedef /* implementation-defined */ psa_generate_key_iop_t;

#define PSA_GENERATE_KEY_IOP_INIT /* implementation-defined */

psa_generate_key_iop_t psa_generate_key_iop_operation_init();

psa_status_t psa_generate_key_iop_setup(psa_generate_key_iop_t *iop,
                                        const psa_key_attributes_t * attr);
psa_status_t psa_generate_key_iop_complete(psa_generate_key_iop_t *iop,
                                           psa_key_id_t *key);
psa_status_t psa_generate_key_iop_abort(psa_generate_key_iop_t *iop);

/* Key Agreement ----------------------------------------*/

typedef /* implementation-defined */ psa_key_agreement_iop_t;

#define PSA_GENERATE_KEY_IOP_INIT /* implementation-defined */

psa_generate_key_iop_t psa_key_agreement_iop_operation_init();

psa_status_t psa_key_agreement_iop_setup(psa_key_agreement_iop_t *iop,
                                         const psa_key_attributes_t *attr,
                                         psa_key_id_t private_key,
                                         const uint8_t *peer_key,
                                         size_t peer_key_length);
psa_status_t psa_key_agrement_iop_complete(psa_key_agreement_iop_t *iop,
                                           psa_key_id_t *key);
psa_status_t psa_key_agreement_iop_abort(psa_key_agreement_iop_t *iop);

/* Key Derivation ----------------------------------------*/

/* Can use either psa_kdf_iop_* or psa_derive_key_iop_* here, but we do not
 * need to solidify this interface as yet. (However it does fit in the 31
 * character limit) */

I am admittedly presuming here that we are using pure key agreement here, not chained with a derivation as the new interface seems to do.

@athoelke
Copy link
Contributor Author

athoelke commented Jun 6, 2024

I am admittedly presuming here that we are using pure key agreement here, not chained with a derivation as the new interface seems to do.

Yes, chaining is easy to do with an output key - just use that as the input key for the kdf.

@athoelke
Copy link
Contributor Author

athoelke commented Jun 6, 2024

I think we should remove _operation from the xxx_init() functions as well. The pattern in the API so far is that a data structure psa_foo_t has an init macro PSA_FOO_INIT and function psa_foo_init().

@athoelke
Copy link
Contributor Author

athoelke commented Jun 6, 2024

For key agreement, the psa_key_agreement_iop_setup() signature is not correct. It is missing the algorithm, and I think we should match the parameter ordering in the non-interruptible API:

psa_status_t psa_key_agreement(psa_key_id_t private_key,
                               const uint8_t * peer_key,
                               size_t peer_key_length,
                               psa_algorithm_t alg,
                               const psa_key_attributes_t * attributes,
                               psa_key_id_t * key);

So perhaps:

psa_status_t psa_key_agreement_iop_setup(psa_key_agreement_iop_t * iop,
                               psa_key_id_t private_key,
                               const uint8_t * peer_key,
                               size_t peer_key_length,
                               psa_algorithm_t alg,
                               const psa_key_attributes_t * attributes);

However, I can't quite decide if maybe the key attributes could be passed in to psa_key_agreement_iop_complete() instead? I think the complex computation is the construction of the shared secret, which for key agreement algorithms is independent of the key type used to extract the result. The implementation might only need to inspect the output key attributes on the very last call to psa_key_agreement_iop_complete().

@paul-elliott-arm
Copy link
Contributor

I think we should remove _operation from the xxx_init() functions as well. The pattern in the API so far is that a data structure psa_foo_t has an init macro PSA_FOO_INIT and function psa_foo_init().

Doh, yes, I agree, copypasta'd myself.

* Operation objects are now psa_xxx_iop_t, initialization PSA_XXX_IOP_INIT etc.
* Operation and support functions use 'iop' as an infix instead of 'interruptible'
Require that one of psa_xxx_iop_hash() or psa_xxx_iop_update() MUST be called in an interruptible signature operation.
@athoelke
Copy link
Contributor Author

I've updated the PR with the revised naming scheme, and tidied up a couple of loose ends.

This might be just about ready?

I note that the introduction of the simpler interruptible operations (see#198) for key generation, key export, and key agreement suggests that the current overview of interruptible operations (in the Functionality chapter) portrays the worst-case complexity of interruptible signature. I think it would be better to use the simpler iops to demonstrate the pattern, and indicate how more complex operations can use the setup/complete pattern multiple times?

@athoelke
Copy link
Contributor Author

I think it would be better to use the simpler iops to demonstrate the pattern, and indicate how more complex operations can use the setup/complete pattern multiple times?

I am happy to address that in a separate PR with the additional interruptible APIs, rather than extend/prolong this already-substantial PR.

Copy link
Contributor

@MarcusJGStreets MarcusJGStreets left a comment

Choose a reason for hiding this comment

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

Looks fine to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API design Related the design of the API Crypto API Issue or PR related to the Cryptography API enhancement New feature or request
Projects
Development

Successfully merging this pull request may close these issues.

Interruptible/bounded-latency asymmetric operations
4 participants