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 capabilities discovery operations #426

Closed
hug-dev opened this issue May 14, 2021 · 15 comments
Closed

Add capabilities discovery operations #426

hug-dev opened this issue May 14, 2021 · 15 comments
Assignees
Labels
good first issue Good for newcomers large Effort label

Comments

@hug-dev
Copy link
Member

hug-dev commented May 14, 2021

See #388 for context.

The operations are described in this comment as GetSupportedAlgorithms and GetSupportedKeyAttributes.

When this issue is picked up, an initial refresher is needed to see if those two operations are still good as they are and still needed. Then the implementation can go through the Parsec stack.

These will allow clients to choose from a pool of allowed key parameters when generating/importing keys and a pool of algorithms when doing operations with the keys.

@hug-dev hug-dev added good first issue Good for newcomers large Effort label labels May 14, 2021
@hug-dev
Copy link
Member Author

hug-dev commented Jul 7, 2021

Brain dump of this

@hug-dev
Copy link
Member Author

hug-dev commented Jul 7, 2021

Also check ideas here

@Kakemone Kakemone self-assigned this Jul 12, 2021
@hug-dev
Copy link
Member Author

hug-dev commented Jul 21, 2021

I think this is fine to first go for GetSupportedAlgorithms and only start with one provider, let's say the PKCS11 one. We can then add GetSupportedKeyAttributes and then implement for other providers, little by little.

For GetSupportedAlgorithms, I think there are two things to look at:

  • the set of algorithms supported by the PKCS11 provider. This is mostly all the algorithms in the TryFrom implementation. This set should reflect the algorithms support on the operations coverage document. Some algorithms contain values, like a hash algorithm. For those we can see what to do:
    • expand all of them: show all algorithms with all of their values. That might create a huge payload to return.
    • use a "dummy" value and document what it means. For example we could take the assumption that if a Hash algorithm is supported, then it is supported for all operations where it is in.
  • the set of algorithms supported by the PKCS11 token (the actual physical device). This must be fetched dynamically at runtime, through PKCS11 operations (C_GetMechanismList, C_GetMechanismInfo).

The result would be the intersection of those two sets.

A good test would be to make sure that all algorithms returned by Get_SupportedAlgorithms can be used successfully.

@Kakemone
Copy link
Contributor

Potential Design Overview

This covers the code changes needed to implement GetSupportedAlgorithms for PKCS11

Motivation

The reason this operation is needed is covered in detail in Hugues' post but concisely this will improve the stability of parsec by telling the clients what algorithms are supported rather than the client just trying algorithms that if the client is newer than the service could not be supported.

Plan Overview

My plan for GetSupportedAlgorithms is to set it up with all of the stuff required for a parsec operation (protobuf contract ect...) and then write a new function which takes parameters of types MechanismInfo and MechanismType and returns the corresponding algorithm. Using get_mechanism_list and get_mechanism_info I can create two lists of inputs for the new function which should correspond to the supported algorithms and so the new function will produce the required list of algorithms which are supported by PKCS11

parsec-book

As GetSupportedAlgorithms is a new operation it will need a page added to the parsec book. The contents of the page will be similar to:

GetSupportedAlgorithms

Get a set of supported algorithms by the provider. Opcode: 29 (0x001D)

Parameters

No parameters are needed for this operation.

Results

Name Type Description
algorithms Vector of Algorithm Vector of supported Algorithm values

Specific response status codes

No specific response status codes returned.

Description

This operation returns the algorithms supported by the provider. The exact values returned can be used directly on the specific operations where they are needed.

Additionally a line would need to be added to the Parsec Operations Coverage table to show that the operation is currently only supported on PKCS11

parsec-operations

As GetSupportedAlgorithms is a new operation it will need a protobuf contract which will be:

/*
 * Copyright 2021 Contributors to the Parsec project.
 * SPDX-License-Identifier: Apache-2.0
 */
syntax = "proto3";

package get_supported_algorithms;

import "psa_algorithm.proto";

message Operation {}

message Result {
  repeated psa_algorithm.Algorithm algorithms = 1;
}

parsec-interface-rs

operations_protobuf

Create convert_get_supported_algorithms.rs which contains the implementations for try_from between the operation from the crate and the operation from the protobuf contract (and vice versa) and similarly for result.

Add include_protobuf_as_module!(get_supported_algorithms); and empty_clear_message!(get_supported_algorithms::Operation); empty_clear_message!(get_supported_algorithms::Result); to generated_ops.rs as it will not contain sensitive data and so can use a no-op zeroize

Add mod convert_get_supported_algorithms; and use generated_ops::get_supported_algorithms as get_supported_algorithms_proto; to mod.rs and add relevant pairings to the match statements in body_to_operation, operation_to_body, body_to_result and result_to_body

requests

Add the opcode for GetSupportedAlgorithms to mod.rs and add the opcode to the list of false pairings in the match statement in is_core and is_admin

rust-cryptoki

Make the new function which converts from MechanismInfo and MechanismType to Algorithm. There is a similar function already made which converts Algorithm to Mechanism most of the mappings in the match statement for this function are one algorithm to one mechanism type however there is one where RsaPkcs1v15Sign and RsaPkcs1v15Crypt both map to Mechanism::RsaPkcs so the new function would use the flags in MechanismInfo to see if signing and/or encryption is supported and return the relevant Algorithm

parsec

Add GetSupportedAlgorithms to execute_request within backend_handler.rs

Add get_supported_algorithms to the implementation of Provide for Provider

Add get_supported_algorithms to relevant file in pkcs11 folder (not sure which one this should go in) which contains the calling of get_mechanism_list, get_mechanism_info and the new function

Testing

The output of this operation should be a list of algorithms and each algorithm in the list should be able to be used as an input for the relevant operations so a test could be checking if they are all supported by either making a function to check or using them as inputs for the relevant operations and seeing if it produces an error.
The operation should also return all the supported algorithms so there should be a test to see if the output is complete rather than just a subset of the desired result but I am unsure how to test this.

@hug-dev
Copy link
Member Author

hug-dev commented Jul 22, 2021

Thank you for the write-up! Some comments on my side:

No parameters are needed for this operation.

We should discuss if we either make this operation a core operation, in which case it should always be directed to the core provider and would take as a parameter the provider ID to request the supported algorithms from (like ListOpcodes for example). Maybe that would make more sense as it's a similar kind of operation than the other List... ones.

This operation returns the algorithms supported by the provider. The exact values returned can be used directly on the specific operations where they are needed.

I think we can start with that but do some tests/checks to see if that wouldn't make the response too big. For example if you have to return the algorithms with all of the hashes they support it might be a lot. I think it should be ok.

Testing

You might also need to implement this operation in the BasicClient so that the test client can use it.

The operation should also return all the supported algorithms so there should be a test to see if the output is complete rather than just a subset of the desired result but I am unsure how to test this.

Good point! For our tests, we can manually find what the supported algorithms are in SoftHSM, which is the pkcs11 implementation we use for tests.

edit: could be nice to have the command available in the Parsec Tool as well! Those kind of oeprations are very useful to have on the command line.

@ionut-arm
Copy link
Member

For example if you have to return the algorithms with all of the hashes they support it might be a lot. I think it should be ok.

Because of this I was wondering if it wouldn't make more sense to turn that query around: what if the client sent a list of fully-defined algorithms it wants to check for support and the service replies with a "filtered" list with only the supported ones. That way you avoid having to expand all the nested algorithms and the client gets what they want. Even with the current design you'll still have to do something similar - ask for the list from the service and then you as a client have your own list of supported algorithms and you compare and take the top one in your preferences. This also has the benefit of not having to implement the search for every client as well (though it's probably not that big of a deal in most languages).

@hug-dev
Copy link
Member Author

hug-dev commented Jul 23, 2021

Yes I think that's a pretty valid idea as well! Clients could then only send the algorithm in an area they are just interested in (signing or encryption for example).

@paulhowardarm
Copy link
Collaborator

PSA were looking at adding capability discovery stuff at one time, but it seems that there is no active work directly in this area right now. The closest thing that is being worked on is a structured data format for driver capabilities, but I'm not sure how directly relevant that is: https://github.com/ARMmbed/mbedtls/blob/development/docs/proposed/psa-driver-interface.md#driver-description-syntax

There may be an opportunity to cross-pollinate what we are doing with PSA.

Let's make sure we're thinking about what kind of client dev experiences we want to enable, rather than just design individual opcode contracts. How does this feed into smart defaulting, for instance? Are there going to be any kinds of preferential ordering semantics to the data being transacted over this API?

It's absolutely fine to implement just in PKCS#11 for now, but let's make sure we have some kind of story (even if only on paper) for how these contracts would be back-ended to the other providers.

@hug-dev
Copy link
Member Author

hug-dev commented Aug 10, 2021

Another idea of a capability discovery operation, linked with a proposal made on the PSA Crypto API. I am thinking of two general client use-cases for capability profiling. For any of the following:

  • using an algorithm with a pre-provisioned key
  • generating a key
  • importing a key
  • deriving a key

clients want to:

  1. check if one of the above will be supported for their own constrains of key type/algorithms/key sizes
  2. for one of the above, list the "best" (in terms of security/performance/power) options possible (key type/algorithms/key sizes)

This operation is based on the fact that the permitted_algorithm field of the key attributes can be directly used to inform on what algorithm is going to be used with the key. But this field can also be set to None.

The operation contract is noted below.
For 1, clients will call the operation repeatedly with their own constraints and check if any of them is supported. If multiple providers are present, they could check them for each provider. If Parsec can not support their use case, they can potentially defer to a software-based solution.
For 2, clients need to make lists of attributes to check and search through them the best one supported. Note that if their lists are ordered, they can perform binary search to reduce the complexity.

A possible improvement would make it take as input a vector of KeyAttributes to reduce the number of operations needed when there is a large number of operations to check. Then we will have to think on the meaning of check_type: is it for all of the attributes given? Should we have one per attribute? Also then the response needs to indicate which of the given attribute and use are supported or not. The response could be a list of support KeyAttributes as the original design or be a vector of boolean.

Notes:

  • that this does not cover key-less operations (like hashing): it could be via Use and any key type/key length?
  • not sure if the key_policy.usage_flags will be needed at all

CanDoCrypto

Check if the provider supports:

  • using a specific algorithm with an existing key
  • generating a key and optionally using it for a specific algorithm
  • importing a key and optionally using it for a specific algorithm
  • deriving a key and optionally using it for a specific algorithm (to be checked)

Parameters

Name Type Description
check_type CheckType Type of the check performed
attributes KeyAttributes Value to be checked

CheckType type

A CheckType type can contain one of the following:

  • Use
  • Generate
  • Import
  • Derive

Results

No values are returned by this operation.

Specific response status codes

  • Success: the check is successfull
  • PsaErrorNotSupported: the check failed (not supported)

Description

The meaning of the operation depends of the value of check_type:

  • Use: the operation checks if an existing key of the same key type than in the attributes and the same length can be used to perform the algorithm in key_policy.key_algorithm
  • Generate: checks if a key with the same attributes can be generated. If the key_algorithm is not None, also perform the Use check. If the key_bits is 0, check for a key of any size.
  • Import: checks if a key with the same attributes can be imported. If the key_algorithm is not None, also perform the Use check. If the key_bits is 0, check for a key of any size.
  • Derive: checks if a key with the same attributes can be derived. If the key_algorithm is not None, also perform the Use check. If the key_bits is 0, check for a key of any size.

@hug-dev
Copy link
Member Author

hug-dev commented Aug 10, 2021

An example for use case 1 with this new operation could be shown in our end-to-end tests. That would allow to remove the compile-time cfg gates and dynamically check for support:

diff --git a/e2e_tests/tests/per_provider/normal_tests/import_key.rs b/e2e_tests/tests/per_provider/normal_tests/import_key.rs
index 8f0989a..2ae398b 100644
--- a/e2e_tests/tests/per_provider/normal_tests/import_key.rs
+++ b/e2e_tests/tests/per_provider/normal_tests/import_key.rs
@@ -99,12 +99,40 @@ fn import_rsa_key() -> Result<()> {
     client.import_rsa_public_key(key_name, KEY_DATA.to_vec())
 }

-#[cfg(not(feature = "tpm-provider"))]
 #[test]
 fn import_ecc_key() {
     let mut client = TestClient::new();
     let key_name = String::from("import_key_ecc");
-    if !client.is_operation_supported(Opcode::PsaImportKey) {
+
+    let attributes = Attributes {
+        lifetime: Lifetime::Persistent,
+        key_type: Type::EccPublicKey {
+            curve_family: EccFamily::SecpR1,
+        },
+        bits: 256,
+        policy: Policy {
+            usage_flags: UsageFlags {
+                sign_hash: false,
+                verify_hash: true,
+                sign_message: false,
+                verify_message: true,
+                export: false,
+                encrypt: false,
+                decrypt: false,
+                cache: false,
+                copy: false,
+                derive: false,
+            },
+            permitted_algorithms: AsymmetricSignature::Ecdsa {
+                hash_alg: Hash::Sha256.into(),
+            }
+            .into(),
+        },
+    };
+
+    if !client.can_do(CheckType::Import, attributes)
+        || !client.is_operation_supported(Opcode::PsaVerifyHash)
+    {
         return;
     }

@Kakemone
Copy link
Contributor

Potential Design Overview

This covers the code changes needed to implement the CanDoCrypto operation for PKCS11 (Most of the changes will be the same for all providers but the checks on the permitted_algorithms will needed to be adjusted depending on the provider)

Motivation

The reason this operation is needed is covered in detail in Hugues' post but concisely this will improve the stability of parsec by telling the clients whether a certain set of attributes will be supported for a given type of operation

Plan Overview

My plan for CanDoCrypto is add all of the standard requirements for a parsec operation (protobuf contract ect...) and then add the required functionality in the PKCS11 part of parsec.
Initially I will only add capability for RSA keys as they have the simplest checks and I will add the others once the rest of the functionality is written.

parsec-book

As CanDoCrypto is a new operation it will need a page added to the parsec book. This page along with the relevant changes to other pages in the book has already been merged into the parsec-book code (parallaxsecond/parsec-book#129).

parsec-operations

As CanDoCrypto is a new operation it will need a protobuf contract which has also already been written and submitted as a PR (parallaxsecond/parsec-operations#35).

parsec-interface-rs

operations_protobuf

Create convert_can_do_crypto.rs which contains the implementations for try_from between the operation from the crate and the operation from the protobuf contract (and vice versa) and similarly for result.

Add include_protobuf_as_module!(can_do_crypto); and empty_clear_message!(can_do_crypto::Operation); empty_clear_message!(can_do_crypto::Result); to generated_ops.rs as it will not contain sensitive data and so can use a no-op zeroize

Add mod convert_can_do_crypto; and use generated_ops::can_do_crypto as can_do_crypto_proto; to mod.rs and add relevant pairings to the match statements in body_to_operation, operation_to_body, body_to_result and result_to_body

requests

Add the opcode for CanDoCrypto to mod.rs and add the opcode to the list of false pairings in the match statement in is_core and is_admin

parsec

Add CanDoCrypto to execute_request within backend_handler.rs

Add can_do_crypto to the implementation of Provide for Provider

Add can_do_crypto to relevant file in pkcs11 folder (not sure which one this should go in). This should contain the checks on the attributes.

The Checks

To check the key_algorithm within key_policy I will use the conversion function from Algorithm to Mechanism and compare that against the list of supported mechanisms as given by get_mechanism_list. I will also use get_mechanism_info to check that the relevent flags are set for the type of check that is being done (Use, Generate ... ). The mechanism_info also contains the min and max key size for the algorithm so I will check that against key_bits.

I will check the usage_flags to see if they are set to allow the type of operation that is being checked.

I will check the key_type to see if it is an RSA key and only allow them (When I add other key types I will also have to check that the key_bits fit the rules that some key_types have such as must be a multiple of 8 or must be one of a certain set of numbers.

Testing

This operation has no output but rather gives a status code depending on whether the attributes are supported. This shouldn't be too hard to check as the tests can consist of many different attribute combinations which cover the possible cases.

I hope this covers everything. If I have missed anything then or just explained something badly then I apologise in advance and will try to give a more complete explaination.

@hug-dev
Copy link
Member Author

hug-dev commented Aug 24, 2021

Thanks for the updated design!

not sure which one this should go in

I think you can put it in a new capability_discovery.rs file to match the section in the book.

To check the key_algorithm within key_policy I will use the conversion function from Algorithm to Mechanism and compare that against the list of supported mechanisms as given by get_mechanism_list.

Sounds good! If the key_algorithm is None, then no checks is needed there (this should be valid only for generate, import and derive). This check will have to be done in all cases.

I will also use get_mechanism_info to check that the relevent flags are set for the type of check that is being done (Use, Generate ... ).

What do you mean? For the other checks than Use, you will have to check if the implementation allows to generate, import or derive a key of that type. For example for PKCS11, for RSA key pairs and for Generate, I think can be done by checking if the mechanism CKM_RSA_PKCS_KEY_PAIR_GEN is there. Then you can also have access to the min and max key sizes.

@Kakemone
Copy link
Contributor

What do you mean? For the other checks than Use, you will have to check if the implementation allows to generate, import or derive a key of that type. For example for PKCS11, for RSA key pairs and for Generate, I think can be done by checking if the mechanism CKM_RSA_PKCS_KEY_PAIR_GEN is there. Then you can also have access to the min and max key sizes

The mechanism info contains a set of flags which indicate whether the mechanism can be used for a given process so I was trying to say that I would check those but I shouldn't have listed Use as an example as I don't believe that any of the flags are relevant in this case.

@ionut-arm
Copy link
Member

ionut-arm commented Sep 28, 2021

A few things that I noticed in https://github.com/parallaxsecond/parsec/pull/522/files :

  • methods on types can be called directly on self, so for example Provider::import_check(&self, attributes) can be replaced with self.import_check(attributes).

  • use_check is partially implemented in can_do_crypto_internal - the match arm that covers the use verification does a check before calling the method, which could be moved inside the method

  • is the last return from can_do_crypto_internal needed if the match before it is exhaustive?

@ionut-arm
Copy link
Member

ionut-arm commented Oct 15, 2021

Closing this issue as the initial work of adding the operations into the service has been done. Will open individual issues for the remaining work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers large Effort label
Projects
None yet
Development

No branches or pull requests

4 participants