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

From: Alexey Melnikov <[email-masked]>
Date: Wed, 19 Apr 2017 12:05:47 +0100

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-masked]" 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.

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.

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):

       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?

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.

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

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.

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.

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

Best Regards,
Alexey

P.S. I might have a couple of extra items, but I need to double check a 
few things first.

---

From: Daniel Margolis <[email-masked]>
Date: Sun, 23 Apr 2017 13:58:27 +0200

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-masked]>
wrote:

> 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:
>

Done. Thanks.

> 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.

> 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!

> 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. :)

> 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.

> 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

Thanks, done.

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

---

From: Janet Jones <[email-masked]>
Date: Tue, 25 Apr 2017 16:40:39 +0000

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

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-masked]<mailto:[email-masked]>> 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:

      "[email-masked]<mailto:[email-masked]>" 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

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:

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:

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-extensions  = sts-extension *(field-delim sts-extension) [field-delim]
                        ; Extension fields at the end in any order

      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.

 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.

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-masked]<mailto:[email-masked]>
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>

---

From: Alexey Melnikov <[email-masked]>
Date: Fri, 28 Apr 2017 13:21:02 +0100

On 23/04/2017 12:58, Daniel Margolis wrote:

> On Wed, Apr 19, 2017 at 1:05 PM, Alexey Melnikov 
> <[email-masked] <mailto:[email-masked]>> wrote:
  [snip]
>
>     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.
>
>

All IETF documents are required to have the "IANA Considerations" 
section. IANA will manage registries if they are created by an RFC.
If you know that a particular protocol or format field is going to be 
extensible, it is much better to define a registry now than be forced to 
create one in a hurry later on.

So just create an issue for this (if you haven't already). I can help 
with some text.