summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorcos <cos>2021-05-25 23:37:13 +0200
committercos <cos>2021-05-25 23:37:13 +0200
commita6ea38c79973ab70f37f08c3b0a3f34e81280e57 (patch)
tree08e22c4b32066efd5365bc071d1c4478cfac31bf
parentba64815d23e01ca83653142f7a6b2290c0a08cd4 (diff)
downloadroughenough-remove_red_paint.zip
Remove some unnecessary ugly red paintremove_red_paint
-rw-r--r--CHANGELOG.md46
-rw-r--r--CONTRIBUTING.md12
-rw-r--r--Dockerfile6
-rw-r--r--README.md62
-rw-r--r--doc/OPTIONAL-FEATURES.md60
-rw-r--r--doc/rfc-commentary.md114
6 files changed, 150 insertions, 150 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 370f372..6af47c0 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,37 +1,37 @@
## Version 1.1.8
New feature:
-* 407f12d client: output local time by default, add -z/--zulu for UTC
+* 407f12d client: output local time by default, add -z/--zulu for UTC
Housekeeping:
-* 02212e2 Switch to std::time and drop use of 'time' crate
-* d42db50 Upgrade several dependencies to latest versions
-* e13d6fd Remove deprecated `std::error::Error::description` calls
-* 32f11aa Update Dockerfile to Rust 1.42
+* 02212e2 Switch to std::time and drop use of 'time' crate
+* d42db50 Upgrade several dependencies to latest versions
+* e13d6fd Remove deprecated `std::error::Error::description` calls
+* 32f11aa Update Dockerfile to Rust 1.42
## Version 1.1.7
* Improved options for client output thanks to @zicklag (f1f834e8c).
- By default the client now outputs just the time reported by the queried server.
- The `-v` or `--verbose` flag will print additional information such as the response's
+ By default the client now outputs just the time reported by the queried server.
+ The `-v` or `--verbose` flag will print additional information such as the response's
midpoint and radius. `-j` or `--json` outputs responses in JSON format instead.
- Non-response text output is written to standard error to enable verbose output
+ Non-response text output is written to standard error to enable verbose output
while redirecting the response(s) to a file or pipe like so:
-
+
```
$ roughenough-client -v roughtime.int08h.com 2002 > time.txt
Requesting time from: "roughtime.int08h.com":2002
Received time from server: midpoint="Oct 08 2019 18:40:38", radius=1000000, verified=No (merkle_index=0)
-
+
$ cat time.txt
Oct 08 2019 18:40:38
```
## Version 1.1.6
-* Fix several Clippy items (266f1adc9)
+* Fix several Clippy items (266f1adc9)
* Update to latest Rusoto (6ff01af52)
* Update to latest google-cloudkms (a0165c019)
* Update Dockerfile to Rust 1.38 (a14c2e8)
@@ -49,9 +49,9 @@ Housekeeping:
## Version 1.1.3
-* Add decrypt option to `roughenough-kms`
+* Add decrypt option to `roughenough-kms`
-## Version 1.1.2
+## Version 1.1.2
* Add client request statistics tracking.
* Clean-up and simplification of server inner loop.
@@ -67,24 +67,24 @@ Housekeeping:
[feature's documentation](https://github.com/int08h/roughenough/blob/master/doc/OPTIONAL-FEATURES.md#http-health-check)
* Support AWS and Google Key Management Systems (KMS) to protect the server's long-term key.
See the [KMS documentation](https://github.com/int08h/roughenough/blob/master/doc/OPTIONAL-FEATURES.md#key-management-system-kms-support).
-* Numerous refactorings and clean ups to support fuzzing of
+* Numerous refactorings and clean ups to support fuzzing of
server components (b801eda, thanks to @Aaron1011)
## Version 1.0.6
-* As pointed out in #10, the client and server binary names were too generic. Rename
+* As pointed out in #10, the client and server binary names were too generic. Rename
them to be packaging friendly. Thank you @grempe. (b43bcb27ad)
-
+
## Version 1.0.5
-* The server now supports configuration from
+* The server now supports configuration from
[environment variables](https://github.com/int08h/roughenough#server-configuration)
-
+
## Version 1.0.4
-* Update `untrusted` dependency to incorporate security fix (see https://github.com/RustSec/advisory-db/pull/24).
+* Update `untrusted` dependency to incorporate security fix (see https://github.com/RustSec/advisory-db/pull/24).
Fixes #6 reported by @tirkarthi (383b0347).
-
+
## Release 1.0.3
* Limit the number of tags in a message to 1024 (0b8c965)
@@ -97,9 +97,9 @@ Housekeeping:
## Release 1.0.1 (yanked)
-* Release 1.0.1 was removed from Github and yanked from crates.io due to a range-check bug.
- 1.0.2 is its replacement.
-
+* Release 1.0.1 was removed from Github and yanked from crates.io due to a range-check bug.
+ 1.0.2 is its replacement.
+
## Release 1.0.0
Thanks to @Aaron1011's work, Roughenough has 1.0 level of functionality.
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index 16d6573..276ccbe 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -1,6 +1,6 @@
# Contributing to Roughenough
-Do you enjoy working on obscure cryptographically secure time synchronization protocols?
+Do you enjoy working on obscure cryptographically secure time synchronization protocols?
:+1::tada: nice, me too!
@@ -10,7 +10,7 @@ Please open a pull request (PR) for your changes and include:
* An overall description/rationale of the PR
* Tests for any new or modified functionality
-* Code formatted with `rustfmt` default style settings
+* Code formatted with `rustfmt` default style settings
* License (Apache 2.0) and copyright statements for your code
* A Developer Certificate of Origin (DCO) sign-off as described below
* A willingness to iterate and make changes ;)
@@ -20,12 +20,12 @@ on `stable` will be declined. Sorry.
# Developer Certificate of Origin
-To provide assurance of the provenance and integrity of contributions
+To provide assurance of the provenance and integrity of contributions
Roughenough uses the [Developer Certificate of Origin](https://developercertificate.org/)
-created by the Linux Foundation instead of lengthy Contributor License
-Agreements (CLAs).
+created by the Linux Foundation instead of lengthy Contributor License
+Agreements (CLAs).
-Please include *verbatim* and *unchanged* the full DCO statement
+Please include *verbatim* and *unchanged* the full DCO statement
below with your PR:
```
diff --git a/Dockerfile b/Dockerfile
index c7722b4..97a4b63 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -7,13 +7,13 @@
FROM rust:1.48 AS stage1
ARG ROUGHENOUGH_RELEASE=1.1.8
-ARG ROUGHENOUGH_FEATURES="default"
+ARG ROUGHENOUGH_FEATURES="default"
# Uncomment and replace above if you want KMS support
#ARG ROUGHENOUGH_FEATURES="awskms"
#ARG ROUGHENOUGH_FEATURES="gcpkms"
RUN git clone -b ${ROUGHENOUGH_RELEASE} https://github.com/int08h/roughenough.git \
- && cd /roughenough \
+ && cd /roughenough \
&& cargo build --release --features ${ROUGHENOUGH_FEATURES}
# Stage 2: runtime image
@@ -39,7 +39,7 @@ ENV ROUGHENOUGH_SEED 111111111aaaaaaaaa222222222bbbbbbbbb333333333ccccccccc44444
# COPY gcp-creds.json /roughenough
# ENV GOOGLE_APPLICATION_CREDENTIALS /roughenough/creds.json
-EXPOSE 2002/udp
+EXPOSE 2002/udp
CMD ["/roughenough/roughenough-server", "ENV"]
diff --git a/README.md b/README.md
index 97f9057..f58f606 100644
--- a/README.md
+++ b/README.md
@@ -1,14 +1,14 @@
-# Roughenough
+# Roughenough
[![crates.io](https://img.shields.io/crates/v/roughenough.svg?style=flat-square)](https://crates.io/crates/roughenough)
[![Build Status](https://img.shields.io/travis/int08h/roughenough/master.svg?style=flat-square)](https://travis-ci.org/int08h/roughenough)
[![Apache License 2](https://img.shields.io/badge/license-ASF2-blue.svg?style=flat-square)](https://www.apache.org/licenses/LICENSE-2.0.txt)
-**Roughenough** is a [Roughtime](https://roughtime.googlesource.com/roughtime) secure time
-synchronization client and server implementation in Rust.
+**Roughenough** is a [Roughtime](https://roughtime.googlesource.com/roughtime) secure time
+synchronization client and server implementation in Rust.
-Roughenough's server and client are functionally complete and
-at feature parity with the reference C++ and Golang implementations.
+Roughenough's server and client are functionally complete and
+at feature parity with the reference C++ and Golang implementations.
Requires latest stable Rust to compile. Contributions welcome, see
[CONTRIBUTING](../master/CONTRIBUTING.md) for instructions and [limitations](#limitations) for areas that could use attention.
@@ -18,14 +18,14 @@ Requires latest stable Rust to compile. Contributions welcome, see
* Original [Roughtime project](https://roughtime.googlesource.com/roughtime)
* My blog posts giving a [technical deep-dive into Roughtime](https://int08h.com/post/to-catch-a-lying-timeserver/) and
exploring details of [on-the-wire Roughtime messages](https://int08h.com/post/roughtime-message-anatomy/).
-* Cloudflare's fantastic [blog post](https://blog.cloudflare.com/roughtime/) and accompanying
+* Cloudflare's fantastic [blog post](https://blog.cloudflare.com/roughtime/) and accompanying
[open-source project](https://developers.cloudflare.com/roughtime/).
## Building and Running
### Rust 1.31 or above required
-Roughenough uses [2018 edition](https://rust-lang-nursery.github.io/edition-guide/rust-2018/index.html)
+Roughenough uses [2018 edition](https://rust-lang-nursery.github.io/edition-guide/rust-2018/index.html)
features and requires Rust 1.31 or newer to build.
### Building
@@ -35,14 +35,14 @@ features and requires Rust 1.31 or newer to build.
$ cargo build --release
```
-The client binary is `target/release/roughenough-client`. After building you can copy the
+The client binary is `target/release/roughenough-client`. After building you can copy the
binary and run on its own (no `cargo` needed) if you wish.
```bash
-$ cp target/release/roughenough-client /usr/local/bin
+$ cp target/release/roughenough-client /usr/local/bin
```
-### Using the Client to Query a Roughtime Server
+### Using the Client to Query a Roughtime Server
```bash
$ target/release/roughenough-client -v roughtime.int08h.com 2002
@@ -59,12 +59,12 @@ You can use the `date` utility on Linux machines to set the system time to the t
sudo date --utc --set "$(roughenough-client -z roughtime.int08h.com 2002)"
```
-### Validating Server Responses
+### Validating Server Responses
Use the `-p` flag with the client to validate the server's response with its public key.
```bash
-# The public key of 'roughtime.int08h.com' is stored in a DNS TXT record
+# The public key of 'roughtime.int08h.com' is stored in a DNS TXT record
$ host -t TXT roughtime.int08h.com
roughtime.int08h.com descriptive text "016e6e0284d24c37c6e4d7d8d5b4e1d3c1949ceaa545bf875616c9dce0c9bec1"
@@ -79,7 +79,7 @@ The **`verified=Yes`** in the output confirms that the server's response had a v
### Server Configuration
-There are two (mutually exclusive) ways to configure the Roughenough server:
+There are two (mutually exclusive) ways to configure the Roughenough server:
1. A YAML file, or
2. Environment variables
@@ -97,7 +97,7 @@ YAML Key | Environment Variable | Necessity | Description
`kms_protection` | `ROUGHENOUGH_KMS_PROTECTION` | Optional | If compiled with KMS support, the ID of the KMS key used to protect the long-term identity. See [Optional Features](#optional-features).
`fault_percentage` | `ROUGHENOUGH_FAULT_PERCENTAGE` | Optional | Likelihood (as a percentage) that the server will intentionally return an invalid client response. An integer range from `0` (disabled, all responses valid) to `50` (50% of responses will be invalid). Default is `0` (disabled).
-#### YAML Configuration
+#### YAML Configuration
The table above lists the YAML keys available in the config file. An example:
@@ -115,7 +115,7 @@ $ /path/to/roughenough-server /path/to/config.yaml
#### Environment Configuration
-Roughenough can be configured via the `ROUGHENOUGH_*` [environment variables](https://12factor.net/config)
+Roughenough can be configured via the `ROUGHENOUGH_*` [environment variables](https://12factor.net/config)
listed in the table above. Start the server with a single `ENV` argument to have Roughenough configure itself
from the environment. Example:
@@ -150,11 +150,11 @@ $ target/release/roughenough-server ENV
2018-07-25 00:05:09 INFO [server] Server listening on 127.0.0.1:8686
```
-The resulting binary is `target/release/roughenough-server`. After building you can copy the
+The resulting binary is `target/release/roughenough-server`. After building you can copy the
binary and run on its own (no `cargo` needed):
```bash
-$ cp target/release/roughenough-server /usr/local/bin
+$ cp target/release/roughenough-server /usr/local/bin
```
### Stopping the Server
@@ -164,13 +164,13 @@ Use Ctrl-C or `kill` the process.
## Optional Features
-Roughenough has two opt-in (disabled by default) features that are enabled either
+Roughenough has two opt-in (disabled by default) features that are enabled either
A) via a config setting, or B) at compile-time.
-* [HTTP Health Check responder](doc/OPTIONAL-FEATURES.md#http-health-check)
+* [HTTP Health Check responder](doc/OPTIONAL-FEATURES.md#http-health-check)
to facilitate detection and replacement of "sick" Roughenough servers.
* [Key Management System (KMS) support](doc/OPTIONAL-FEATURES.md#key-management-system-kms-support)
- to protect the long-term server identity using envelope encryption and
+ to protect the long-term server identity using envelope encryption and
AWS or Google KMS.
See [OPTIONAL-FEATURES.md](doc/OPTIONAL-FEATURES.md) for details and instructions
@@ -181,18 +181,18 @@ how to enable and use.
Roughtime features not implemented by the server:
-* On-line (while server is running) key rotation. The server must be restarted to generate a new delegated key.
-* The Roughenough server depends on the host's time source to comply with the smeared leap-second
- requirement of the Roughtime protocol. A Roughenough server sourcing time from
+* On-line (while server is running) key rotation. The server must be restarted to generate a new delegated key.
+* The Roughenough server depends on the host's time source to comply with the smeared leap-second
+ requirement of the Roughtime protocol. A Roughenough server sourcing time from
[Google's public NTP servers](https://developers.google.com/time/) would produce compliant
smeared leap-seconds but time sourced from members of `pool.ntp.org` likely will not.
## About the Roughtime Protocol
-[Roughtime](https://roughtime.googlesource.com/roughtime) is a protocol that aims to achieve rough
+[Roughtime](https://roughtime.googlesource.com/roughtime) is a protocol that aims to achieve rough
time synchronisation in a secure way that doesn't depend on any particular time server, and in such
-a way that, if a time server does misbehave, clients end up with cryptographic proof of it. It was
+a way that, if a time server does misbehave, clients end up with cryptographic proof of it. It was
created by Adam Langley and Robert Obryk.
-
+
## Contributors
* Stuart Stock (stuart {at} int08h.com)
* Aaron Hill (aa1ronham {at} gmail.com)
@@ -201,15 +201,15 @@ created by Adam Langley and Robert Obryk.
* Zicklag (github.com/zicklag)
## Copyright and License
-Roughenough is copyright (c) 2017-2020 int08h LLC. All rights reserved.
+Roughenough is copyright (c) 2017-2020 int08h LLC. All rights reserved.
-int08h LLC licenses Roughenough (the "Software") to you under the Apache License, version 2.0
-(the "License"); you may not use this Software except in compliance with the License. You may obtain
+int08h LLC licenses Roughenough (the "Software") to you under the Apache License, version 2.0
+(the "License"); you may not use this Software except in compliance with the License. You may obtain
a copy of the License from the [LICENSE](../master/LICENSE) file included with the Software or at:
http://www.apache.org/licenses/LICENSE-2.0
-Unless required by applicable law or agreed to in writing, software distributed under the License
+Unless required by applicable law or agreed to in writing, software distributed under the License
is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
-implied. See the License for the specific language governing permissions and limitations under
+implied. See the License for the specific language governing permissions and limitations under
the License.
diff --git a/doc/OPTIONAL-FEATURES.md b/doc/OPTIONAL-FEATURES.md
index 80310fd..963409e 100644
--- a/doc/OPTIONAL-FEATURES.md
+++ b/doc/OPTIONAL-FEATURES.md
@@ -1,6 +1,6 @@
# Optional Features
-These features are **disabled by default** and must be explicitly enabled as
+These features are **disabled by default** and must be explicitly enabled as
described below.
* [HTTP Health Check responder](#http-health-check)
@@ -8,10 +8,10 @@ described below.
# HTTP Health Check
-## Description
+## Description
-Intended for use by load balancers or other control plane facilities to monitor
-the state of Roughenough servers and remove unhealthy instances automatically.
+Intended for use by load balancers or other control plane facilities to monitor
+the state of Roughenough servers and remove unhealthy instances automatically.
The server unconditionally emits a response to *any TCP connection* to the health
check port, then closes the connection:
@@ -23,12 +23,12 @@ Connection: Close
```
-No attempt is made to parse the request, the server *always* emits this response.
+No attempt is made to parse the request, the server *always* emits this response.
## How to enable
-Provide a value for the `health_check_port` setting. This enables the HTTP
-health check responder on the configured port.
+Provide a value for the `health_check_port` setting. This enables the HTTP
+health check responder on the configured port.
```yaml
interface: 127.0.0.1
@@ -39,11 +39,11 @@ health_check_port: 8000
## DoS Warning
-**An unprotected health-check port can be used to DoS the server. Do NOT expose
-the health check port to the internet!**
+**An unprotected health-check port can be used to DoS the server. Do NOT expose
+the health check port to the internet!**
-To accurately reflect the ability of a Roughenough server to respond to requests,
-the health check socket is serviced in the same event loop executing the primary Roughtime
+To accurately reflect the ability of a Roughenough server to respond to requests,
+the health check socket is serviced in the same event loop executing the primary Roughtime
protocol. Abuse of the health-check port can denial-of-service the whole server.
If enabled, ensure the health check port is accessible only to the *intended load-balancer(s)
@@ -52,14 +52,14 @@ and/or control plane components*.
# Key Management System (KMS) Support
-## Description
+## Description
The server's long-term identity can be protected by encrypting it, storing the encrypted value
-in the configuration, and invoking a cloud key management system to temporarily decrypt
-(in memory) the long-term identity at server start-up.
+in the configuration, and invoking a cloud key management system to temporarily decrypt
+(in memory) the long-term identity at server start-up.
-This way the server's long-term identity is never stored in plaintext. Instead the encrypted
-long-term identity "blob" is safe to store on disk, on Github, in a container, etc. Ability
+This way the server's long-term identity is never stored in plaintext. Instead the encrypted
+long-term identity "blob" is safe to store on disk, on Github, in a container, etc. Ability
to access the unencrypted identity is controlled "out of band" by the KMS system.
## How to enable KMS support
@@ -76,16 +76,16 @@ $ cargo build --release --features "awskms"
## Google or Amazon: choose one and one only
-Sadly, due to incompatibilities with dependencies of the KMS libraries, only **one**
+Sadly, due to incompatibilities with dependencies of the KMS libraries, only **one**
KMS system can be enabled at a time. Attempting `--features "awskms,gcpkms"` will result
in a build failure.
## Using `roughtime-kms` to encrypt the long-term seed
-Use the command line tool `roughtime-kms` to encrypt the seed value for the
-server's long-term identity. To do this you will need:
+Use the command line tool `roughtime-kms` to encrypt the seed value for the
+server's long-term identity. To do this you will need:
- 1. The long-term key seed value
+ 1. The long-term key seed value
2. Access credentials for your cloud of choice
3. An identifier for the KMS key to be used
4. Necessary permissions to perform symmetric encrypt/decrypt operations
@@ -103,13 +103,13 @@ projects/PROJECT_NAME/locations/GCP_LOCATION/keyRings/KEYRING_NAME/cryptoKeys/KE
### AWS Example
-#### Credentials
+#### Credentials
[Rusoto](https://rusoto.org/) is used by Roughenough to access AWS. If your system
has AWS credentials in the typical `~/.aws/credentials` then everything should "just work".
-Otherwise Rusoto supports alternative ways to provide AWS credentials. See
-[Rusoto's documentation](https://github.com/rusoto/rusoto/blob/master/AWS-CREDENTIALS.md)
+Otherwise Rusoto supports alternative ways to provide AWS credentials. See
+[Rusoto's documentation](https://github.com/rusoto/rusoto/blob/master/AWS-CREDENTIALS.md)
for details.
#### `roughenough-kms` Command line
@@ -124,7 +124,7 @@ $ cargo build --release --features "awskms"
$ target/release/roughenough-kms \
-k arn:aws:kms:SOME_AWS_REGION:111122223333:key/1234abcd-12ab-34cd-56ef-1234567890ab \
-s a0a31d76900080c3cdc42fe69de8dd0086d6b54de7814004befd0b9c4447757e
-
+
# Output of above will be something like this
kms_protection: "arn:aws:kms:SOME_AWS_REGION:111122223333:key/1234abcd-12ab-34cd-56ef-1234567890ab"
seed: b8000c000102020078d39e85c7386e9e2bed1f30fac6dd322db96b8aaac8974fc6c0e0f566f8f6c971012fca1e69fffffd947fe82a9e505baf580000007e307c06092a864886f70d010706a06f306d020100306806092a864886f70d010701301e060960864801650304012e3011040c55d16d891b3b2a1ae2587a9c020110803bcc74dd96336009087772b28ec908c40e4113b1ab9b98934bd3b4f3dd3c1e8cdc6da82a4321fd8378ad0e2e0507bf0c5ea0e28d447e5f8482533baa423b7af8459ae87736f381d87fe38c21a805fae1c25c43d59200f42cae0d07f741e787a04c0ad72774942dddf818be0767e4963fe5a810f734a0125c
@@ -156,20 +156,20 @@ $ export ROUGHENOUGH_SEED=b8000c000102020078d39e85c7386e9e2bed1f30fac6dd322db96b
#### Credentials
-Only **Service Account credentials** (in `.json` format) are currently supported. OAuth, bearer tokens,
+Only **Service Account credentials** (in `.json` format) are currently supported. OAuth, bearer tokens,
GAE default credentials, and GCE default credentials are **not** supported (contributions to
add support are particularly welcome!).
To obtain Service Account credentials if you don't already have them:
* Creating a new service account?
- 1. Create the account
+ 1. Create the account
2. Download the credentials when prompted
-
+
* Existing service account?
1. Open the Cloud Console (https://console.cloud.google.com)
2. Navigate to `IAM -> Service accounts`
- 3. Locate the service account row, click on its "Actions" menu (the three dots on the right)
+ 3. Locate the service account row, click on its "Actions" menu (the three dots on the right)
4. Choose `Create key` and `JSON` format
5. Download the credentials when prompted
@@ -179,7 +179,7 @@ Make note of the full path where the credentials are saved, it's needed in the n
```bash
# Set environment variable pointing to downloaded Service Account credentials
-$ export GOOGLE_APPLICATION_CREDENTIALS=/path/to/creds.json
+$ export GOOGLE_APPLICATION_CREDENTIALS=/path/to/creds.json
# Build roughenough with Google KMS support
$ cargo build --release --features "gcpkms"
@@ -188,7 +188,7 @@ $ cargo build --release --features "gcpkms"
$ target/release/roughenough-kms \
-k projects/PROJECT_NAME/locations/GCP_LOCATION/keyRings/KEYRING_NAME/cryptoKeys/KEY_NAME \
-s a0a31d76900080c3cdc42fe69de8dd0086d6b54de7814004befd0b9c4447757e
-
+
# Output of above will be something like this
kms_protection: "projects/PROJECT_NAME/locations/GCP_LOCATION/keyRings/KEYRING_NAME/cryptoKeys/KEY_NAME"
seed: 71000c000a2400c7f2553954873ef29aeb37384c25d7a937d389221207c3368657870129d601d084c8da1249008d6fd4640f815596788e97bb3ce02fd007bc25a1019ca51945c3b99283d3945baacd77b1b991f5f6f8848c549a5767f57c9c999e97fe6d28fdb17db1d63c2ea966d8236d20c71e8e9c757c5bab62472c65b48376bc8951700aceb22545fce58d77e7cc147f7134da7a2cca790b54f29e4798442cee6e0d34e57f80ce983f7e5928cceff2
diff --git a/doc/rfc-commentary.md b/doc/rfc-commentary.md
index e4d1a15..fe3d6aa 100644
--- a/doc/rfc-commentary.md
+++ b/doc/rfc-commentary.md
@@ -10,138 +10,138 @@ I am the original creator and current maintainer of two Roughtime implementation
* Roughenough (Rust) https://github.com/int08h/roughenough
* Nearenough (Java) https://github.com/int08h/nearenough
-Have written deep-dive articles about the Roughtime protocol:
+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-Googler) Roughtime server
+
+And operate the longest running publicly accessible (non-Googler) 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
+
+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
+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
+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
+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
+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
+## 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]:
+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"
+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
+## 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
+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.
+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,
+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.
+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:
+Draft 02 states that:
- Responding to requests shorter than 1024 bytes is OPTIONAL
- and servers MUST NOT send responses larger than the requests
+ 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*
+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:
+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
+ 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
+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:
+In Draft 02:
- Responding to requests shorter than 1024 bytes is OPTIONAL
- and servers MUST NOT send responses larger than the requests
+ 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.
+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.
+size once the response *after* batch size has been determined.
-This is more complex and incurs additional processing compared to simply rejecting all
+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
+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
+Suggestion: use "bare" Roughtime messages as the packet format
-## Stick with SHA-512; eliminate use of truncated SHA-512/256
+## 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.
+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
@@ -149,7 +149,7 @@ which now need two hashing primitives (SHA-512/256 initialization is different t
Suggestion: use SHA-512 throughout and drop any use of SHA-512/256
-## References
+## 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/