Closed zmre closed 4 years ago
I would like to see this building (and testing?) on windows before we merge it. We could add that to Travis or do it as part of #111.
One thing that I didn't do that we should consider: check the return value from mlock
and munlock
. Different systems have different error codes, but across all of them, zero is success. I don't imagine the caller will care if it succeeds or not, but it would be something we could unit test for and maybe detect hidden issues when we cross compile. Thoughts?
Benchmark results comparing with base:
group base current
----- ---- -------
256-bit compute public key 1.02 1292.7±33.11µs ? B/sec 1.00 1263.5±37.18µs ? B/sec
256-bit decrypt (level 0) 1.05 11.3±0.26ms ? B/sec 1.00 10.7±0.26ms ? B/sec
256-bit decrypt (level 1) 1.08 41.2±1.24ms ? B/sec 1.00 38.2±1.09ms ? B/sec
256-bit decrypt (level 2) 1.04 69.1±1.63ms ? B/sec 1.00 66.4±2.00ms ? B/sec
256-bit derive symmetric key 1.00 3.5±0.39µs ? B/sec 1.65 5.8±0.55µs ? B/sec
256-bit generate ed25519 keypair 1.00 26.5±1.17µs ? B/sec 1.14 30.1±1.07µs ? B/sec
256-bit generate key pair 1.00 1275.7±22.38µs ? B/sec 1.02 1304.2±30.95µs ? B/sec
256-bit generate transform key 1.03 26.4±0.59ms ? B/sec 1.00 25.6±0.71ms ? B/sec
256-bit transform (level 1) 1.06 32.4±1.09ms ? B/sec 1.00 30.6±1.12ms ? B/sec
256-bit transform (level 2) 1.05 73.3±0.82ms ? B/sec 1.00 70.0±1.46ms ? B/sec
@coltfred will you rebase this on master, please?
see #107
These changes should prevent private keys from getting written to disk by the operating system. I went through and looked for any memory that we were zeroing out and added the protections there. All unit tests continue to work well.
Do we run unit tests in a Windows environment? If so, I'll attempt to add Windows support to this.
One ugly: in
memlock.rs
I have conditional compilation so this only happens on unix platforms, but there are empty no-op functions on non-unix. But I have to annotate each function with the conditional compilation. I initially usedmod memlock { ... }
and had two of those blocks each with an annotation. So then the file only had two of those, which was nice. But Clippy whined about inception. The use would beuse crate::internal::memlock::memlock
which is ugly. I couldn't find a way around this. Any ideas? I went for the thing I think is least ugly there.One other ugly: there's no way to call out to these functions without unsafe code. I don't believe it's dirty in this case; it just is what it is. We don't mutate the data values.