Re: [Jmap] Orie Steele's Discuss on draft-ietf-jmap-sharing-07: (with DISCUSS and COMMENT)

[Orie Steele <[e-mail]>]

Hi Neil,

Thanks for your updates, there are is only 1 remaining discussion point,
which will repeat here:

" The server MAY limit the maximum number of notifications it will ..." -
denial of service / resource exhaustion ?

See comments inline for the rest:

On Thu, Apr 11, 2024 at 10:12 PM Neil Jenkins <[e-mail]>
wrote:

> Hi Orie,
>
> Thank you for your review. I have published an updated draft
> <https://www.ietf.org/archive/id/draft-ietf-jmap-sharing-08.html>
> addressing your comments, as detailed below.
>
> The repeated '*' make this section difficult to read, I initially assumed
> this
> syntax indicated the field is not optional.
>
> After reviewing RFC8620, it seems to simply be a "bolding" / "emphasis"
> effect.
>
>
> That's correct. In the HTML version of the document I think this makes it
> clearer, but in the plain text version I agree it's more confusing. I would
> probably defer to whatever the RFC Editors think is the best option here.
> (Ideally in my mind, it keeps the bold for the HTML version but doesn't
> wrap it in the asterisks for the plain text, if that's possible.)
>
> This section (and similar ones) contain many instances of "String" which is
> defined in https://datatracker.ietf.org/doc/html/rfc8620#section-1.1
>
> As has been discussed on the list regarding a related document:
> https://mailarchive.ietf.org/arch/msg/jmap/nEumbk7DKcV6AOg6q-7yD_qetU0/
>
> Unicode / i18n processing of these fields is possibly non trivial...
> The document would be greatly improved by a section commenting on what
> kinds of
> JSON Strings / UTF-8 strings must remain supported. Examples in an appendix
> demonstrating interesting edge cases / unicode characters might help.
>
>
> I have added an Internationalisation Considerations section, with the
> following text:
>
> Experience has shown that unrestricted use of Unicode can lead to problems
> such as inconsistent rendering, users reading text and interpreting it
> differently than intended, and unexpected results when copying text from
> one location to another. Servers MAY choose to mitigate this by restricting
> the set of characters allowed in otherwise unconstrained String fields.
> The FreeformClass, as documented in [RFC7564
> <https://author-tools.ietf.org/api/export/75dc1bfd-a43e-4f48-a1e9-9622161dd7aa/sharing.html#RFC7564>
> ], Section 4.3 <https://rfc-editor.org/rfc/rfc7564#section-4.3> may be a
> good starting point for this.
>
> Attempts to set a value containing code points outside of the permissible
> set can be handled in a few ways by the server. The first option is to
> simply strip the forbidden characters and store the resulting string. This
> is likely to be appropriate for control characters for example, where they
> can end up in data accidentally due to copy-and-paste issues, and are
> probably invisible to the end user. JMAP allows the server to transform
> data on create/update, as long as any changed properties are returned to
> the client in the /set response, so it knows what has changed, as per [
> RFC8620
> <https://author-tools.ietf.org/api/export/75dc1bfd-a43e-4f48-a1e9-9622161dd7aa/sharing.html#RFC8620>
> ], Section 5.3 <https://rfc-editor.org/rfc/rfc8620#section-5.3>.
> Alternatively, the server MAY just reject the create/update with an
> invalidProperties SetError.
>

"Section 4.3 may be a good starting point for this." -> "Section 4.3 might
be a good starting point for this."

It's a nit, but "might be..." avoids the lower case "may".

Thanks for adding this section, it addresses most of my discuss.


> ```
> 310        *  *accountIds*: String[]
> 311           A list of account ids.  The Principal matches if any of the
> ids in
> 312           this list are keys in the Principal's "accounts" property
> (i.e.,
> 313           if any of the account ids belong to the principal).
> ```
>
> Should this not be Id[] ? Is full UTF-8 equality check expected here?
>
>
> It should indeed be Id[], thanks for spotting, I've fixed this.
>
> ```
> 354        The server MAY limit the maximum number of notifications it will
> 355        store for a user.  When the limit is reached, any new
> notification
> 356        will cause the previously oldest notification to be
> automatically
> 357        deleted.
> ```
>
> Why not SHOULD or MUST? What happens if there is no limit?
>
>
> You could end up with a lot of notifications. This is not necessarily a
> problem as long as your server and client are well architected. The point
> of this text was mainly to make it permissible for servers to choose not to
> store all the notifications if it thinks it's not required.
>

My concern was denial of service / resource exhaustion.
I'd prefer a "SHOULD, unless" framing instead of MAY here, but it's up to
you.


>
> ```
> 359        The server MAY coalesce notifications if appropriate, or remove
> 360        notifications that it deems are no longer relevant or after a
> certain
> 361        period of time.  The server SHOULD automatically destroy a
> 362        notification about an object if the user subscribes to that
> object.
> ```
> Why not MUST? Should 362 say "unsubscribes" ?
>
>
> The original intention here was "subscribe" — if the notification was
> telling you that you now have access to an object and the user subscribes
> to it, they know it exists and so don't need the notification. Upon
> reflection though, I think this is more something a user presentation
> issue, and something a client should handle (when subscribing, it could
> choose to destroy the notification), so I have just removed this sentence.
>

Thanks.


>
> ```
> 439        *  *objectAccountId*: String
> 440           The objectAccountId value must be identical to the given
> value to
> 441           match the condition.
> ```
>
> Should this be `*objectAccountId*: Id` ?
>
>
> Yes, thanks. Fixed.
>
> ```
> 173        The server MAY reject the user's attempt to subscribe to some
> 174        resources even if they have permission to access them, e.g., a
> 175        calendar representing a location.
> ```
>
> Can this be strengthened to a SHOULD with clear conditions for when the
> server
> MUST allow subscription?
>
>
> Not really, it's dependent on the implementation. For example, with JMAP
> calendars, the client MUST be allowed to set their own sort order and a few
> other properties on subscribed calendars, so if the server can't support
> this for some calendars (e.g. a calendar representing a room that's
> actually auto-generated from another backend booking system) it must reject
> the subscription attempt. While the calendars spec also notes the server
> may reject the subscription, I think it's worth having up front as a
> general thing here too so they can't be seen as in conflict.
>

Ok.


>
> ```
> 177        A user may query the set of Principals they have access to with
> 178        "Principal/query" (see Section 2.4).  The Principal object may
> then
> 179        provide Account objects if the user has permission to access
> data for
> 180        that principal, even if they are not yet subscribed.
> ```
>
> Can this be strengthened with normative language?
> For example:
> The Principle object MUST contain / provide when...
> The Principle object MUST NOT contain / provide when...
>
>
> I have rewritten this as:
>
> *The Principal object will contain an Account object for all accounts
> where the user has permission to access data for that principal, even if
> they are not yet subscribed.*
>
> I don't think the normative MUST is appropriate here as this is simply a
> statement of the structure of the object. It should avoid any hint that
> there is any kind of choice here!
>

Thanks.


>
> ```
> 295        However, the server may reject this change, and probably will
> reject
> 296        any other change, with a forbidden SetError.  Managing
> principals is
> 297        likely tied to a directory service or some other vendor-specific
> 298        solution, and may occur out-of-band, or via an additional
> capability
> 299        defined elsewhere.
> ```
>
> Can this language be strengthened?
>
> For example: When the server rejects updates it MUST use a SetError of type
> forbidden as described in Section 5.3 of RFC8620.
>
>
> I have rewritten this as:
>
> *Managing principals is likely tied to a directory service or some other
> vendor-specific solution and may occur out-of-band, or via an additional
> capability defined elsewhere. Allowing direct user modification of
> properties has security considerations, as noted in **Section 5*
> <https://author-tools.ietf.org/api/export/f6ba9860-9013-488b-8553-0f520eccc727/sharing.html#security-considerations>*.
> Servers MUST reject any change it doesn’t allow with a **forbidden**
> SetError.*
>

Thanks.


>
>
> ```
> 314        *  *email*: String
> 315           Looks for the text in the email property.
> ```
>
> I suggest a stronger reference for IDNA compatible email identifiers.
> There are many combinations of unicode characters that will not result in a
> valid email address. Consider:
> https://datatracker.ietf.org/doc/html/rfc6531 If
> this field is not required to be a well formed email (possibly one build
> of an
> IDNA), perhaps note that directly.
>
>
> I have added to the "email" property definition:
>
> *If given, the value MUST be conform with the "addr-spec" syntax, as
> defined in **[**RFC5322*
> <https://author-tools.ietf.org/api/export/4a43b11b-ad50-4bc3-90cd-0c00e02ce767/sharing.html#RFC5322>*],
> **Section 3.4.1* <https://rfc-editor.org/rfc/rfc5322#section-3.4.1>*.*
>

This reads awkwardly, I suggest:

If present, the value MUST conform to the "addr-spec" syntax, as defined
in [RFC5322
<https://author-tools.ietf.org/api/export/4a43b11b-ad50-4bc3-90cd-0c00e02ce767/sharing.html#RFC5322>
], Section 3.4.1 <https://rfc-editor.org/rfc/rfc5322#section-3.4.1>.


>
> ```
> 318        *  *text* String
> 319           Looks for the text in the name, email, and description
> properties.
> ```
>
> The use of the word text is slightly misleading here without inclusion of
> the
> charset, since strictly speaking this is filtering on octets (encoding some
> UTF-8 string), correct?
>
> Thanks to Yaron Sheffer for his comments to this effect in the SEC DIR
> review.
>
>
> I have updated this wording to address Yaron's comments.
>

Thanks


>
> ```
> 347        The server SHOULD create a ShareNotification whenever the user's
> 348        permissions change on an object.  It SHOULD NOT create a
> notification
> 349        for permission changes to a group principal, even if the user
> is in
> 350        the group.
> ```
>
> Why not MUST? The way this is written, an implementation could easily
> decide to
> reverse these recommendations.
>
>
> The SHOULD is in recognition that this API is often going to be an
> interface onto an existing directory management system (unlike most other
> JMAP APIs, where we can be more proscriptive).
>

Ok.


>
> ```
> 614     5.  Security Considerations
> ```
>
> Please consider adding security considerations regarding deceptive use of
> unicode characters, perhaps drawing from previous work, for example from:
> https://datatracker.ietf.org/doc/html/rfc5894#section-1.4
>
> """
>    The introduction of the larger repertoire of characters
>    potentially makes the set of misspellings larger, especially given
>    that in some cases the same appearance, for example on a business
>    card, might visually match several Unicode code points or several
>    sequences of code points.
> """
>
>
> I have added into the security considerations:
>
> *Note that simply forbidding the use of a name already in the system is
> insufficient protection, as a malicious user could still change their name
> to something easily confused with the existing name by using trivial
> misspellings or visually similar unicode characters.*
>
>
Thanks

> ```
> 639        The set of principals within a shared environment SHOULD be
> strictly
> 640        controlled.  If adding a new principal is open to the public,
> risks
> 641        include:
> ```
>
> Why not MUST?
>
>
> This should be a MUST, I've changed it.
>
> Is consent from existing principles required or recommended when adding
> new principles?
>
>
> Not normally, no. Think of a standard office environment, where the
> principals correspond to employees.
>
> ## Nits
>
> ```
> 536        "u12345678": {
> 537            "name": "[e-mail]"
> 538            "isPersonal": true
> 539            "isReadOnly": false
> 540            "accountCapabilities": {
> 541                "urn:com.example:jmap:todo": {},
> 542                "urn:ietf:params:jmap:principals:owner": {
> 543                    accountIdForPrincipal: "u33084183",
> 544                    principalId: "P105aga511jaa"
> 545                }
> 546            }
> 547        }
> ```
>
> This example is malformed JSON. accountIdForPrincipal and principalId are
> missing quotes, and the there is no indication of elision, consider
> starting
> with { ... "u12345678": ... }
>
>
> Thanks, I've fixed this.
>
> Its also strange that "name" is an email in this example
>
>
> I used this because the account name is most commonly the email address of
> the owner of the account.
>

Ok


>
> Cheers,
> Neil.
>


-- 


OS, ART AD