Add Roughtime client pseudo-Node #845
Conversation
|
I needed a break from fighting Bazel and Cargo, so I digressed this morning… |
docs/concepts.md
Outdated
| Applications to retrieve an approximate wall clock time using the | ||
| [Roughtime protocol](https://roughtime.googlesource.com/roughtime). Note that | ||
| retrieving the current time is relatively slow, and so is best done | ||
| infrequently. |
There was a problem hiding this comment.
Not yet needed, but mentioning it just to keep track of the point somewhere: once the policy system is fully in place, it should be noted in the documentation that the Roughtime pseudo node is a publice node (timing/frequencies of requests can be used to leak data). Nodes should probably only call this at start-up time before they are tainted by secret data.
There was a problem hiding this comment.
Added a note for now.
oak/server/time/roughtime_client.cc
Outdated
| : servers_(servers.size() > 0 ? servers : roughtime::kDefaultServers), | ||
| min_overlapping_intervals_(min_overlapping_intervals > 0 | ||
| ? min_overlapping_intervals | ||
| : roughtime::kDefaultMinOverlappingIntervals), |
There was a problem hiding this comment.
Assuming the default instead of an invalid value might be unexpected. If there are fewer servers than the minimum number of overlaps it will fail later without it being clear to the users it was because of a default value. Perhaps default to 1 or return an error status immediately?
There was a problem hiding this comment.
I went for defaulting overlap to min(default, servers.size()); I've also added an extra test of a deliberately misconfigured pseudo-Node.
oak/proto/roughtime_service.proto
Outdated
| // on 1 January 1970). Leap seconds are linearly smeared over a 24-hour | ||
| // period. That is, the smear extends from UTC noon to noon over 86,401 or | ||
| // 86,399 SI seconds, and all the smeared seconds are the same length. | ||
| uint64 rough_time = 1; |
There was a problem hiding this comment.
I would call this time_usec or similar, make it super clear what the unit is. Even better, perhaps use the dedicated well-known type? https://developers.google.com/protocol-buffers/docs/reference/csharp/class/google/protobuf/well-known-types/timestamp
There was a problem hiding this comment.
Added _usec suffix, but skipped Timestamp (partly to keep the information flowing as close to what Roughtime emits as possible, partly to avoid fiddling with a new dep).
oak/proto/application.proto
Outdated
| string name = 1; | ||
| string host = 2; | ||
| uint32 port = 3; | ||
| bytes public_key_base64 = 4; |
There was a problem hiding this comment.
It seems odd for this field to be both bytes and base64 encoded. I would expect either bytes and binary, or string and base64?
There was a problem hiding this comment.
Good spot – I was going to make this have the raw key but changed my mind and forgot to change the type back.
Tweak a few other things along the way: - put internal constants and methods into an unnamed namespace - convert port to be of type uint32_t - consistently capitalize Roughtime - make defaults externally visible in the oak::roughtime namespace
Checklist
cover any TODOs and/or unfinished work.
construction.