Re: [Uta] Review of draft-ietf-uta-mta-sts-04

[Daniel Margolis <[e-mail]>]

Thanks. Comments inline, mostly ticking off changes. :)

I have pushed all my changes in response to this to the git repo and they
should appear in our next draft.


On Wed, Apr 19, 2017 at 1:05 PM, Alexey Melnikov <[e-mail]>
wrote:

> Hi,
> Below is my early "AD review" of the document. I think it is in pretty
> good shape and is ready for WG Last Call (I am Ok with the question of JSON
> versa something else be settled during or after WGLC.)
>
> 1) In 1.1:
>
>    o  Policy Domain: The domain for which an MTA-STS Policy is defined.
>       This is the next-hop domain; when sending mail to
>       "[e-mail]" this would ordinarly be "example.com", but
>       this may be overriden by explicit routing rules (as described in
>       "Policy Selection for Smart Hosts").
>
> Nit: This needs an internal section reference.
> I think there was another place in the document when an internal section
> number is not mentioned.
>

Done. Thanks.


>
> 2) In 3.1:
>
>       sts-version     = "v" *WSP "=" *WSP %x53 %x54        ; "STSv1"
>                         %x53 %x76 %x31
>
> Do you intend for this to be matched case-sensitively?
> What you wrote above is that "v" is case-insensitive, but "STSv1" is.
>

Good point. I actually would have intended the field names to also be
case-sensitive. (At any rate, the code I have is case sensitive.) I see no
reason to tolerate mixed case here given we're requiring specific strings
anyway.


>
> 3) Section 3.2 says that unrecognized fields are to be ignored, so you
> need to update ABNF in 3.1 to make it clear.
>
> Current ABNF:
>
>       sts-text-record = sts-version *WSP %x3B *WSP sts-id [%x3B]
>
>       sts-version     = "v" *WSP "=" *WSP %x53 %x54        ; "STSv1"
>                         %x53 %x76 %x31
>
>       sts-id          = "id" *WSP "=" *WSP 1*32(ALPHA / DIGIT)
>
> I suggest something like the following (this implies that position of the
> first 2 fields is fixed, extensions at the end. If you prefer that any
> fields are in any order (other than the version), I can update the ABNF):
>

Good point.

Looking at SPF, DMARC, and DKIM, all three require the v= to be first in
the record (which makes some sense, I suppose, to allow future versions to
have different parsing syntaxes), so I suppose we can just keep it as you
have it here. Thanks for that!


>
>       sts-text-record = sts-version *WSP field-delim *WSP sts-id
> [field-delim [sts-extensions]]
>
>       field-delim     = %x3B
>
>       sts-version     = "v" *WSP "=" *WSP %x53 %x54        ; "STSv1"
>                         %x53 %x76 %x31
>
>       sts-id          = "id" *WSP "=" *WSP 1*32(ALPHA / DIGIT)
>
>       sts-extensions  = sts-extension *(field-delim sts-extension)
> [field-delim]
>                         ; Extension fields at the end in any order
>
>       sts-extension   = sts-ext-name *WSP "=" *WSP sts-ext-value
>
>       sts-ext-name    = (ALPHA / DIGIT) *31(ALPHA / DIGIT / "_" / "-" /
> ".")
>
>       sts-ext-value   = 1*(%x21-3A / %x3C / %x3E-7E)
>                         ; like esmtp-value from RFC 5321, but doesn't
> allow ";".
>                         ; So basically any CHAR excluding "=", ";", SP,
> and control
>                         ; characters.
>
> 4) In 3.2: Should "SHOULD ignore unrecognized fields" be a MUST? I.e., why
> would it not be Ok to ignore unrecognized fields?
>

It's a bug. Thanks. :)


>
> 5) In 3.3: RFC 6125 use needs more details, because you need to specify
> answers to every question in section 3 of RFC 6125.
> In particular you should say that when checking certificates, you only use
> DNS-ID and CN-ID (SRV-ID and URI-ID are not used) and that you allow
> wildcards in them.
>

Thanks, I've clarified this.


> 6) Last para on page 7: this is also true in RFC 6125.
>

 Correct.

7) In 5.1, last para: I think you mean that if there are too many failures
> to deliver when using MTA-STS, regular SMTP rules for generating a bounce
> apply? I think this needs rewording to say that.
>

Fixed, and included a reference to rfc5321's relevant section, to hopefully
make clear we expect existing rules to apply.


>
> 8) If you want to allow for extensibility, you probably need an IANA
> registry of fields allowed, so that developers can find them easily. I can
> help with some text.
>

I'd appreciate any suggestions.

Is there a need for that now, though? I would not want to overengineer
this, either. :)


>
> 9) On page 13: I think pseudocode should make it clear that you retrieve
> DNS-ID SAN.
>

Thanks, done.


>
> Best Regards,
> Alexey
>
> P.S. I might have a couple of extra items, but I need to double check a
> few things first.
>
> _______________________________________________
> Uta mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/uta
>