-
Notifications
You must be signed in to change notification settings - Fork 2
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
Replace openssl hash functions with RustCrypto hashes #3
base: main
Are you sure you want to change the base?
Conversation
See RustCrypto/traits#1683 for more details on errors
} else if Oid::const_new(&[2, 16, 840, 1, 101, 3, 4, 2, 2]).eq(oid) { | ||
Ok(Box::new(sha2::Sha384::default())) | ||
} else if Oid::const_new(&[2, 16, 840, 1, 101, 3, 4, 2, 1]).eq(oid) { | ||
Ok(Box::new(sha2::Sha256::default())) |
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.
There might be a better way. But probably slower
https://github.com/RustCrypto/formats/blob/0bd9dea37e3d8cccdb5f7ba5a402f62553d07e05/cms/src/builder.rs#L1061-L1077
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.
Hey, thanks for the comment. It seems that with that approach, I would need to use an oid database (der::oid::db::DB
). Since oids do not change, I think it's better to have them hardcoded. Even though it looks cleaner, I would rather have very little dependency footprint. Currently, I have quite many dependencies but I plan to reduce them in time.
RustCrypto is missing brainpool curves, so the signature verification is not implemented still RSA and DSA are also not implemented
RustCrypto is missing Brainpool and some other elliptic curve algorithms (RustCrypto/elliptic-curves#114) |
// <https://www.icao.int/publications/Documents/9303_p12_cons_en.pdf> | ||
// > MUST be v3 | ||
if cert.tbs_certificate.version.ne(&rasn_pkix::Version::V3) { | ||
error!("DSC TBSCertificate version must be v3"); |
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.
s/DSC/certificate
)); | ||
let signature_algorithm = &signer_info.signature_algorithm; | ||
let signature = der::encode(&signer_info.signature).map_err(EmrtdError::RasnEncodeError)?; | ||
// let pub_key = dsc.public_key().map_err(EmrtdError::OpensslErrorStack)?; |
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.
Remove commented code
return Err(EmrtdError::VerifySignatureError( | ||
"Signature verification failure during Master List parsing", | ||
)); | ||
// let pub_key = master_list_signer |
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.
Remove commented code
// let chain = Stack::new().map_err(EmrtdError::OpensslErrorStack)?; | ||
// let mut store_bldr = | ||
// X509StoreBuilder::new().map_err(EmrtdError::OpensslErrorStack)?; | ||
// store_bldr |
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.
Remove commented code
// <https://www.icao.int/publications/Documents/9303_p12_cons_en.pdf> | ||
// > Dates through 2049 MUST be in UTCTime UTCTime MUST be represented as YYMMDDHHMMSSZ | ||
rasn_pkix::Time::Utc(date_time) => { | ||
// todo!() |
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.
easy todos
Aim is to remove openssl dependency eventually. See RustCrypto/traits#1683 for more details on where I am stuck right now.