Hi Alexey,

Resending links to closed issues  and current pull requests to be incorporated 
based on feedback.

https://github.com/mrisher/smtp-sts/issues

https://github.com/mrisher/smtp-sts/pulls

Thanks again for your feedback!
Janet


From: Uta [mailto:[email protected]] On Behalf Of Daniel Margolis
Sent: Sunday, April 23, 2017 4:58 AM
To: Alexey Melnikov <[email protected]>
Cc: [email protected]
Subject: Re: [Uta] Review of draft-ietf-uta-mta-sts-04

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 
<[email protected]<mailto:[email protected]>> 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
      "[email protected]<mailto:[email protected]>" this would ordinarly be 
"example.com<https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fexample.com&data=02%7C01%7CJanet.Jones%40microsoft.com%7C8f9a55de6af947ac545508d48a4014f7%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636285455239419598&sdata=MVyalwAYv9yfPeWm8RpK2Cqa2zVQEYhQlVTyMRJlWvg%3D&reserved=0>",
 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
[email protected]<mailto:[email protected]>
https://www.ietf.org/mailman/listinfo/uta<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.ietf.org%2Fmailman%2Flistinfo%2Futa&data=02%7C01%7CJanet.Jones%40microsoft.com%7C8f9a55de6af947ac545508d48a4014f7%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636285455239419598&sdata=6sjsJP5fw6mFqt8FBKHweAemN1%2Fh%2Fd1G%2BHVVNzjpkQY%3D&reserved=0>

_______________________________________________
Uta mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/uta

Reply via email to