Subject: [Uta] draft-ietf-uta-smtp-tlsrpt second WGLC

From: "Valery Smyslov" <[email-masked]>
Date: Wed, 21 Feb 2018 09:13:27 +0300

Hi,

this is the second working group last call (WGLC) for the "SMTP TLS Reporting"
(https://datatracker.ietf.org/doc/draft-ietf-uta-smtp-tlsrpt/).
The WGLC will be short and will last for one week till March the 1st. The document has received substantial changes recently, so
please review 
the latest version of the draft (-15) to make sure that these changes didn't break WG consensus.

Regards,
Leif & Valery.

---

From: Jim Fenton <[email-masked]>
Date: Mon, 26 Feb 2018 13:41:06 -0800

> the latest version of the draft (-15) to make sure that these changes didn't break WG consensus.
I read the draft again and did a review pretty much from scratch.
Apologies in advance if some of my comments are redundant or have
already been discussed.

Section 1 paragraph 4: "failures in routing" -- suggest also including
DNS failures (spoofing, etc.)

Section 1 paragraph 5: While this document derived from the MTA-STS
effort, it seems like it's as much a companion for RFC 7672. Also wonder
whether RFC 7672 should be a normative rather than informative
reference. (same comment in section 2)

Section 1.1 bullet 2: Perhaps also mention RFC 7672 since this is the
SMTP policy piece of DANE policy

Section 1.1 bullet 5: delivery -> relay

Section 3.1: Redundant with Appendix A.

Sections 3.1.2 and A.2 : I'm concerned that the trailing \ within quotes
might be interpreted as a literal CRLF which would not be allowed by the
ABNF.

Section 4 bullet 2c: The MX host. Apparently separate reports are
generated per MX host, which totally makes sense. But this seems like it
could have been introduced better.

Section 4 bullet 2d: An identifier for the policy (where applicable) --
This is mentioned nowhere else in the specification, so where would this
identifier come from? Suggest it be deleted.

4.3.1 bullet 2: MX -> MX hostname

4.3.1 bullets 4 and 5: failure-reason -> failure-reason-code  (also
might be good to have a forward reference because these are the first
mentions of it.)

Section 4.4 below "policy-string": The examples are poorly delimited
here; putting them in the middle of the bulleted list breaks the
continuity of the list. Suggest a reference to examples located elsewhere.

Section 5.3 paragraph 3: "MUST match the value found in the filename"
but the value in the filename is only a recommendation (section 5.1). I
continue to lean toward not depending on values in other than the
message body (single source of truth).

Section 5.3.1 last paragraph "...sending MTAs MUST NOT honor...": This
bit of normative text is easy to miss, tacked onto an example. It also
seems like it may be very hard to implement: you can't just drop a
report into the mail stream (at least not in the absence of "REQUIRETLS
NO"). Have the authors considered suggesting that the reports be sent to
another domain instead (a domain that does not have MTA-STS policy)?

-Jim

---

From: "Valery Smyslov" <[email-masked]>
Date: Wed, 28 Feb 2018 15:09:56 +0300

Hi,

a few comments.

1.
IANA Considerations section is not properly aligned with the draft text:
In particular, the STARTTLS Validation Result Types initial entries in Section 6.5 
don't include the " certificate-not-trusted" and the "dane-required" reasons 
listed in Section 4.3.2. 

2.
Section 4.4.
   For DANE TLSA policies, a JSON array array of strings each
   representing the RDATA of a single TLSA resource record as a space-
   separated list of its four TLSA fields (in RFC6698 Section 2.2)
   presentation form with no internal spaces or grouping parentheses:

   ["3 0 1
   1F850A337E6DB9C609C522D136A475638CC43E1ED424F8EEC8513D747D1D085D",
   3 0 1
   12350A337E6DB9C6123522D136A475638CC43E1ED424F8EEC8513D747D1D1234"]

   MTA-STS (array of JSON strings):

   ["version: STSv1","mode: report","mx: mx1.example.com","mx:
   mx2.example.com","mx: mx.backup-example.com","max_age: 12345678"]

It seems to me that the description of MTA-STS policy example is too brief
comparing to the description of DANE policy example. Is it possible to expand it a bit? 
And I also think that a reference to MTA-STS policy definition from [I-D.ietf-uta-mta-sts] 
is appropriate here (or in the bullet text above).

3.
In Appendix B:
   Figure: Example JSON report for a messages from Company-X to Company-
   Y, where 100 sessions were attempted to Company Y servers with an
   expired certificate and 200 sessions were attempted to Company Y
   servers that did not successfully respond to the "STARTTLS" command.

"Figure:" looks strange here, since the example above has no any title.
I suggest to s/Figure/Above. Even better would be to move this para
to the beginning of the section (Appendix B), so that first it is described
what is this example about and then the example follows. In this case 
s/Figure:/Below is.

And one more thing in this para: "a messages" - is it a typo?

And on Viktor's comment:
> A small editorial comment:
> 
> Old:
> 
>    For DANE TLSA policies, a JSON array array of strings each
>    representing the RDATA of a single TLSA resource record as a space-
>    separated list of its four TLSA fields (in RFC6698 Section 2.2)
>    presentation form with no internal spaces or grouping parentheses:
> 
> New:
> 
>    For DANE TLSA policies, a JSON array of strings each
>    representing the RDATA of a single TLSA resource record as a space-
>    separated list of its four TLSA fields in (RFC6698 Section 2.2)
>    presentation form with no internal spaces or grouping parentheses:
> 
> That is, drop the duplication of "array" and
> 
> 	s/fields (in /fields in (/ 

It seems to me that the new sentence is still too long and a bit hard to parse.
Is it possible to rephrase it? Something along the lines:

    For DANE TLSA policies, a JSON array of strings each
    representing the RDATA of a single TLSA resource record as a space-
    separated list of its four TLSA fields; the fields are in presentation format 
    (defined in RFC6698 Section 2.2) with no internal spaces or grouping parentheses:

(hope this doesn't change its meaning).

Regards,
Valery.