Add PIV smart card keyfile encryption #1451
Conversation
src/Common/Language.xml
Outdated
| <entry lang="en" key="IDM_REVEAL_REDKEY">Reveal Red Key...</entry> | ||
| <entry lang="en" key="REVEAL_REDKEY_INFO">This will produce decrypted keyfile which can be used without using the smart card, thus reducing security to two factors. This keyfile should be stored securely. Are you sure to continue?</entry> | ||
| <entry lang="en" key="REVEAL_REDKEY_DONE">Red key is saved</entry> | ||
| <entry lang="en" key="REVEAL_REDKEY_PATH">Choose file to save red key to</entry> |
There was a problem hiding this comment.
Addition of new entries must always be done at the bottom of the XML file to preserve position of existing entries.
src/Common/Language.xml
Outdated
| <entry lang="en" key="IDC_SECURITY_TOKEN_KEY">Security token key</entry> | ||
| <entry lang="en" key="IDT_SECURITY_TOKENS">Security Tokens...</entry> | ||
| <entry lang="en" key="TOKEN_SLOT_ID">Slot ID</entry> | ||
| <entry lang="en" key="SELECT_TOKEN_KEYS">Select token keys</entry> | ||
| <entry lang="en" key="TOKEN_NAME">Token name</entry> | ||
| <entry lang="en" key="TOKEN_KEY_LABEL">Key label</entry> |
There was a problem hiding this comment.
Same as above: new entries should be added at the bottom.
src/Common/SecurityToken.cpp
Outdated
| if (privateAttrib.size() == sizeof (CK_BBOOL) && *(CK_BBOOL *) &privateAttrib.front() != CK_TRUE) | ||
| continue; | ||
|
|
||
| GetObjectAttribute (slotId, dataHandle, CKA_MODULUS_BITS, privateAttrib); | ||
| if (privateAttrib.size() == sizeof (CK_ULONG)) { | ||
| CK_ULONG modulus = *(CK_ULONG *) &privateAttrib.front(); | ||
| key.maxDecryptBufferSize = modulus / 8 - 11; | ||
| key.maxEncryptBufferSize = modulus / 8; | ||
| } | ||
|
|
There was a problem hiding this comment.
This logic will fail for cards that contain ECC keys. Since the current implementation supports only RSA keys, the code should use CKA_KEY_TYPE to restrict logic to RSA keys only (CKK_RSA)
src/Common/SecurityToken.cpp
Outdated
| GetObjectAttribute (slotId, dataHandle, CKA_MODULUS_BITS, publicAttrib); | ||
| if (publicAttrib.size() == sizeof (CK_ULONG)) { | ||
| CK_ULONG modulus = *(CK_ULONG *) &publicAttrib.front(); | ||
| key.maxDecryptBufferSize = modulus / 8 - 11; | ||
| key.maxEncryptBufferSize = modulus / 8; | ||
| } | ||
|
|
There was a problem hiding this comment.
same as above: the code should restrict logic to RSA keys only.
src/Common/SecurityToken.cpp
Outdated
| CK_MECHANISM m = { CKM_RSA_PKCS, 0, 0 }; | ||
| CK_RV status = Pkcs11Functions->C_EncryptInit (Sessions[slotId].Handle, &m, tokenObject); |
There was a problem hiding this comment.
We should not use CKM_RSA_PKCS since it is insecure. We must use CKM_RSA_PKCS_OAEP with SHA256 MGF.
src/Common/SecurityToken.cpp
Outdated
| CK_MECHANISM m = { CKM_RSA_PKCS, 0, 0 }; | ||
| CK_RV status = Pkcs11Functions->C_DecryptInit (Sessions[slotId].Handle, &m, tokenObject); |
There was a problem hiding this comment.
Like above, we should not use CKM_RSA_PKCS since it is insecure. We must use CKM_RSA_PKCS_OAEP with SHA256 MGF.
|
@the-dan Thank you for this contribution. I will need some time to review the code changes because there are so many...this is the largest PR I have ever had to review. That being said, I have already provided some comments and will add more as I progress with the review. |
In addition to security token keys show compatible and approved security token mechanism. Together they will be used to create an encrypted keyfile
|
@idrassi I’m happy that this change aligns with VeraCrypt’s direction. Thanks for going through this! I haven’t touched the encryption part since the initial prototype started working, which is why plain RSA PKCS was used. Instead of selecting only RSA keys and locking in the mechanism, I revisited the approach to make it more flexible. For now, it supports only RSA OAEP, but I believe it could be extended in the future. Please take a look when you have time. Apologies for the large commits—this time, I also did a lot of renaming for clarity, which affected many files |
Add an option to create a keyfile which is encrypted using a smart card's private key (e.g. YubiKey)