mrisher / smtp-sts

SMTP Strict Transport Security
Apache License 2.0
35 stars 19 forks source link

Adam Roach Comments #212

Closed abrotman closed 6 years ago

abrotman commented 6 years ago

COMMENT:

Thanks to everyone who worked on this document. The mechanism seems useful, and I hope it aids in the deployment of TLS-secured email transfer. I have a mix of editoral and substantive comments.


§1.1:

This document also uses lowercase forms of these words. Consider using the boilerplate from RFC 8174.


§3:

If the reporter does not support one of the report mechanisms, then it SHOULD NOT attempt delivery to
those rua destinations.

This seems tautological. If this is intended to restrict some action that is not impossible by its own nature, then it needs rephrasing. Otherwise, it should probably just be removed.


§4.4:

o "total-successful-session-count": The aggregate number (integer) of successfully negotiated TLS-enabled connections to the receiving site.

o "total-failure-session-count": The aggregate number (integer) of failures to negotiate a TLS-enabled connection to the receiving site.

o "failed-session-count": The number of (attempted) sessions that match the relevant "result-type" for this section.

Although these use the word "number," its use seems to be colloquial rather than technical. It is probably worthwhile, for avoidance of doubt (and to prevent errant string encodings), to add ", encoded as a JSON number" to the end of each of these.


§4.4:

dec-octet = DIGIT ; 0-9 / %x31-39 DIGIT ; 10-99 / "1" 2DIGIT ; 100-199 / "2" %x30-34 DIGIT ; 200-249 / "25" %x30-35 ; 250-255

Presumably, this is ANBF? Please indicate that. Also, in ABNF, comments start with a semicolon and continue to the end of the line, which makes this production syntactically invalid.

I beleive you want the following instead:

dec-octet = DIGIT ; 0-9 / %x31-39 DIGIT ; 10-99 / "1" 2DIGIT ; 100-199 / "2" %x30-34 DIGIT ; 200-249 / "25" %x30-35 ; 250-255


§4.5:

For the MTA-STS policy, an array of JSON strings that represents the
policy that is declared by the receiving site, including any errors
that may be present.

This isn't a sentence. Perhaps insert "this is" before "an array"?


§5.2:

The report SHOULD be subjected to GZIP compression for both email and
HTTPS transport.

This "SHOULD" makes it necessary to include GZIP as a normative reference. I believe the correct reference here is RFC 1952. While it is Informational, it is already in the downref registry, which saves us a lot of headache.


§5.4:

Alternately, if a receiving system offers "Accept-Encoding" value of
"gzip"...

Offers it where?


§6.4:

Security considerations: Security considerations relating to SMTP TLS
Reporting are discussed in Section 7.

Please also cite RFC 6713 Section 4.

                Magic number(s):  n/a

Please replace "n/a" with "first two bytes are 0x1f, 0x8b."


§6.5:

This document creates a new registry, "STARTTLS Validation Result
Types". The initial entries in the registry are:

This table omits a column for the defining document and/or party to contact, which is usually present for "Expert Review" registries. Is that intentional?


Appendix B:

Please use IPv6 or a mix of IPv4 and IPv6 in your example. See https://www.iab.org/2016/11/07/iab-statement-on-ipv6/ for further guidance.

Please select addresses from the ranges reserved by RFC 3849 (and RFC 5737, if necessary), rather than the allocated addresses (owned by Yahoo, Alisoft, and Windstream) and private-use addresses currently in the example.

danmarg commented 6 years ago

§5.4:

Alternately, if a receiving system offers "Accept-Encoding" value of "gzip"...

Offers it where?

I'm confused by this. Accept-Encoding is a request header, not a response header, so there's no way for servers to use it to indicate to clients that they accept GZIP. It seems like clients would have to just try GZIP and see if they get a 200 or a 415 response code (415 = unsupported media type).

For now I'm just removing this paragraph, but I'm happy to take suggestions on how this should work.

danmarg commented 6 years ago

Please also cite RFC 6713 Section 4.

In the tlsrpt+json media type, or the +gzip media type?

danmarg commented 6 years ago

https://github.com/mrisher/smtp-sts/pull/216

excluding the following two points:

  1. This table omits a column for the defining document and/or party to contact, which is usually present for "Expert Review" registries. Is that intentional?

Alternately, if a receiving system offers "Accept-Encoding" value of "gzip"...

danmarg commented 6 years ago

OK, only remaining issues are

This table omits a column for the defining document and/or party to contact, which is usually present for "Expert Review" registries. Is that intentional?

Alternately, if a receiving system offers "Accept-Encoding" value of "gzip"...

abrotman commented 6 years ago

Looks fine to me, closing