Re: [Extra] Benjamin Kaduk's No Objection on draft-ietf-extra-imap4rev2-27: (with COMMENT)

[Alexey Melnikov <[e-mail]>]

Hi Ben,

Thank you for your extensive comments! My comments/replies below:

On 04/02/2021 09:33, Benjamin Kaduk via Datatracker wrote:
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
>
> There are several places where we see a:
>
>     Note: Since this document is restricted to 7-bit ASCII text, it is
>     not possible to show actual UTF-8 data.  [...]
>
> But this document is *not* restricted to 7-bit ASCII text!
> (I guess the (not-quoted) bit about not being possible to show actual KOI8-R data is
> still true, though.)  Showing actual non-ASCII text may not be as
> helpful as the current formulation, though, so I'd suggest just a
> modification to the disclaimer.
Ok.
> Section 1.3
>
>     IMAP was originally developed for the older [RFC-822] standard, and
>     as a consequence several fetch items in IMAP incorporate "RFC822" in
>     their name.  In all cases, "RFC822" should be interpreted as a
>     reference to the updated [RFC-5322] standard.
>
> It looks like it's down to just one (not "several"), now -- RFC822.SIZE.
I reworded, thank you.

  [snip]

> Section 3.2
>
>     In the authenticated state, the client is authenticated and MUST
>     select a mailbox to access before commands that affect messages will
>     be permitted.  This state is entered when a pre-authenticated
>     connection starts, when acceptable authentication credentials have
>     been provided, after an error in selecting a mailbox, or after a
>     successful CLOSE command.
>
> I think after a successful UNSELECT as well, right?  §6.4.2 says
> "returns the server to the authenticated state" about UNSELECT.
Thanks for spotting this. Added.
> Section 3.4
>
>              (6) CLOSE command, unsolicited CLOSED response code or
>                  failed SELECT or EXAMINE command
>
> [UNSELECT here as well, if above.]
Right.
> Section 5.1.2.2
>
>     Previous version of this protocol does not define a default server
>     namespace.  Two common namespace models have evolved:
>
> nit: maybe "the previous version of this protocol did not define" or
> "previous versions of this protocol did not define"
Ok.
> Section 6.1.1
>
>     Other capability names refer to extensions, revisions, or amendments
>     to this specification.  See the documentation of the CAPABILITY
>     response in Section 7.2.2 for additional information.  No
>     capabilities, beyond the base IMAP4rev2 set defined in this
>     specification, are enabled without explicit client action to invoke
>     the capability.
>
> Should we also note here that even the base IMAP4rev2 set can require
> explicit client action to enable (e.g., when IMAP4rev1 is also
> advertised)?
Yes.
> Section 6.2
>
>     Server implementations MAY allow access to certain mailboxes without
>     establishing authentication.  This can be done by means of the
>     ANONYMOUS [SASL] authenticator described in [ANONYMOUS].  [...]
>
> To be clear, from the perspective of the state machine, this entails
> entering the "authenticated" state but without actually authenticating
> as a specific client identity?
Yes. The assumed identity is a special anonymous identity, typically 
with fewer privileges.

> Section 6.2.1
>
> Do we really want the example to show use of LOGIN (which per §6.2.3 is
> be considered a "last resort" and SHOULD NOT be used) even when
> AUTH=PLAIN is available?
Good point. Changed.
> Section 6.2.2
>
>     As with any other client response, this initial response MUST be
>     encoded as BASE64.  It also MUST be transmitted outside of a quoted
>
> nit: it looks like we added another paragraph or two between the
> previous mention of "initial response" and here, so maybe s/this/the/ is
> in order.
Ok.
>        authentication.  (Note that SASL framework allows creation of SASL
>        mechanisms that support 2FA (2-factor authentication), however
>        none are fully ready to be recommended by this document.)
>
> (side note) With sasl/gssapi/kerberos it's possible to know that the
> client used 2fa for its authentication exchange with the KDC even if it
> only has the one (ticket) factor to present to the IMAP server.  But
> this is probably more detail than we need to get into here...
Ok. The point of the added text was to handwavy point out that 2FA and 
SASL are compatible.
>                 C: A01 AUTHENTICATE PLAIN dGVzdAB0ZXN0AHRlc3Q=
>                 S: A001 OK Success (tls protection)
>
> (nit) A01 is reusing the client tag, and doesn't seem to match the
> response ... typo?

Good catch. Fixed.

  [snip]

> Section 6.3.1
>
>     In the following example, the client enables CONDSTORE:
>
> Should we reference RFC 7162 here?
Added.
> Section 6.3.2
>
>     fails is attempted, no mailbox is selected.  When deselecting a
>     selected mailbox, the server MUST return an untagged OK response with
>     the "[CLOSED]" response code when the currently selected mailbox is
>     closed (see Paragraph 10).
>
> I'm not sure how to find Paragraph 10.
This is an xml2rfc referencing problem. I will talk to RFC Editor about 
this.
> Section 6.3.5
>
> It kind of looks like the "examples" contains two similar examples stuck
> together (or some other client has (re)created some folders
> mid-session).  I think in RFC 3501 the blank line separating examples
> also crossed a page boundary, so it got missed when converting to XML(?)
> for the new document.
Yes, a very good point.
> Section 6.3.6
>
>     If the server's hierarchy separator character appears in the name,
>     the server SHOULD create any superior hierarchical names that are
>     needed for the RENAME command to complete successfully.  In other
>
> Is this specifically in the "new mailbox name"?
Yes.
>     the normalized new mailbox name (see Section 6.3.9.7).  This would
>     allow the client to correlate supplied name with the normalized name.
>
> nit: "the supplied name".

Fixed, thanks.

  [snip]

> Section 6.4.4
>
>     However all options specified above MUST result in a single ESEARCH
>     response if used by themselves or in combination.  This guaranty
>     simplifies processing in IMAP4rev2 clients.  Future SEARCH extensions
>
> nit: s/guaranty/guarantee/
>
>     MAY be supported.  Clients SHOULD use UTF-8.  Note that if "CHARSET"
>     is not provided IMAP4rev2 server MUST assume UTF-8, so selecting
>
> nit: "an IMAP4rev2 server".
Both fixed, thanks.
> Section 6.4.4.4
>
>        Example 4:
>                 C: P282 SEARCH RETURN (SAVE) SINCE 1-Feb-1994
>                     NOT FROM "Smith"
>                 S: P282 OK SEARCH completed
>                 C: P283 SEARCH CHARSET UTF-8 (OR $ 1,3000:3021) TEXT {8}
>                 C: YYYYYYYY
>
> That snippet doesn't seem consistent with a synchronizing literal;
> should it be a non-synchronizing literal instead?

Yes, good catch.

   [snip]

> Section 6.5
>
>     Server implementations MUST NOT send any added (not specified in this
>     specification) untagged responses, unless the client requested it by
>     issuing the associated experimental command or the ENABLE command
>     (Section 6.3.1).
>
> We don't really have much text remaining to describe what the
> "associated experimental command" would be, now that the "X<atom>
> Command" section is removed.
I've added "(specified in an extension document)" after "experimental 
command". I hope this is clearer.
> Section 7.1
>
>     CAPABILITY
>
>           Followed by a list of capabilities.  This can appear in the
>           initial OK or PREAUTH response to transmit an initial
>           capabilities list.  It can also appear in tagged responses to
>           LOGIN or AUTHENTICATE commands.  This makes it unnecessary for
>           a client to send a separate CAPABILITY command if it recognizes
>           this response.
>
> (and if the implicit capability list is sent in the same
> authentication/security-mechanism context as subsequent commands)
Good point. I added 
"and there was no change to the TLS and/or authentication 
state since it was received" to the end of the last sentence.
>     COPYUID
>
>           Followed by the UIDVALIDITY of the destination mailbox, a UID
>           set containing the UIDs of the message(s) in the source mailbox
>           that were copied to the destination mailbox and containing the
>           UIDs assigned to the copied message(s) in the destination
>           mailbox, indicates that the message(s) have been copied to the
>           destination mailbox with the stated UID(s).
>
> (editorial) Is there one UID set in the response or two (one per
> source/destination)?  The following paragraph suggests two, but this one
> seems to just say one.
Yes, 2 sets. I will clarify.
>     NOPERM
>
>           The access control system (e.g., Access Control List (ACL), see
>           [RFC4314] does not permit this user to carry out an operation,
>           such as selecting or creating a mailbox.
>
> nit: missing close paren.
>
> Section 7.1.3
>
> The example doesn't seem to show the tagged BAD usage, and I'm having
> trouble convincing myself whether "very long command line" should
> qualify for the tagged form or not.

Either. I think it depends on how the parser is written./*
*/

[snip]
> /**/Section 7.2.2
  [snip]
>     A server MAY send capabilities automatically, by using the CAPABILITY
>     response code in the initial PREAUTH or OK responses, and by sending
>     an updated CAPABILITY response code in the tagged OK response as part
>     of a successful authentication.  It is unnecessary for a client to
>     send a separate CAPABILITY command if it recognizes these automatic
>     capabilities.
>
> IIRC, the earlier mention of automatic capabilities said that an
> explicit CAPABILITY is still needed for the case when (e.g.)
> AUTHENTICATE enables a new security layer.
I added similar text here.
> Section 7.3.4
>
>     [[TBD: describe the most common search data pairs returned.]]
>
> Is this still current?
I pretty much need to cut and paste text from another section. Doing it now.
> Section 7.5.2
>
>     ENVELOPE
>           [...]
>           An address structure is a parenthesized list that describes an
>           electronic mail address.  The fields of an address structure
>           are in the following order: personal name, [SMTP] at-domain-
>           list (source route, obs-route), mailbox name, and host name.
>
> The "obs-route" was not in RFC 3501, is not listed in any published
> errata reports, and does not seem to be called out in the list of
> changes from RFC 3501 in Appendix E.  This isn't the formal protocol
> description, so I guess it's not a breaking change, but I still don't
> understand why it's different (presumably just my ignorance...).
"obs-route" is defined in RFC 5322. I will clarify.
>
>     If the server chooses to send unsolicited FETCH responses, they MUST
>     include UID FETCH item.  Note that this is a new requirement when
>     compared to RFC 3501.
>
>        Example:    S: * 23 FETCH (FLAGS (\Seen) RFC822.SIZE 44827)
>
> I guess this is intended to just be a generic FETCH example, but it's a
> bit jarring to not see the UID FETCH item in the example right after the
> text that mentions a requirement to send it, with no other commentary.
Yes, good point :-). Fixed.
> Section 8
>
>     The following is a transcript of an IMAP4rev2 connection on a non TLS
>     port.  A long line in this sample is broken for editorial clarity.
>
> More than one line, now.
>
> C:   A001 AUTHENTICATE SCRAM-SHA-256
>        biwsbj11c2VyLHI9ck9wck5HZndFYmVSV2diTkVrcU8=
> S:   + cj1yT3ByTkdmd0ViZVJXZ2JORWtxTyVodllEcFdVYTJSYVRDQWZ1eEZJbGopaE5s
>       RiRrMCxzPVcyMlphSjBTTlk3c29Fc1VFamI2Z1E9PSxpPTQwOTYNCg==
> C:   Yz1iaXdzLHI9ck9wck5HZndFYmVSV2diTkVrcU8laHZZRHBXVWEyUmFUQ0FmdXhG
>       SWxqKWhObEYkazAscD1kSHpiWmFwV0lrNGpVaE4rVXRlOXl0YWc5empmTUhnc3Ft
>       bWl6N0FuZFZRPQ==
> S:   + dj02cnJpVFJCaTIzV3BSUi93dHVwK21NaFVaVW4vZEI1bkxUSlJzamw5NUc0PQ==
>
> These correspond quite nicely to (base64'd copies of) the example in RFC
> 7677, with the exception of the first server line, that includes an
> additional CRLF in the decoded data.
Ok, I will double check and correct.
> Section 9
>
>    body-fld-enc    = (DQUOTE ("7BIT" / "8BIT" / "BINARY" / "BASE64"/
>                      "QUOTED-PRINTABLE") DQUOTE) / string
>                      ; Content-Transfer-Encoding header field value.
>                      ; Defaults to "7BIT" (as per RFC 2045)
>                      ; if not present in the body part.
>
> Is this comment still accurate?
Yes.
>    capability      = ("AUTH=" auth-type) / atom
>                        ; New capabilities MUST begin with "X" or be
>                        ; registered with IANA in
>                        ; a standards-track, an experimental
>                        ; or an informational RFC.
>
> Is this comment still accurate?
Good catch. I fixed in my copy.
>    capability-data = "CAPABILITY" *(SP capability) SP "IMAP4rev2"
>                      *(SP capability)
>                        ; Servers MUST implement the STARTTLS, AUTH=PLAIN,
>                        ; and LOGINDISABLED capabilities.
>                        ; Servers which offer RFC 1730 compatibility MUST
>                        ; list "IMAP4" as the first capability.
>                        ; Servers which offer RFC 3501 compatibility MUST
>                        ; list "IMAP4rev1" as one of capabilities.
>
> I don't remember us mentioning an "IMAP4" capability in the previous
> text, and I definitely remember an assertion that the order in which
> capabilities are listed does not have significance, which seems to
> conflict with the comment about "IMAP4" as the first capability.
"IMAP4" capability is described in RFC 1730, which required it to be the 
first. All subsequent versions don't require specific position, so they 
can remain backward compatible with "IMAP4", if needed.
>    command-any     = "CAPABILITY" / "LOGOUT" / "NOOP" / x-command
>                        ; Valid in all states
>
> Is x-command still valid?
Good point. Removed.
>    media-basic     = ((DQUOTE ("APPLICATION" / "AUDIO" / "IMAGE" /
>                      "FONT" / "MESSAGE" / "MODEL" / "VIDEO" ) DQUOTE)
>                      / string)
>                      SP media-subtype
>                        ; Defined in [MIME-IMT].
>                        ; FONT defined in RFC 8081.
>
> Why does only FONT get a comment?  I don't see "MODEL" in [MIME-IMT],
> either.
Yes, I think adding a reference for MODEL (RFC 2077) is a good idea. The 
rest should be covered.
> When the namespace-command production is defined, it's spelled all in
> lowercase, but it is spelled "Namespace-Command" when it appears in the
> command-auth production.
Production names are case-insensitive. I checked.
> The "partial-range" production doesn't seem to be used anywhere.

It was originally defined in RFC 5092 (IMAP URL). If that document gets 
updated, it would be good to point to this document.

>    return-option   =  "SUBSCRIBED" / "CHILDREN" / status-option /
>                       option-extension
>
> (nit) This seems to only be used in list-return-opts, so maybe the
> generic name is not the best fit for it.

I almost renamed it to list-return-opt, but then realised that several 
other RFCs extend it.

I added "list-return-opt = return-option" with a comment.

> Section 11
>
> It might be worth putting in some bromide about how while md5 is used
> in the BODYSTRUCTURE response, the usage is not particularly security
> relevant and so there is not a vulnerability due to its use.
If you can suggest some text on this, that would be great.
> There are also some forms of DoS attack that we don't say much about
> (slowloris, many parallel connections, etc.), and the mitigations are
> fairly well known.  It might be worth expounding on these a little bit
> (though since in most cases both parties have authenticated in some
> manner, the situation is not as bad as it sometimes is).
>
> Section 11.3
>
>        as well as any response codes other than CAPABILITY.  Client
>        SHOULD ignore the ALERT response code until after TLS has been
>        successfully negotiated (whether using STARTTLS or TLS negotiation
>        on implicit TLS port).  Unless explicitly allowed by an IMAP
>
> Up in §7.1 we said that this was "without TLS or SASL security layer
> confidentiality", not limited to TLS.
> (Also, nit: "Clients" plural.)
Changed, thanks.
> Section 11.6
>
>     A server SHOULD report any authentication failure and analyze such
>     authentication failure attempt with regard to a password brute force
>     attack as well as a password spraying attack.  Accounts that match
>     password spraying attacks MUST be blocked and request to change their
>     passwords and only password with significant strength SHOULD be
>     accepted.
>
> I'm not 100% sure that "password spraying attack" is a well-known
> concept.  It probably is, but it's hard to be sure.
Ok, I will see if I can add a reference.
> Also, I assume that "accounts that match password spraying attacks"
> means accounts where the password being tested succeeds at
> authenticating, which could be worth clarifying with a wording tweak.
Done.
> Section 13.1
>
> It's not clear to me that [ANONYMOUS] is referenced in a manner that
> requires classification as normative; likewise for [SCRAM-SHA-256].
> Similarly, if we use a modified form of [UTF-7] that we describe in
> whole ourselves, that does not seem to be normative.
I will review.
> Section 13.2
>
> If we refer to RFC 3503 for more details on how the mechanism is used,
> should that be a normative reference?
Good point. Moved to normative references.
> Appendix E
>
>     29.  Revised IANA registration procedure for IMAP extensions and
>          removed "X" convention.
>
> Is that worth a BCP 178 reference?

Added.

Best Regards,

Alexey