Added export_key #40
Added export_key #40
Conversation
|
Are you quietly fixing a bug in |
I've been rumbled! I'm not sure it is actually a bug. We had a discussion about it a while ago, I think the result was 'it doesn't really matter' but returning the error for closing the key handle before the error for the actual operation doesn't seem right to me. Key handles should be going away at some point anyway. |
|
Might be good to add something like the following to the log message, then: Also, minor improvement to export_public_key(): Return operation error in preference to close error. |
| /// let size = key_management::export_public(my_key, &mut data).unwrap(); | ||
| /// data.resize(size, 0); | ||
| /// ``` | ||
| pub fn export_pair(key: Id, data: &mut [u8]) -> Result<usize> { |
There was a problem hiding this comment.
I think the name here should match the PSA Crypto one: export_key
There was a problem hiding this comment.
Ah yes, I did that before reading the description, doh. Fixed.
sbailey-arm
force-pushed
the
add-export-key
branch
from
b4242f9
to
1bd5ce9
Compare
I've added that to the commit message. |
sbailey-arm
force-pushed
the
add-export-key
branch
2 times, most recently
from
37bb2d3
to
6c34628
Compare
| @@ -78,9 +78,45 @@ fn import_integration_test() { | |||
| } | |||
| } | |||
|
|
|||
| #[test] | |||
| fn export_key_pair_test() { | |||
There was a problem hiding this comment.
Would it be possible to have a test where you put a previously generated key file in the right place and export that key? It would help with more accurate testing (instead of just making sure you get something on the other side). Would also help with testing in Parsec, as we won't have to be that thorough when testing there.
There was a problem hiding this comment.
I guess another option is to import and export a "static" key (i.e. a byte vector).
There was a problem hiding this comment.
You mean like has been done in Parsec? Yes I can add that test here. I wasn't sure what kind of/how many tests are added to psa-crypto as there are only generate and import tests there currently.
There was a problem hiding this comment.
Yes, I think it makes more sense to put the tests at the lowest level where they apply - then neither Parsec nor other users of the crate need to test for that
There was a problem hiding this comment.
Ah ok, good point. I've used the PK from Parsec's test, imported it, exported it, and verified the original an exported values match.
Also, minor improvement to export_public_key(): Return operation error in preference to close error. Signed-off-by: Samuel Bailey <[email protected]>
sbailey-arm
force-pushed
the
add-export-key
branch
from
6c34628
to
dcea030
Compare
Added export_key() along with a simple test.
Signed-off-by: Samuel Bailey [email protected]