Some feedback while implementing a verification tool

[aykevl]

I've written a small (possibly buggy) testing tool for MTA-STS: https://github.com/aykevl/mta-sts. While doing that, I found a few issues with the spec as it is. Some are things that I suspect are real issues or omissions, others may just be because I misunderstood some portion of the spec.

I've put this in a single issue so the issue list won't get flooded.

For the MTA-STS part:

• There is no ABNF yet for the policy file - but I see it's being worked on.
• No charset has been declared for the policy file. What if the server sends UTF-16 text? I'm now assuming ASCII.
• Currently the Content-Type for the policy file is undefined, but with the change to the .txt extension I think this should be text/plain. Maybe the Content-Type should be explicitly listed in the spec? What should a verifier do when it encounters another Content-Type?
• How should a verifier handle non-200 HTTP status codes? I'm assuming it should fail, but I don't see anything in the spec about this.
• What about HTTP caching? How does it work together with max_age? In particular:
  • Should a client honor caching headers (Last-Modified, Cache-Control, etc.)?
    Note that mail server may not be able able to use a cache similar to a browser or HTTP proxy.
  • What about (reverse) proxies? Currently it's possible that the TXT record is updated with a new version but the verifier receives a cached policy file.
• I've seen a comment saying that the current key-value format is intended to be similar to RFC2822 for easier parsing. It might be a good idea to mention this in the spec.

• Parsers MUST accept TXT records and policy files which are syntactically valid (i.e. valid key-value pairs separated by semi-colons for TXT records) and implementing a superset of this specification, in which case unknown fields SHALL be ignored.

  "implementing a superset of this specification" seems vague to me - what is a superset? Maybe it's better to say something like "key-value pairs with an unknown key must be ignored to allow extension to this specification".

• For the mx field, all valid entries will be utilized when enforcing the stated policy.

  So, does that mean there can be invalid mx entries in the policy file? What should a parser do when it encounters invalid mx entries?

• What if some fields in a policy file are unavailable (e.g. mode)? Should a parser assume defaults (which?) or should the policy file be ignored altogether?
• If we're using a key-value pair, why is max_age formatted using an underscore? It makes sense in JSON (which is derived from JavaScript and can't contain dashes in variable names) but for key-value pairs (like mail or HTTP headers) using dashes seems to be more common. (But I don't really care about this - not worth bikeshedding about).

And for the reporting side of the spec:

• The ABNF for the TXT record mentions:

  "URI" is imported from [@!RFC3986]; commas (ASCII 0x2C) and exclamation points (ASCII 0x21) MUST be encoded; the numeric portion MUST fit within an unsigned 64-bit integer

  I don't get the "the numeric portion MUST fit within an unsigned 64-bit integer" part. What's the numeric portion? The only part I can think of is the port number, which is 16-bit. I also can't find anything about a numeric portion in RFC3986.

• I see the ABNF for the MTA-STS TXT record has been updated (8059c9acda79cd5b7e024cb4376ba17fb91475f2), but I'm missing a similar update in SMTP-TLSRPT. In particular, a value like v=TLSRPTv1;rua=mailto:reports@example.com ; would be illegal while v=TLSRPTv1;rua = mailto:reports@example.com would be legal, if I've interpreted the ABNF correctly.
• Issue #167 also applies here.

[danmarg]

Hey, thanks for the detailed feedback. A few comments:

Points 1-3: these are rectified in the KV_Version branch (which will become the master branch as soon as I do the merge ;) ).

Regarding the "numeric portion" bit, I have no idea what this refers to, either. Maybe @abrotman does.

All the rest is, I think, good actionable feedback. I'll try to address it all before the 08 draft.

[aykevl]

I have a few more comments, of which two possible security considerations. Should I add them here or post them on the uta mailing list?

[danmarg]

I would say editorial (of the nature above) are best tracked simply as issues, since they tend to be noncontroversial.

If they are more significant (as your Security Considerations may be), I would post on the UTA list so others can give feedback.

On Mon, Aug 14, 2017 at 7:03 PM Ayke notifications@github.com wrote:

I have a few more comments, of which two possible security considerations. Should I add them here or post them on the uta mailing list?

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/mrisher/smtp-sts/issues/171#issuecomment-322247900, or mute the thread https://github.com/notifications/unsubscribe-auth/AB1vi3EwfvCzc7snotKgjBHpsjZnH-E4ks5sYH3zgaJpZM4Ozqsa .

[danmarg]

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

[danmarg]

I think these were all addressed. Let me know if I missed anything.