Skip to content
This repository has been archived by the owner on Nov 10, 2022. It is now read-only.

IESG Last Call: Address Adam Roach's DISCUSS and COMMENT feedback #310

Merged
merged 3 commits into from Jun 20, 2019

Conversation

robstradling
Copy link
Contributor

On 13th March 2019, Adam Roach wrote:
Thanks to everyone who worked on updating this protocol to reflect experience
gathered from the initial CT protocol. I have one blocking comment, and a small
number of editorial suggestions.


§5:

Clients are configured with a base URL for a log and construct URLs
for requests by appending suffixes to this base URL. This structure
places some degree of restriction on how log operators can deploy
these services, as noted in [RFC7320]. However, operational
experience with version 1 of this protocol has not indicated that
these restrictions are a problem in practice.

The synthesis of URLs by a protocol in this fashion is prohibited by BCP 190:

Scheme definitions define the presence, format, and semantics of a
path component in URIs; all other specifications MUST NOT constrain,
or define the structure or the semantics for any path component.

Unless the intention of this document is to update BCP 190 to change this
normative requirement, we can't publish it in its current form. Note that doing
so would require a change of venue, as updates to BCP 190 would not be covered
by the current TRANS charter.

Please see BCP 190 section 3 for alternate approaches. All three approaches
could be made to work for CT, and I would be happy to explain how to do so if
clarification is desired.

Addressed by robstradling@84998e6

§1.1:

The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
"SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in this
document are to be interpreted as described in [RFC2119].

Consider using the boilerplate from RFC 8174.

Already addressed in PR #309.

§1.3:

This document revises and obsoletes the experimental CT 1.0 [RFC6962]
protocol, drawing on insights gained from CT 1.0 deployments and on
feedback from the community.

Given that this document is also experimental, it seems a bit odd to call out
RFC 6962 as experimental.

Addressed by robstradling@36ceb2b

§2.1.1:

We have established a registry of acceptable hash algorithms (see

The use of first person here is awkward. Consider: "This document
establishes..."

Addressed by robstradling@36e4ff4

§10.2:

| 0x01 - | Unassigned | | Specification |
| 0xDF | | | Required and |
| | | | Expert Review |

The policy being cited here is confusing. It is unclear whether the intention is
that values can be registered under both §4.5 and §4.6 of RFC 8126. I suspect
the intention here is the policy specified in RFC 8126 §4.6 only, without
reference to the policy in §4.5. If so, please use the formal name
"Specification Required."

Already addressed in PR #309.

§10.4:

| 0x0008 - | Unassigned | Specification Required and |
| 0xDFFF | | Expert Review |

Same comment as above.

Already addressed in PR #309.

§10.5:

| 0x0000 - | Unassigned | n/a | Specification Required and |
| 0xDFFF | | | Expert Review |

Same comment as above.

Already addressed in PR #309.

not indicated that these restrictions are a problem in practice.
Clients are configured with a log's Base URL (see {{log_parameters}}), and they
construct URLs for requests by appending suffixes to this Base URL, as described
in the subsections below.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I note you left off the sentence about restricting the deployment of the log as a service and it not being an issue based on V1 deployment. Any particular reason other than it not fitting the current structure of the text?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that sentence was our justification for not having switched to using .well-known. And so by switching to using .well-known, that sentence became irrelevant.

Copy link
Contributor

@eranmes eranmes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks so much for doing this.

@robstradling robstradling merged commit e2418c8 into google:master Jun 20, 2019
@robstradling robstradling deleted the iesg_discuss_adamroach branch June 20, 2019 09:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants