Compatibility with rust-ece breaking API changes #3941
Conversation
rfk
force-pushed
the
ece-interface-cleanups
branch
5 times, most recently
from
f2a90c1 to
1c66e5b
Compare
Codecov Report
@@ Coverage Diff @@
## main #3941 +/- ##
=====================================
Coverage 0.00% 0.00%
=====================================
Files 177 177
Lines 12142 12135 -7
=====================================
+ Misses 12142 12135 -7
Continue to review full report at Codecov.
|
rfk
changed the title
rfk
force-pushed
the
ece-interface-cleanups
branch
from
1c66e5b to
a073394
Compare
There was a problem hiding this comment.
@lougeniaC64 could you please take a look at these corresponding changes on the consumer side of the rust-ece crate?
| @@ -468,7 +468,7 @@ mod test { | |||
| .with_body(body) | |||
| .create(); | |||
| let mut conn = connect(config.clone(), None, None).unwrap(); | |||
| let channel_id = hex::encode(crate::crypto::get_bytes(16).unwrap()); | |||
| let channel_id = hex::encode(crate::crypto::get_random_bytes(16).unwrap()); | |||
There was a problem hiding this comment.
Sorry, I just, I couldn't handle the name of this function - what sort of bytes are we getting??
| @@ -50,14 +48,12 @@ impl Key { | |||
| } | |||
| } | |||
|
|
|||
| pub fn key_pair(&self) -> error::Result<RcCryptoLocalKeyPair> { | |||
| RcCryptoLocalKeyPair::from_raw_components(&self.p256key).map_err(|e| { | |||
There was a problem hiding this comment.
I think we were accidentally biting ourselves here with a bit of bad API design, which meant that we couldn't use the aesgcm scheme without knowing which specific crypto backend was in use by the rust-ece crate. That has been fixed now (thanks to an external contributor! ref mozilla/rust-ece#47) so we can remove quite a bit of key parsing-and-re-serializing here.
| let mut auth_secret = vec![0u8; 16]; | ||
| let mut salt = vec![0u8; 16]; | ||
| rc_crypto::rand::fill(&mut auth_secret).unwrap(); | ||
| rc_crypto::rand::fill(&mut salt).unwrap(); |
There was a problem hiding this comment.
Not letting you pass in this salt, was the main breaking API change. The latest version of rust-ece just makes one for you automatically.
| @@ -178,260 +178,10 @@ pub(crate) fn init() { | |||
| #[cfg(test)] | |||
| mod tests { | |||
| use super::*; | |||
| use ece::*; | |||
|
|
|||
| // Copy-pasta from the tests in the ECE crate. | |||
There was a problem hiding this comment.
Pretty happy to dump this copy-pasta...
rfk
force-pushed
the
ece-interface-cleanups
branch
from
a073394 to
bc9567d
Compare
| @@ -188,7 +184,6 @@ impl Cryptography for Crypto { | |||
| } | |||
| } | |||
|
|
|||
| // IIUC: objects created on one side of FFI can't be freed on the other side, so we have to use references (or clone) | |||
There was a problem hiding this comment.
I wasn't able to map what this comment is saying to anything happening in the code below it, so I presumptuously deleted it.
rfk
force-pushed
the
ece-interface-cleanups
branch
from
bc9567d to
01568ec
Compare
The rust-ece crate has had its interface simplified to remove footguns, let's update to that release and get a few cleanups of our own while we're at it.
rfk
force-pushed
the
ece-interface-cleanups
branch
from
01568ec to
d4457b2
Compare
WIP updates for compatibility with the work I'm doing over in mozilla/rust-ece#52The rust-ece crate has had its interface simplified to remove footguns, let's update to that release and get a few cleanups of our own while we're at it.