Skip to content

Conversation

sbailey-arm
Copy link
Contributor

Added missing documentation for all single part operations (excluding copy - requires discussion on implementation for Parsec)

Also removed multi-part ops from single part ops table

Signed-off-by: Samuel Bailey <[email protected]>
Fixed verify hash typo

Signed-off-by: Samuel Bailey <[email protected]>
Copy link
Member

@hug-dev hug-dev left a comment

Choose a reason for hiding this comment

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

Thanks!! Everything look code for me, small comment about PurgeKey.

@@ -0,0 +1,48 @@
# PsaPurgeKey
Copy link
Member

Choose a reason for hiding this comment

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

Similarly to the comment on your PR on parsec-operations, I think it is best to remove it from now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🧹

@@ -37,7 +37,7 @@ such signature algorithm: Raw PKCS#1 v1.5 signature.
**Note:** To perform a hash-and-sign algorithm, the hash must be calculated before passing it to
this function. This could be done with the operation PsaHashCompute or with a multi-part hash
operation. Those operations are not yet implemented. Alternatively, to hash and sign a message in a
single call, you could use PsaSignMessage (not yet implemented).
single call, you could use PsaSignMessage.
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for that 👍

Comment on lines +113 to +116
| Algorithm | Rust client |
|-----------|-------------|
| Hmac | ❌ |
| CbcMac | ❌ |
| Cmac | ❌ |
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For things like these algorithms, where the algorithms are supported, but the operations that use them are not, should they be ticks instead?

Copy link
Member

Choose a reason for hiding this comment

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

I would say no personally. I think this page could be presented in a better way. The first table kind of assume that the operation is fully supported when we only really support one specific algorithm.
Ideally, we would have one massive matrix of combination of providers X combination of operations X combinations of their input parameters.

Copy link
Member

Choose a reason for hiding this comment

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

And now that all single-part operations are defined, we could think of generating such table programatically, nightly, by either defining new operations that would give us the elvel of support for each operation OR by making test for each one of them. That could be a cool internship project 😉

@sbailey-arm sbailey-arm force-pushed the add-final-single-part-ops branch 2 times, most recently from cd2d6ed to 20250b6 Compare August 26, 2020 16:55
@sbailey-arm
Copy link
Contributor Author

This should pass CI now the new protobuf contracts are in master.

Copy link
Member

@hug-dev hug-dev left a comment

Choose a reason for hiding this comment

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

Looks good to me, thank!
The link checker is palying us tricks. I think it's worth investigating why it fails, or using another one. Maybe the GitHub links do some kind of weird redirection that it does not support?
In any case, happy to merge this even if hte CI fails.

Copy link
Member

@ionut-arm ionut-arm left a comment

Choose a reason for hiding this comment

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

Thanks!! That is a lot of changes 😮

@@ -0,0 +1,36 @@
# PsaSignMessage

Sign a message with a private key. Opcode: 23 (`0x0017`)
Copy link
Member

Choose a reason for hiding this comment

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

PsaMacVerify and this one have the same opcode!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👀

| Cfb | ❌ | ❌ | ❌ |
| Ofb | ❌ | ❌ | ❌ |
| Xts | ❌ | ❌ | ❌ |
| EbcNoPadding | ❌ | ❌ | ❌ |
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
| EbcNoPadding ||||
| EcbNoPadding ||||

Copy link
Member

Choose a reason for hiding this comment

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

This one and the one below also apply to the client coverage page

| Algorithm | Mbed Crypto provider | PKCS 11 provider | TPM 2.0 provider |
|--------------|----------------------|------------------|------------------|
| StreamCipher | ❌ | ❌ | ❌ |
| Crt | ❌ | ❌ | ❌ |
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
| Crt ||||
| Ctr ||||

@sbailey-arm sbailey-arm force-pushed the add-final-single-part-ops branch 3 times, most recently from 74778e0 to 39ef2af Compare August 28, 2020 11:30
@sbailey-arm sbailey-arm force-pushed the add-final-single-part-ops branch from 39ef2af to 1e1cd61 Compare August 28, 2020 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants