Re: [Jmap] Benjamin Kaduk's Discuss on draft-ietf-jmap-mail-15: (with DISCUSS and COMMENT)

["Chris Newman" <[e-mail]>]

On 4 Mar 2019, at 18:40, Benjamin Kaduk wrote:
> Section 4.1.2.1
>
>                                        A server SHOULD replace any 
> octet
>    or octet run with the high bit set that violates UTF-8 syntax with
>    the unicode replacement character (U+FFFD).  [...]
>
> This seems problematic, given that this is supposed to be the "Raw"
> format.  I guess the justification for the replacement is that we use
> JSON and JSON requires UTF-8, but if that's the case then shouldn't 
> this
> be a MUST and not a SHOULD?  In particular, a client can't rely on the
> server providing the SHOULD, so it doesn't seem to provide much value.

There's a MUST in the core document section 1.5 that requires the server 
to always send valid JSON/UTF-8. Use of the Unicode replacement 
character (URC) is one way to meet that requirement, but there are other 
ways to turn invalid data into JSON-compliant data and there may be good 
reasons to use mechanisms other than URC so I think SHOULD is 
appropriate here.

		- Chris

> Section 4.7
>
>    The server MAY forbid two email objects with the same exact 
> [RFC5322]
>    content, or even just with the same [RFC5322] Message-ID, to 
> coexist
>    within an account; if the target account already has the email the
>    copy will be rejected with a standard "alreadyExists" error.
>
> This has some security considerations that should probably be 
> mentioned
> in Section 9.4: when a user only has read privileges to a
> subset of the folders in an account, this behavior can be abused as an
> oracle to determine whether a given message exists in the inaccessible
> portions of that account.  (Similarly for /import.)
>
> Section 4.9
>
>    The following metadata properties on the Email objects will be 
> "null"
>    if requested:
>    [...]
>    o  mailboxIds
>
> This seems in conflict with the Section 4.1.1 text that every Email 
> "MUST
> belong to one or more mailboxes at all times (until it is deleted)."
> Presumably we want a broader disclaimer in 4.1.1 rather than any 
> changes
> here...
>
> There may also be a related condition wherein an EmailSubmission 
> object
> refers to an Email after the Email is deleted -- I didn't (yet) see 
> text
> to indicate whether the emailId in the EmailSubmission is expected to
> still be resolvable, in which case there would potentially not be an
> associated Mailbox.
>
> Section 9.3
>
> It's 2019.  Why are we still recommending SASL PLAIN?
> We have better options even if we are resigned to passwords, like 
> SCRAM.
>
>
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
>
> Section 1.3.1
>
>    o  *maxMailboxesPerEmail*: "UnsignedInt|null" The maximum number of
>       mailboxes that can be can assigned to a single email.  [...]
>
> nit: My understanding (shaped solely by experience and inference) is 
> that
> "email" is generally used to refer to a message, whereas "email 
> account"
> or "email address" would be used to refer to or associate with a thing
> that can be a container for folders.
>
>    o  *maxSizeMailboxName*: "UnsignedInt" The maximum length, in 
> (UTF-8)
>       octets, allowed for the name of a mailbox.  This MUST be at 
> least
>       100, although it is recommended servers allow more.
>
> The Unicode normalization form used by client/server could cause
> disagreement about whether a given name is permitted, though for the
> type of use this will get it's not clear that we need to be more
> rigorous.
>
> Section 1.3.2
>
>    o  *submissionExtensions*: "String[String[]]" A JMAP implementation
>       that talks to a Submission [RFC6409] server SHOULD have a
>       configuration setting that allows an administrator to expose a 
> new
>       submission EHLO capability in this field.  This allows a JMAP
>
> I think I'm confused by the workflow here.  Suppose we have a JMAP
> client A talking to a JMAP server B, and B is also an SMTP client that
> talks to MTA C.  This capability is supposed to be about letting B
> expose a new EHLO capability to A, but only when C supports it?
> We probably need some more text here to explain the scenario, even if 
> my
> guess is correct.
>
>       server to gain access to a new submission extension without code
>       changes.  By default, the JMAP server should hide EHLO
>       capabilities that are to do with the transport mechanism and 
> thus
>       are only relevant to the JMAP server (for example PIPELINING,
>       CHUNKING, or STARTTLS).  Each key in the object is the _ehlo-
>       name_, and the value is a list of _ehlo-args_.  Examples of
>       Submission extensions to include:
>
> The description here could probably be reworded/reorderd for clarity,
> since we don't start talking about what the actual map keys/values are
> until the penultimate sentence, at present.
>
> Section 1.5
>
>    In addition, servers MUST support pushing state changes for a type
>    called "EmailDelivery".  [...]
>
> Er, is this only servers that are implementing
> urn:ietf:params:jmap:mail?  Or is the bit about "Servers MUST support
> all properties specified for the new data types defined in this
> document"  supposed to imply that I have to implement all three as a
> unit, even if not all of them are going to be available for every
> account/credentials?
>
> Section 2
>
>    o  *id*: "Id" (immutable; server-set) The id of the mailbox.
>
> Since jmap-core now has all Ids as immutable and server-set, is this
> sort of notation considered redundant in jmap-mail?  (Throughout; this
> is just the first instance.)
>
>    o  *role*: "String|null" (default: null) Identifies mailboxes that
>       [...]
>       same role.  Servers providing IMAP access to the same data are
>       encouraged to enforce these extra restrictions in IMAP as well.
>       Otherwise, it is implementation dependent how to modify the IMAP
>       attributes to ensure compliance when exposing the data over 
> JMAP.
>
> If we had a "see also" document relation, this might justify one from 
> IMAP
> to here.  It probably doesn't qualify for Updates: though.
> Nonetheless, I'd consider having a top-level "Updates Affecting IMAP"
> section like RFC 8446 did for TLS 1.2, to collect all the bits that
> someone deploying IMAP and JMAP together might need to think about for
> their IMAP implementation.
>
>    o  *sortOrder*: "UnsignedInt" (default: 0) Defines the sort order 
> of
>       [...]
>       equal order SHOULD be sorted in alphabetical order by name.  The
>       sorting SHOULD take into account locale-specific character order
>       convention.
>
> I think this last SHOULD is probably not a 2119 SHOULD, and therefore
> should just be "should".
>
>    o  *unreadThreads*: "UnsignedInt" (server-set) An indication of the
>       number of "unread" threads in the mailbox.  For compatibility 
> with
>       existing implementations, the way "unread threads" is determined
>       is not mandated in this document.  The simplest solution to
>
> Do we really have competing *J*MAP implementations that disagree on
> this?  Or is the idea to preserve compatibility with IMAP
> clients or other current implementations that attempt to determine
> threading relationships (in which case, that should probably be 
> mentioned)?
>
>    o  *myRights*: "MailboxRights" (server-set) The set of rights 
> (ACLs)
>       the user has in relation to this mailbox.  These are backwards
>       compatible with IMAP ACLs, as defined in [RFC4314].  A
>       _MailboxRights_ object has the following properties:
>
> I read this as saying that all of the properties must be present in 
> the
> object, such that omitting a property is not a permitted synonym for 
> it
> having a value of false.  Is this reading correct?
> If so, what should a client do if the server misbehaves?
>
>       *  *mayReadItems*: "Boolean" If true, the user may use this
>          [...]
>          but not the parent mailbox, this may be "false".  Corresponds
>          to IMAP ACLs "lr".
>
> "Corresponds to" is perhaps imprecise, if one is thinking about the 
> IMAP
> ACL as being the authoritative source of information (which not all
> readers will!).  The point being that just 'l' or just 'r' would not 
> be
> enough to get this, and since JMAP is specifying a slightly more
> abstract mapping than standard IMAP rights, we should be precise about
> the mapping, and arguably in both directions.  (Not just here, but for
> all the compound ACLs, of course.  Also for IMAP ACL 'x', which has 
> two
> corresponding JMAP permissions.)
>
>       *  *maySetSeen*: "Boolean" The user may add or remove the 
> "$seen"
>          keyword to/from an email.  If an email belongs to multiple
>          mailboxes, the user may only modify "$seen" if *all* of the
>          mailboxes have this permission.  Corresponds to IMAP ACL "s".
>
> nit: in the intro, we talk of "rights the user has in relation to this
> mailbox", so it's a bit disjoint to talk of mailboxes having 
> permissons.
> (Here and below.)
>
>    o  *isSubscribed*: "Boolean" Has the user indicated they wish to 
> see
>       [...]
>       choose to ignore this property, either entirely for ease of
>       implementation, or just for the primary account (which is 
> normally
>
> nit: We don't really provide a formal definition of "primary account"
> either here or in jmap-core.
>
> Section 2.1
>
>    Standard "/get" method.  The _ids_ argument may be "null" to fetch
>
> I thought I had said something on jmap-core but maybe I only thought
> about it: my personal preference would be for section references into
> jmap-core to provide a foundation for what the "standard method" is,
> though I will not insist upon it.
>
>    all at once.
>
> (Also, this behavior is part of the standard method now.)
>
> Section 2.2
>
> I might say explicitly that "a non-null updatedProperties response
> argument indicates that the mailbox contents are unchanged from the 
> old
> state, with only the counts having changed", since these semantics are
> potentially unexpected.
>
> Section 2.3
>
>    Standard "/query" method, but with the following additional 
> argument:
>
> (Generic comment, not scoped to just this section): are we going to 
> need
> to draw a distinction between input and ouput arguments for any of the
> additions we make to standard methods?
>
>    o  *role*: "String|null" The Mailbox _role_ property must match the
>       given value exactly.
>
> So the client is responsible for lower-casing values from the IMAP
> Mailbox Name Attributes registry and the server must not do a
> case-insensitive comparison?
>
>    The following properties MUST be supported for sorting:
>
> Just to be pedantic, we're talking about the values of the "property"
> property within the Comparator object, right?  (This is one of those
> annoying things at the intersection of technical specifications and
> natural English language.)
>
> Section 4
>
>    Due to the number of properties involved, the set of _Email_
>    properties is specified over the following three sub-sections.
>
> It's probably worth a note that the subsections are for purposes of
> document organization and are not reflected in the wire protocol
> structure -- the properties involved are all top-level peers, across 
> the
> three subsections.  (Assuming that's correct, of course.)
>
> Section 4.1
>
> I assume it was a conscious WG decision to not present a full
> consolidated schema for Email.  But I want to check, since I think 
> (not
> having one) that it would have helped my understanding, and I try to 
> be
> open to discovering the errors of my ways...
>
>    o  _textBody_/_htmlBody_: These provide a list of parts that should
>       be rendered sequentially as the "body" of the message.  This is 
> a
>       list rather than a single part as messages may have headers 
> and/or
>       footers appended/prepended as separate parts as they are
>       transmitted, and some clients send text and images intended to 
> be
>       displayed inline in the body (or even videos and sound clips) as
>       multiple parts rather than a single HTML part with referenced
>       images.
>
> Some guidance related to interpreting these lists and avoiding the 
> eFail
> (efail.de) class of attacks is probably in order -- the HTML parts
> should get some different containers around their processing.  This
> could potentially go here, or in Section 9.2.
>
>    Because MIME allows for multiple representations of the same data
>    (using "multipart/alternative"), there is a textBody property 
> (which
>    prefers a plain text representation) and an htmlBody property 
> (which
>    prefers an HTML representation) to accommodate the two most common
>    client requirements.  The same part may appear in both lists where
>    there is no alternative between the two.
>
> (soapbox) It's annoying when I get mime/multipart with HTML in the
> text/plain section.  Is this clause going to allow for that same sort 
> of
> misclassification?
>
>    Due to the number of properties involved, the set of _Email_
>    properties is specified over the following three sub-sections.
>
> nit: are we at four sub-sections now?
>
> Section 4.1.1
>
>       The IMAP "\Recent" keyword is not exposed via JMAP.  The IMAP
>       "\Deleted" keyword is also not present: IMAP uses a 
> delete+expunge
>       model, which JMAP does not.  Any message with the "\Deleted"
>       keyword MUST NOT be visible via JMAP (including as part of any
>       mailbox counts).  Users may add arbitrary keywords to an email.
>
> IIRC, Trash gets special handling with respect to deletion in JMAP
> commands; does it also get special treatment w.r.t \Deleted 
> translation
> from IMAP to JMAP?
>
> Section 4.1.2.2
>
> This escape valve for "Any header not defined in [RFC5322] or 
> [RFC2369]"
> (here, et seq) seems like it might benefit from some general guidance
> about implementations applying common sense, i.e., allowing servers 
> the
> ability to deny requests for a given form for such new headers in 
> order
> to prevent nonsense behavior.
>
> Section 4.1.4
>
>    o  *partId*: "String|null" Identifies this part uniquely within the
>       Email.  This is scoped to the _emailId_ and has no meaning 
> outside
>       of the JMAP Email object representation.  This is "null" if, and
>       only if, the part is of type "multipart/*".
>
> The prose isn't the super-best indicator that the asterisk is
> intended to have wildcarding behavior.
>
> Section 4.4.1
>
> Just to double-check: the different fencepost behavior for
> minSize/maxSize is intentional?
>
>    o  *text*: "String" Looks for the text in emails.  The server 
> SHOULD
>       look up text in the _from_, _to_, _cc_, _bcc_, _subject_ header
>       fields of the message, and inside any "text/*" or other body 
> parts
>       that may be converted to text by the server.  The server MAY
>       extend the search to any additional textual property.
>
> side note: as a mail user, I like to be able to use text search to
> conclusively determine that a given message/topic is *not* in a given
> mailbox.  This weak "SHOULD" language does not provide me that
> guarantee, though I can see how it makes sense in the protocol design 
> to
> leave the flexibility for implementors, here.
>
>    o  When searching inside a "text/html" body part, any text 
> considered
>       markup rather than content SHOULD be ignored, including HTML 
> tags
>       and most attributes, anything inside the "<head>" tag, CSS and
>       JavaScript.  Attribute content intended for presentation to the
>       user such as "alt" and "title" SHOULD be considered in the 
> search.
>
> This would seem to leave no reliable way for a security researcher to
> (e.g.) search for snippets of attack javascript in received mails
> without downloading all of them.
>
>    o  Text SHOULD be matched in a case-insensitive manner.
>
> Is the server going to have a sense of the user's locale as needed for
> fully generic case-insensitive comparison?
>
>    o  Tokens MAY be matched on a whole-word basis using stemming (so 
> for
>       example a text search for "bus" would match "buses" but not
>       "business").
>
> I think this is supposed to not apply to the "phrase search" two 
> bullets
> above, but greater clarity would be appreciated.
>
>    o  *hasKeyword* - This value MUST be considered "true" if the email
>       has the keyword given as an additional _keyword_ property on the
>       _Comparator_ object, or "false" otherwise.
>
> I strongly suggest an explicit listing of the "additional properties 
> as
> reuqired for specific sort operations" of the _Comparator_ type when
> used for Emails.
>
> Section 4.6
>
>    When emptying the trash, clients SHOULD NOT destroy emails which 
> are
>    also in a mailbox other than trash.  For those emails, they SHOULD
>    just remove the Trash mailbox from the email.
>
> This last SHOULD seems to be duplicated in Section 2.
>
>    For successfully created Email objects, the _created_ response
>    contains the _id_, _blobId_, _threadId_ and _size_ properties of 
> the
>    object.
>
> For partial drafts, the behavior where a threadId always gets assigned
> on first touch is perhaps interesting, as subsequent edits might cause
> the effective threading to change, which would necessitate a new Id as
> well (IIUC).  I'm not sure if there's anything weird there that a 
> client
> would need to be prepared for, though.  (Maybe not, given that it 
> always
> is going to get back an _id_ from /set, and should be using that.)
>
> Section 4.8
>
>    If the blob referenced is not a valid [RFC5322] message, the server
>    MAY modify the message to fix errors (such as removing NUL octets 
> or
>    fixing invalid headers).  If it does this, the _blobId_ on the
>    response MUST represent the new representation and therefore be
>    different to the _blobId_ on the EmailImport object.  
> Alternatively,
>    the server MAY reject the import with an "invalidEmail" SetError.
>
> In general, having more options like this can increase the fragility 
> of
> the ecosystem, as a client might be written to assume one behvaior and
> then be not as portable to a different server.  I'm not sure that
> there's a clear way to mandate a single type of behavior here, but 
> want
> to be sure that the topic was discussed.
>
> Section 6
>
>    o  *email*: "String" (immutable) The "From" email address the 
> client
>       MUST use when creating a new message from this identity.  The
>       value MAY alternatively be of the form "*@example.com", in which
>       case the client may use any valid email address ending in
>       "@example.com".
>
> I mostly assume this is supposed to be for a generic domain and not
> special-casing example.com literally.  Some extra text/formatting 
> would
> help with that.
>
> Section 7.3
>
> The associated identityId is not a queriable property?
>
> Section 7.5
>
>    o  *onSuccessUpdateEmail*: "Id[Email]|null" A map of 
> _EmailSubmission
>       id_ to an object containing properties to update on the Email
>       object referenced by the EmailSubmission if the create/update/
>       destroy succeeds.  (For references to EmailSubmission creations,
>       this is equivalent to a back-reference so the id will be the
>       creation id prefixed with a "#".)
>
> I'm confused by the "Id[Email]" part -- we describe it as a map to "an
> object containing properties to update", but that's not exactly what 
> an
> Email object is.  Is this more of a PatchObject than an Email per se?
> nit: I'd also tweak the wording of the parenthetical a bit (here and
> below), to something like "when applying to EmailSubmissions created 
> in
> the same "/set" invocation, ..."
>
> Section 8
>
> By a literal reading, fromDate and toDate are in conflict with each
> other (when non-null).  That is, the fromDate text does not admit the
> possibility of an end to the vacation response period, and vice versa.
>
> Section 9
>
> I'd consider adding another sentence like "Additional considerations
> specific to the data types and functionality introduced by this 
> document
> are described in the following subsections."
>
> Section 9.3
>
> I know we don't want this to devolve into a generic discussion of the
> flaws of email, but perhaps the envelope-from/body-from distinction is
> worth repeating, with a note that JMAP has provisions for ACLs on
> submission that check both.
>
>
> _______________________________________________
> Jmap mailing list
> [e-mail]
> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.ietf.org_mailman_listinfo_jmap&d=DwICAg&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=K_BObr5Kfkr3rxt1oBPF9KFiEU3xl9LcD2OOJG3TXfI&m=Kd4khyWKu1OdyQCDkytHP56BfjUiYrtqcTi99Nnq9Og&s=0q4Ry-Xka_xREe6pZmV1qritx4bxQmiUMONF99Vlejo&e=