-
Notifications
You must be signed in to change notification settings - Fork 15
Configuration: TOS agreement and mailto: contacts by default #15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
cb4e22a
to
0fdf4b8
Compare
README.md
Outdated
uri https://acme.example.com/directory; | ||
contact mailto:[email protected]; | ||
state_path /var/lib/nginx/acme-example; | ||
terms_of_service_agreed; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's probably fine, but looks somewhat unusual. Consider accept_tos on|off;
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At nginx, we have a log of long directives.
accept_terms_of_service
looks more explicit to me than accept_tos
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The unusual part is the absence of argument for a directive with looks like a boolean.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed.
But as we discussed offline, I don't believe adding a boolean argument would make the directive behavior less ambiguous.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It won't change the "ambiguity level", as "off" and directive absence should be treated equally like it's typically done with flags.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The API type for this option has 3 states: true
, false
, unset. The behavior is specified for true
; it's not specified whether false
and unset can have different meaning, leaving this up to the implementation.
The ambiguity I'm talking about is how to translate "off": should it be false
or no value, are there any implementations that distinguish these, etc. No option is better than an option that may change behavior in the future, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having different behavior for false and unset doesn't make much sense to me, the user either agrees to TOS or not.
We don't have to change the behavior in future either...
RFC8555 Section 7.3: > Clients SHOULD NOT automatically agree to terms by default. Rather, > they SHOULD require some user interaction for agreement to terms. Right now this only sets the flag, the corresponding logic will appear with the client implementation.
The only contact URL scheme required in RFC8555 is mailto:. While other schemes are allowed and can be accepted by an ACME server, most of the clients ignore that. The "contact" directive parser is updated to prepend mailto: to any URLs without a scheme. This can be resolved unambiguously, as ':' is not allowed in the email address unquoted, and quotes are forbidden in the URL scheme.
0fdf4b8
to
27111ff
Compare
The resolver configuration is an implementation detail that might become unneeded in the future, thus we should avoid exposing it unless absolutely necessary.
Overall, looks fine. |
Some clients get away with auto-accepting the terms of service, but most that I've seen follow the RFC and require user configuration.