diff options
author | Rasmus Dahlberg <rasmus@mullvad.net> | 2021-12-09 11:14:31 +0100 |
---|---|---|
committer | Rasmus Dahlberg <rasmus@mullvad.net> | 2021-12-09 11:14:31 +0100 |
commit | 89f0a41e8baefddf2c6962a8f0eee71dfd124d21 (patch) | |
tree | 85d7723d8e66225d9fe02304b0bac4c9f00c8eac /issues | |
parent | e74021bee14cdc6a5aa22ddc2068c2f72dfe277f (diff) |
added issues
Diffstat (limited to 'issues')
-rw-r--r-- | issues/add-integration-test.md | 11 | ||||
-rw-r--r-- | issues/add-multi-instance-support.md | 17 | ||||
-rw-r--r-- | issues/add-rate-limit-support.md | 16 | ||||
-rw-r--r-- | issues/add-read-only-mode.md | 24 | ||||
-rw-r--r-- | issues/ed25519-clamping-behavior.md | 19 | ||||
-rw-r--r-- | issues/error-handling.md | 7 | ||||
-rw-r--r-- | issues/fix-http-status-405.md | 24 | ||||
-rw-r--r-- | issues/fix-strict-hex-parsing.md | 10 | ||||
-rw-r--r-- | issues/http-status-405.md | 19 | ||||
-rw-r--r-- | issues/improve-error-messages.md | 16 | ||||
-rw-r--r-- | issues/improve-server-configuration.md | 17 | ||||
-rw-r--r-- | issues/investigate-ed25519-clamping.md | 20 | ||||
-rw-r--r-- | issues/other.md | 9 | ||||
-rw-r--r-- | issues/server-configuration.md | 24 | ||||
-rw-r--r-- | issues/strict-hex-parsing.md | 5 | ||||
-rw-r--r-- | issues/upgrade-trillian-version.md | 9 |
16 files changed, 162 insertions, 85 deletions
diff --git a/issues/add-integration-test.md b/issues/add-integration-test.md new file mode 100644 index 0000000..33351c9 --- /dev/null +++ b/issues/add-integration-test.md @@ -0,0 +1,11 @@ +**Title:** Add integration test </br> +**Date:** 2021-12-09 </br> + +# Summary +Add integration test that runs sigsum-log-go hooked-up to Trillian. + +# Description +Today we don't have any integration tests. Before a new version is tagged, it +is tested by (i) running unit tests, and (ii) running manual tests against a +local setup of Trillian and sigsum-log-go. Automating (ii) would be helpful +for development and increased confidence that everything works as expected. diff --git a/issues/add-multi-instance-support.md b/issues/add-multi-instance-support.md new file mode 100644 index 0000000..ce32755 --- /dev/null +++ b/issues/add-multi-instance-support.md @@ -0,0 +1,17 @@ +**Title:** Add multi-instance support </br> +**Date:** 2021-12-09 </br> + +# Summary +Add support for multiple active sigsum-log-go instances for the same log. + +# Description +A sigsum log accepts add-cosignature requests to make the final cosigned tree +head available. Right now a single active sigsum-log-go instance is assumed per +log, so that there is no need to coordinate cosigned tree heads among instances. + +Some log operators will likely want to run multiple instances of both the +Trillian components and sigsum-log-go, backed by a managed data base setup. +Trillian supports this, but sigsum-log-go does not due to lack of coordination. + +This issue requires both design considerations and an implementation of the +`StateManager` interface to support multi-instance setups of sigsum-log-go. diff --git a/issues/add-rate-limit-support.md b/issues/add-rate-limit-support.md new file mode 100644 index 0000000..167199d --- /dev/null +++ b/issues/add-rate-limit-support.md @@ -0,0 +1,16 @@ +**Title:** Add rate limit support </br> +**Date:** 2021-12-09 </br> + +# Summary +Add support for rate-limiting add-leaf requests via second-level domain name. + +# Description +A sigsum log requires a submitter to prove that a domain name is aware of their +public verification key. Rate limits can then be applied per second-level +domain name. Trillian has built-in rate-limiting using a so-called quota +manager; gRPC calls include an arbitrary `charge_to` string that is used as an +identifier with regards to who should be charged for the request. + +First investigate whether Trillian's built-in rate limiting can be used and with +which assumptions. For example, is `etcd` a required process? Then implement +and document how an operator can configure sigsum-log-go with rate limits. diff --git a/issues/add-read-only-mode.md b/issues/add-read-only-mode.md index d87c244..24336f9 100644 --- a/issues/add-read-only-mode.md +++ b/issues/add-read-only-mode.md @@ -1,14 +1,16 @@ -# Add read-only mode -Reported by: rgdd +**Title:** Add read-only mode </br> +**Date:** 2021-12-09 </br> -The process of shutting down a log will likely consist of at least two steps: -1. Stop accepting new logging requests. Serve the final (co)signed tree heads -for a while. -2. Take the log offline. +# Summary +A read-only mode is needed to facilitate maintenance and shutdowns of production +logs. For example, after an operator has decided to cease their operations the +log in question should be kept around for some time to allow final monitoring. -The first step requires some form of read-only mode. For example: -- Disable all write endpoints (`add-leaf` and `add-cosignature`) -- Implement a `StateManager` that serves fixed (co)signed tree heads. +# Description +This issue requires design considerations. For inspiration, you may refer to + [CTFE](https://github.com/google/certificate-transparency-go/tree/master/trillian/ctfe). -For inspiration we could look at certificate transparency: -- https://github.com/google/certificate-transparency-go/tree/master/trillian/ctfe +At minimum it should be possible to (i) disable all write endpoints, and (ii) +serve a cosigned tree head for all add-leaf requests that were already merged. + +It would be good to consider if we need a mirror-mode before getting started. diff --git a/issues/ed25519-clamping-behavior.md b/issues/ed25519-clamping-behavior.md deleted file mode 100644 index 6e8fed7..0000000 --- a/issues/ed25519-clamping-behavior.md +++ /dev/null @@ -1,19 +0,0 @@ -# Ed25519 clamping behavior -Reported by: rgdd - -If I recall correctly an Ed25519 signature has 3 bits that should always be -zero. What happens if any of the 3 bits are not zero during signature -verification? It probably depends on the implementation. I would expect that the -signature is rejected. However, a possible behavior that I would not expect is -that the three bits are zeroed ("fixed"). - -We need the signature to be rejected; not fixed. Otherwise it is possible to -replay a logged entry several times by enumerating the remaining bit patterns. -Replays are bad for the log (overhead). Replays are also bad for the legitimate -submitter because it will eat into their rate limit (DoS vector). - -It would be great if anyone could: -- Confirm if I recall correctly. And if so, confirm if the behavior of -`crypto/ed25519` is to reject signatures if any of the three bits are set. -- After a quick look this might be the place to understand: -https://cs.opensource.google/go/go/+/refs/tags/go1.16.4:src/crypto/ed25519/ed25519.go;l=208 diff --git a/issues/error-handling.md b/issues/error-handling.md deleted file mode 100644 index 20eb97c..0000000 --- a/issues/error-handling.md +++ /dev/null @@ -1,7 +0,0 @@ -# Error handling -Reported by: rgdd - -- Some error messages are probably too verbose. We should take a pass over this -and ensure that they are masked appropriately. A production mode would be nice. -- For some reason our current error messages have one too many new lines. In -other words, we want `Error: some text\n` and not `Error: some text\n\n`. diff --git a/issues/fix-http-status-405.md b/issues/fix-http-status-405.md new file mode 100644 index 0000000..7a06288 --- /dev/null +++ b/issues/fix-http-status-405.md @@ -0,0 +1,24 @@ +**Title:** Fix HTTP status 405 </br> +**Date:** 2021-12-09 </br> + +# Summary +Stop returning HTTP Status 405 or ensure that RFC 2616 is followed. + +# Description +When using HTTP GET for a POST endpoint or vice versa, HTTP status code 405 is +currently returned by sigsum-log-go. According to RFC 2616, an Allow header MUST +be included in the response. This issue requires figuring out what +sigsum-log-go should do: not return HTTP Status 405 or adhere to RFC 2616? + +Extract from RFC 2616: +``` +10.4.6 405 Method Not Allowed + +The method specified in the Request-Line is not allowed for the resource +identified by the Request-URI. The response MUST include an Allow header +containing a list of valid methods for the requested resource. +``` + +To find the relevant parts in the sigsum-log-go code, see the output of + + git grep StatusMethodNotAllowed diff --git a/issues/fix-strict-hex-parsing.md b/issues/fix-strict-hex-parsing.md new file mode 100644 index 0000000..bab188e --- /dev/null +++ b/issues/fix-strict-hex-parsing.md @@ -0,0 +1,10 @@ +**Title:** Fix strict hex parsing </br> +**Date:** 2021-12-09 </br> + +# Summary +Fix so that sigsum-log-go is strict about lower-case hex parsing. + +# Description +The current sigsum-log-go implementation uses "encoding/hex" which accepts +upper-case and lower-case hex. This is a violation of the Sigsum API +specification and needs to be fixed: upper-case hex must be rejected. diff --git a/issues/http-status-405.md b/issues/http-status-405.md deleted file mode 100644 index 3278f42..0000000 --- a/issues/http-status-405.md +++ /dev/null @@ -1,19 +0,0 @@ -# HTTP status 405, no Allow header -Reported by: ln5 - -When using HTTP GET for a POST endpoint or vice versa, HTTP status code 405 is -returned by the server. According to RFC2616 an Allow header MUST be included in -the response. - -``` -10.4.6 405 Method Not Allowed - -The method specified in the Request-Line is not allowed for the resource -identified by the Request-URI. The response MUST include an Allow header -containing a list of valid methods for the requested resource. -``` - -Find relevant places to start looking: -``` -$ git grep StatusMethodNotAllowed -``` diff --git a/issues/improve-error-messages.md b/issues/improve-error-messages.md new file mode 100644 index 0000000..773d2d7 --- /dev/null +++ b/issues/improve-error-messages.md @@ -0,0 +1,16 @@ +**Title:** Improve error messages </br> +**Date:** 2021-12-09 </br> + +# Summary +Error messages that are returned by the log need to be looked-over. + +# Description +Some error messages are too verbose and may even span multiple lines. Error +messages that span multiple lines violate the Sigsum API specification. This +issue requires seeing over what error messages are currently returned, then +ensuring that what becomes externally visible is appropriate. + +Examples of appropriate error messages: +- `Error=unknown witness with key hash $hash` +- `Error=invalid tree head signature for tree head with timestamp $t` +- `Error=rate limit exceeded for $domain_hint` diff --git a/issues/improve-server-configuration.md b/issues/improve-server-configuration.md new file mode 100644 index 0000000..7e9de71 --- /dev/null +++ b/issues/improve-server-configuration.md @@ -0,0 +1,17 @@ +**Title:** Improve server configuration and documentation </br> +**Date:** 2021-12-09 </br> + +# Summary +Make server configuration more robust and dynamically updatable without restart. + +# Description +All server configurations are currently done via command-line arguments. This +may be OK for settings that last through a log's entire lifetime. However, it +is inappropriate for parameters like `--witnesses` which are not static. + +Reading a configuration file at start and when receiving, say, SIGHUP, is an +alternative. Implementing a "control port", typically via a TCP endpoint, where +an administrator can "program" the log instance is another alternative. + +This issue requires some design considerations before getting started. It would +be good to improve documentation on how to run sigsum-log-go at the same time. diff --git a/issues/investigate-ed25519-clamping.md b/issues/investigate-ed25519-clamping.md new file mode 100644 index 0000000..46aaa39 --- /dev/null +++ b/issues/investigate-ed25519-clamping.md @@ -0,0 +1,20 @@ +**Title:** Investigate Ed25519 clamping behavior</br> +**Date:** 2021-12-09 </br> + +# Summary +Ed25519 signatures have three bits that should be zero due to clamping. What +happens when verifying a signature that has these three bits set to something +else? Sigsum requires that such a signature is rejected. + +# Description +First confirm that Ed25519 signatures are clamped as described in the summary, +then investigate how `Verify()` is implemented in `"crypto/ed25519"`. The +assumed sigsum-log-go behavior is that `Verify()` is strict. In other words, a +signature that is not clamped correctly should be rejected and not "fixed". + +If a signature is "fixed" it would be possible to replay add-leaf requests. A +replay is bad for the log due to overhead. A replay is also bad for the +legitimate submitter because it eats into their rate limit (DoS vector). + +The following part of Go's implementation might be a good place to start: +- https://cs.opensource.google/go/go/+/refs/tags/go1.16.4:src/crypto/ed25519/ed25519.go;l=208 diff --git a/issues/other.md b/issues/other.md new file mode 100644 index 0000000..de7df3b --- /dev/null +++ b/issues/other.md @@ -0,0 +1,9 @@ +**Title**: Other </br> +**Date**: 2021-12-09 </br> + +# Summary +A list of TODOs that lacks a better home for now. + +# Description +- Monitoring: requires both design and implementation in a separate repo. There +are no particular requirements regarding which programming language to use. diff --git a/issues/server-configuration.md b/issues/server-configuration.md deleted file mode 100644 index dab70b9..0000000 --- a/issues/server-configuration.md +++ /dev/null @@ -1,24 +0,0 @@ -# Server configuration -Reported by: ln5 - -All server configuration is done via command-line arguments. - -This is good for configuration settings which last through the lifetime of an -invocation of a log instance, i.e., from launch to Ctrl-C. Examples: -- `--http_endpoint` -- `--key`. - -It is less good for settings that change over time. Examples: -- `--witnesses` - -Reading a configuration file at start and when receiving, say, SIGHUP, is an -alternative. - -Implementing a "control port", typically a TCP endpoint, where an administrator -can "program" the log instance is another alternative. Such an interface can -also be used for diagnostics. - -We also need to add better documentation on how to run and configure -sigsum-log-go. There is a rough start in cmd/sigsum-log-go/README.md. It -assumes a little bit too much of the reader, and does not document everything -that is relevant. For example, configuration of sharding is not documented. diff --git a/issues/strict-hex-parsing.md b/issues/strict-hex-parsing.md deleted file mode 100644 index 9155ef1..0000000 --- a/issues/strict-hex-parsing.md +++ /dev/null @@ -1,5 +0,0 @@ -# Strict hex parsing -reported by: rgdd - -Our API spec requires use of lower-case hex. The `encoding/hex` library already -outputs lower-case hex, but it also parses upper-case hex. Needs to be fixed. diff --git a/issues/upgrade-trillian-version.md b/issues/upgrade-trillian-version.md new file mode 100644 index 0000000..bcf59b9 --- /dev/null +++ b/issues/upgrade-trillian-version.md @@ -0,0 +1,9 @@ +**Title:** Upgrade Trillian version </br> +**Date:** 2021-12-09 </br> + +# Summary +Upgrade Trillian version to v1.4.0. + +# Description +Trillian v1.4.0 was released in September. Some structures changed format and +so will require a little bit of adapting in pkg/trillian, see compile errors. |