-
Notifications
You must be signed in to change notification settings - Fork 5
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
Review corrections for KeepassXC support #71
Conversation
This PKCS#7 padding version is possibly incompatible with Yubikey's PKCS#7 version, as it expects the last byte to always be the padding byte value. See KeepassXC implementation comments for the details. Everything works with the challenge length up to 63 bytes though, and YK implementation would not handle more anyway, hence accepting this potential incompatibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't look to me like the Kind/Algorithm matching is actually checked?
CredentialFlat::try_from does not appear to check the max lengh of the secret, or that the algorithm is Sha1:
CredentialData::HmacData(data) => {
cred.kind = Kind::Hmac;
cred.secret = ShortData::from_slice(data.secret)?;
cred.algorithm = data.algorithm;
}
CredentialFlat is meant to be a simple struct for data serialization. All checks should be done on construction in the upstream. The check for the secret length and Algo kind is done here: trussed-secrets-app/src/command.rs Lines 512 to 532 in 43ee41c
|
Ok, I missed it sorry. |
Review corrections for KeepassXC support #64.
In the end the unpadding crate was used without modifications - it does not matter in this case, as payload bigger than 63 bytes can't be used anyway.