diff options
author | Stuart Stock <stuart@int08h.com> | 2020-06-14 14:13:22 -0500 |
---|---|---|
committer | Stuart Stock <stuart@int08h.com> | 2020-06-14 14:13:22 -0500 |
commit | 08be7295bf67ee3bc97e3030078c66a2cbdbc583 (patch) | |
tree | 8cb079c6e6993a12e9537d40c49ca2226a632236 | |
parent | dad2ecaec8c69c1e37dec79521a47b3e938bad01 (diff) | |
download | roughenough-08be7295bf67ee3bc97e3030078c66a2cbdbc583.zip |
Commentary on draft-ietf-ntp-roughtime-02
-rw-r--r-- | doc/rfc-commentary.md | 162 |
1 files changed, 162 insertions, 0 deletions
diff --git a/doc/rfc-commentary.md b/doc/rfc-commentary.md new file mode 100644 index 0000000..14d4b59 --- /dev/null +++ b/doc/rfc-commentary.md @@ -0,0 +1,162 @@ +# Comments on draft-ietf-ntp-roughtime-02 + +Author: Stuart Stock +Date : 2020-06-14 + +## Introduction + +I am the original creator and current maintainer of two Roughtime implementations: + + * Roughenough (Rust) https://github.com/int08h/roughenough + * Nearenough (Java) https://github.com/int08h/nearenough + +Have written deep-dive articles about the Roughtime protocol: + + * https://int08h.com/post/to-catch-a-lying-timeserver/ + * https://int08h.com/post/roughtime-message-anatomy/ + +And operate the longest running publicly accessible (non-Google) Roughtime server +on the internet + + * roughtime.int08h.com:2002 + +I offer my comments on draft-ietf-ntp-roughtime-02 as someone well versed in the Roughtime +protocol and intimately familiar with its implementation and trade-offs. + +## Keep the "rough" in "Roughtime" + +The authors of the original Roughtime protocol definition [roughtime] state that a +deliberate goal of the Roughtime protocol was "...time synchronisation to within 10 +seconds of the correct time." Those authors go on to state that "[i]f you have +_serious_ time synchronisation needs you‘ll want the machinery in NTP or even PTP..." + +Addition of the DUT, DTAI, and LEAP fields run contrary to this design intent. These fields +provide time synchronization features that duplicate those in NTP and PTP. Precise time +sync should be delegated to NTP and PTP and their ecosystems. + +Simplicity of the Roughtime protocol encourages simple implementations. Simplicity +in implementation is a key to assurance that implementations are correct and secure. +Increasing the number of fields and field types that must be implemented runs contrary +to ease of implementation. Simplicity, and by extension security, should be a deliberate +and top-of-mind design goal of the Roughtime standardization process. + +Multiple existing IETF protocols address needs of highly accurate time synchronization +and should be used where timing precision requires. Keep Roughtime "rough". + +Suggestion: remove DUT, DTAI, and LEAP fields along with int32 type + +## The LEAP field is not necessary + +Roughtime can handle leap seconds and time uncertainty/discontinuity complications without the +addition of new tags/fields. The RADI field already provides a method for a Roughtime +implementation to express time uncertainty in responses [draf02]: + + The RADI tag value is a uint32 representing the server's estimate of + the accuracy of MIDP in microseconds. Servers MUST ensure that the + true time is within (MIDP-RADI, MIDP+RADI) at the time they compose + the response message. + +In the case of a leap second, a Roughtime server can increase the value returned in +the RADI field to account for the uncertainty introduced by a leap second. The increase +in RADI value can persist as long as necessary (the duration of a leap second "smear" +for example [AWS-smear] and [Google-smear]). + +Suggestion: remove DUT, DTAI, and LEAP fields along with int32 type + +## Introduction of a signed int32 type creates a new security risk + +The addition of the signed int32 field type (required to support the DUT, DTAI, and +LEAP fields) exposes Roughtime implementations to a new *class* of software errors +around sign/unsigned conversions. + +The Common Weaknesses Enumeration Top 25 Most Dangerous Software Errors [CWE-TOP25] +lists "Integer Overflow or Wraparound" as the #8 most frequent software weakness. The +detail page on integer overflow [CWE-190] identifies signed/unsigned confusion, +unintentional wrap-around, and lack of range checks as sources of software errors. + +Introducing a signed integer type into Roughtime does not guarantee these issues +occur. However it does introduces an entirely new area of concern that would not +exist if these signed fields were omitted. + +Suggestion: remove DUT, DTAI, and LEAP fields along with int32 type + +## Restore the 1024 byte minimum request size requirement + +Draft 02 states that: + + Responding to requests shorter than 1024 bytes is OPTIONAL + and servers MUST NOT send responses larger than the requests + they are replying to. + +The minimum request size requirement exists to prevent a roughtime server from becoming +a DDoS amplifier [CF-DDoS]. A minimum request size of of 1024 bytes ensures that even +a busy roughtime server signing a Merkle tree of 64 requests generates a response *smaller* +than the 1024 byte request minimum (response would be 744 bytes, see [int08h]). + +If requests smaller than 1024 bytes are permitted, how small could a request be? A valid +Roughtime request *without* a PAD field would be 72 bytes long: + + 4 bytes number of fields + 4 bytes NONC tag + 64 bytes NONC value + +Given that the *minimum* Server response size is 360 bytes [int08h], a minimal size request +presents a DDoS attacker with a potential 5x gain in size. + +Making the minimum response size OPTIONAL requires Roughtime server operators to decide +"how small is too small" and a wrong choice will create more DDoS amplifiers in the world. + +Suggestion: mandate requests >= 1024 bytes + +## Checking response vs. request size complicates server implementation + +In Draft 02: + + Responding to requests shorter than 1024 bytes is OPTIONAL + and servers MUST NOT send responses larger than the requests + they are replying to. + +Roughtime servers can batch multiple requests into a single response making response +size a function of server load/batching parameters plus concurrent requests. Roughtime +Server implementations may gather or buffer client requests prior to constructing the +response. + +"...servers MUST NOT send responses larger than the requests..." will require implementations +to perform additional tracking of per-request sizes and then compute the resulting response +size once the response *after* batch size has been determined. + +This is more complex and incurs additional processing compared to simply rejecting all +requests <1024 bytes. + +Suggestion: mandate requests >= 1024 bytes + +## The "ROUGHTIM" packet format is redundant + +The addition of the constant "ROUGHTIM" plus additional length field is redundant to +the message format (which also has a length field). The value this additional +packet format is not clear. + +Suggestion: use "bare" Roughtime messages as the packet format + +## Stick with SHA-512; eliminate use of truncated SHA-512/256 + +Truncated SHA-512/256 is performed by a) compute SHA-512, then b) truncate the result. +The resulting computational effort of SHA-512 and SHA-512/256 is equivalent. + +The draft utilizes SHA-512/256 for its 32 byte output, as opposed to 64 bytes for +SHA-512. The motivation for this change is unclear and it complicates implementations +which now need two hashing primitives (SHA-512/256 initialization is different than SHA-512). + +Suggestion: use SHA-512 throughout and drop any use of SHA-512/256 + +## References + +* [AWS-smear] https://aws.amazon.com/blogs/aws/look-before-you-leap-the-coming-leap-second-and-aws/ +* [CF-DDoS] https://www.cloudflare.com/learning/ddos/ntp-amplification-ddos-attack/ +* [CWE-190] https://cwe.mitre.org/data/definitions/190.html +* [CWE-TOP25] https://cwe.mitre.org/top25/archive/2019/2019_cwe_top25.html +* [draft02] https://tools.ietf.org/html/draft-ietf-ntp-roughtime-02 +* [Google-smear] https://developers.google.com/time/smear +* [int08h] https://int08h.com/post/to-catch-a-lying-timeserver/#keeping-response-sizes-compact +* [roughtime] https://roughtime.googlesource.com/roughtime + |