Insecure time through rdtscp
#648
Conversation
raoulstrackx
force-pushed
the
raoul/rte-204-insecure_time_through_rdtscp
branch
from
a9025ee to
f5643ac
Compare
insecure-time/src/lib.rs
Outdated
| pub fn from_duration(duration: Duration, freq: &Freq) -> Self { | ||
| let freq = freq.as_u64(); | ||
| let ticks_secs = duration.as_secs() * freq; | ||
| let ticks_nsecs = duration.subsec_nanos() as u64 * freq / NANOS_PER_SEC as u64; |
There was a problem hiding this comment.
consider using subsec_millis
| let time = time::SystemTime::now() | ||
| .duration_since(time::UNIX_EPOCH) | ||
| .unwrap(); | ||
| (time.subsec_nanos() as u64) + time.as_secs() * 1_000_000_000 | ||
| let t = (time.subsec_nanos() as u64) + time.as_secs() * 1_000_000_000; |
There was a problem hiding this comment.
Use a constant as before NANOS_PER_SECOND
| /// # Miscellaneous | ||
| impl Usercalls { | ||
| /// This returns the number of nanoseconds since midnight UTC on January 1, | ||
| /// 1970\. The enclave must not rely on the accuracy of this time for | ||
| /// security purposes, such as checking credential expiry or preventing | ||
| /// rollback. | ||
| pub fn insecure_time() -> u64 { unimplemented!() } | ||
| pub fn insecure_time() -> (u64, *const InsecureTimeInfo) { unimplemented!() } |
There was a problem hiding this comment.
PR guard, in case this needs to be addressed in a subsequent commit
There was a problem hiding this comment.
PR change was approved
insecure-time/src/lib.rs
Outdated
| fn abs_diff(&self, other: &Self) -> Duration { | ||
| match self.duration_since(*other) { | ||
| Ok(duration) => duration, | ||
| Err(e) => e.duration() |
There was a problem hiding this comment.
Just learned about duration_since. Since it is odd, I think it merits a comment explaining that Err(e) contains the duration when other is earlier than self.
There was a problem hiding this comment.
Added a comment
insecure-time/src/lib.rs
Outdated
| } | ||
| } | ||
|
|
||
| pub struct Freq(AtomicU64); |
There was a problem hiding this comment.
From the code I understand this is an integer number representing the amount of ticks per second (add a comment).
Also is this a reasonable assumption? E.g. each second has an (exact) integer amount of ticks? Put otherwise, are ticks aligned to seconds?
There was a problem hiding this comment.
Keeping track of the time is tricky. Some platforms do report their frequency, but do that incorrectly, others report it not at all (for older platforms, probably), or incompletely. The expectation is that processors that do support SGXv2 (and can thus call rdtsc in the enclave) do report their frequency, but running the main application it was observed that this reported frequency isn't correct (and deviates more than a fraction of a tick). The logic in this crates was made to be able to handle this.
I've added comments on the Tsc struct.
raoulstrackx
force-pushed
the
raoul/rte-204-insecure_time_through_rdtscp
branch
from
f5643ac to
e7f9251
Compare
|
I've addressed the PR comments, but more work was needed to the runner. I've split up the work in two parts. I'll close this PR, and open up two new ones. Sorry for the mess. |
SGXv2 platforms support calling the
rdtscpinstruction inside an enclave. This PR implements logic to keep track of the time inside of an enclave. Note that the valuerdtscpreturns, can be manipulated by an attacker; even with these changesinsecure_timeis (almost) as insecure as before. There's only an option to force the time to be monotonic.This PR is blocked on internal API review