latchset / kryoptic

a pkcs#11 software token written in Rust
GNU General Public License v3.0
10 stars 4 forks source link

This is the next step in changing how storage works #62

Closed simo5 closed 3 months ago

simo5 commented 3 months ago

As a shortcut the initial implementation on PIN handling was done by storing plain text PINs in objects.

This PR changes completely how PINs are handled and how information about their status is exposed in the token.

The SO PIN is only used for initialization operations, and is stored as the result of a PBKDF2 operation with 10000 iterations.

The USER PIN uses the same derivation, but produces an AES key that is used to wrap a randomly generated other AES key (KEK) using AES GCM (so that the wrapped key is authenticated upon decryption).

~The KEK is currently not used, but this is the ground work to change the storage layer to encrypt all sensitive values before storing them.~ The KEK is used to encrypt all the attributes marked as Sensitive for all object types.

If the SO Pin is used to change the User PIN the KEK will be lost and the all the secrets in the Database will become unusable. In the future we may want to consider whether the KEK should be also encrypted in the SO PIN as a recovery option, but currently this is not done.

simo5 commented 3 months ago

Fixed the broken commit and rebase on main

simo5 commented 3 months ago

With the encryption, the testsuite runs waaay slower.

I did not notice (to my surprise) a significant slow down, can you quantify what you see and in what conditions?

Jakuje commented 3 months ago

Just rough numbers on my machine show its 10x slower:

[jjelen@t14s kryoptic (storage3)]$ make check
test result: ok. 190 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 25.06s
[jjelen@t14s kryoptic (main)]$ make check
test result: ok. 184 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 2.60s
simo5 commented 3 months ago

Just rough numbers on my machine show its 10x slower:

[jjelen@t14s kryoptic (storage3)]$ make check
test result: ok. 190 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 25.06s
[jjelen@t14s kryoptic (main)]$ make check
test result: ok. 184 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 2.60s

Yeah, I just realized my machine takes 22s now ... I will see if I can add a no-encryption mode, so we can use it for most tests

simo5 commented 3 months ago

Turned out 90% of the slow down was not due to encryption but due to the pbkdf to unlock the pin.

With encryption turned off I reduced local testing time from ~28s to ~23s, with pbkdf2 rounds brought down from 10000 to 1000 I got from ~23s to ~2.3s ...

simo5 commented 3 months ago

I addressed the requested changes and added commits to bring down the time in testing to: test result: ok. 190 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 5.38s

simo5 commented 3 months ago

Thanks for the review, I am too lazy to go through another round just for some minor wording, so merging as is :-)