Subject: [Extra] AD review of draft-ietf-extra-imap-fetch-preview-00

From: Alexey Melnikov <[email-masked]>
Date: Mon, 14 Jan 2019 11:59:23 +0000

Hi,

This document is nearly ready for IETF LC and IESG review, but I have a
few comments that require a bit of work to address:

In Section 1:

   A server that supports the PREVIEW extension indicates this with one
   or more capability names consisting of "PREVIEW=" followed by a
   supported preview algorithm name.  This format provides for future
   upwards-compatible extensions and/or the ability to use locally-
   defined preview algorithms.

This is not reflected in the ABNF section (Section 7). See my comment on
section 7 below.

In Section 3.2:

   The algorithm used by the server to generate the preview is returned
   preceding the preview string.

   The server returns a variable-length string that is the generated
   preview for that message.

Having an example at this point would make it easier for reader to
understand the document.

In Section 4.1:

   If the FUZZY algorithm generates a preview that is not based on the
   body content of the message and the LANGUAGE [RFC5255] extension is

Is preview is not based on the body content, what is it based on?
Do you mean header fields from the message?

   supported by the server, the preview text SHOULD be generated
   according to the the language rules that apply to human-readable
   text.

In Section 5.1:

How would the client know when to try to retrieve LAZY preview again?

In Section 7:

   The following syntax specification uses the augmented Backus-Naur
   Form (BNF) as described in ABNF [RFC5234].  It includes definitions
   from IMAP [RFC3501].

     capability        =/ "PREVIEW=FUZZY"

This doesn't quite much what you specify in Section 1.
Maybe change it to:

        capability        =/ "PREVIEW=" preview-alg
?

     fetch-att         =/ "PREVIEW" [SP "(" preview-alg-fetch *(SP
                          preview-alg-fetch) ")"]

     msg-att-dynamic   =/ "PREVIEW" SP "(" preview-alg SP nstring ")"

     preview-alg       =  "FUZZY" / preview-alg-ext

     preview-alg-ext   =  preview-atom  ; New algorithms MUST be
                                        ; registered with IANA

And you don't define the procedure for registering these in the IANA
Considerations section.

     preview-mod-ext   =  preview-atom  ; New priority modifiers MUST be
                                        ; registered with IANA

As above: this needs to have the corresponding IANA Considerations text.

In Section 10:

   There are no known additional security issues with this extension
   beyond those described in the base protocol described in IMAP4
   [RFC3501].

I am not entirely convinced that this is true. This extension either
requires more disk storage or more computation on the server. Both can
cause some form of Denial-of-Service.
(And disk storage space might be worth calling out as a separate
operational consideration)

Best Regards,
Alexey

---

From: Michael Slusarz <[email-masked]>
Date: Mon, 21 Jan 2019 17:48:21 -0700 (MST)

Thanks for the review/comments.  Discussion below.

> On January 14, 2019 at 4:59 AM Alexey Melnikov <[email-masked]> wrote:
> 

Ack.  Will address in Section 7.

> Having an example at this point would make it easier for reader to

I think a counter-argument to an example here would be that we have yet to define the "FUZZY" algorithm, so any example would use terminology that has yet to have been explained.

Notwithstanding this, how about this:

 Example: Retrieving preview information in a SELECTed mailbox

     C: A1 FETCH 1 (PREVIEW)
     S: * 1 FETCH (PREVIEW (FUZZY {15}
     S: Preview text!
     S: ))
     S: A1 OK FETCH complete.

> Do you mean header fields from the message?

I was thinking more of messages containing entirely non-textual data.

For example: a message consisting of a single image/jpg MIME part.  There is no "text" to use for preview data in this example.  However, a server might want to instead provide the user a description of what the message contains.  Such as "This message contains an image file of size 800x600 pixels".  This description text should be generated in the language that the user has indicated via RFC 5255, if possible.

> How would the client know when to try to retrieve LAZY preview again?

I explicitly did not want to provide any specific retry duration recommendations, as there is no way of making a blanket statement on what that correct value can be across server implementations.

Additionally, the expected use case would be to issue a single FETCH LAZY command when initially building the mailbox listing, and then issuing a background FETCH command without the LAZY modifier to return the data once generation is complete.  Thus, there generally would only be a maximum of two FETCH commands issued for any message in the mailbox.

We could add text describing this "maximum two FETCH approach" to make this clear, to help client implementers.  Note that this recommended behavior is already demonstrated in Examples 4 & 5 in Section 6, so maybe we just need to point to those examples.

>         capability        =/ "PREVIEW=" preview-alg

That looks correct to me.

> As above: this needs to have the corresponding IANA Considerations text.

In a very early draft of the spec, I had this text (using old snippet language):

   This document also requests that IANA adds a new IMAP4 [RFC3501]
   snippet algorithms registry, which registers snippet algorithms by
   publishing a standards track or IESG-approved experimental RFC.  This
   document constitutes registration of the FUZZY algorithm in that
   registry.

I can resurrect that text and add similar language for modifiers, if agreed.

If we do add a registry, I think the text in ABNF section should be changed from:

"New algorithms MUST be registered with IANA"

to:

"New algorithm names MUST conform with the recommendations described in RFC 6648, Section 3"

See: https://tools.ietf.org/html/rfc6648#section-3

> operational consideration)

How about this text, adapted from SEARCH=FUZZY Security Considerations section (RFC 6203):

   Implementation of this extension might enable denial-of-service
   attacks against server resources, due to excessive memory or CPU usage during generation
   or increased storage usage if preview results are cached on the server after
   generation.  Servers MAY limit the resources that preview generation uses.

michael

---

From: Alexey Melnikov <[email-masked]>
Date: Tue, 22 Jan 2019 10:57:18 +0000

Hi Michael,

>       S: * 1 FETCH (PREVIEW (FUZZY {15}
Yes, this looks good. I think this would certainly help understanding of 
"The algorithm used by the server to generate the preview is returned 
preceding the preview string".
> For example: a message consisting of a single image/jpg MIME part.  There is no "text" to use for preview data in this example.  However, a server might want to instead provide the user a descripti…
Ok. Then I suggest you add your explanation as an example of why this 
might be needed.
> Additionally, the expected use case would be to issue a single FETCH LAZY command when initially building the mailbox listing, and then issuing a background FETCH command without the LAZY modifier…
This was not clear to me from the text.
> We could add text describing this "maximum two FETCH approach" to make this clear, to help client implementers.
Yes, this would be very helpful.
> Note that this recommended behavior is already demonstrated in Examples 4 & 5 in Section 6, so maybe we just need to point to those examples.
>
>
>> In Section 7:
>>
Nit: RFC 3501 reference here is a distraction, as the whole document is 
an IMAP4 extension and snippets are not defined in RFC 3501.
> I can resurrect that text and add similar language for modifiers, if agreed.
Yes, this is roughly what is needed.
> See: https://tools.ietf.org/html/rfc6648#section-3

I am happy with the <https://tools.ietf.org/html/rfc6648#section-3> 
reference, but I think it would be better to say both:

"New algorithm names MUST be registered with IANA and MUST conform with the recommendations described in RFC 6648, Section 3"

>     generation.  Servers MAY limit the resources that preview generation uses.

Yes, this is much better.

Thank you,

Alexey

---

From: Michael Slusarz <[email-masked]>
Date: Tue, 22 Jan 2019 13:35:50 -0700 (MST)

I've edited the draft based on comments in this thread and have pushed -01.

> "New algorithm names MUST be registered with IANA and MUST conform with the recommendations described in RFC 6648, Section 3"

On additional review, and re-reading RFC 6648, I think the correct language is to omit the "MUST be registered" language.

One of the goals of this spec is to allow flexibility for local implementers to expand the PREVIEW functionality as they see fit.  (We are already thinking about at least one additional algorithm we might implement.)  These additional algorithms/modifiers may only be relevant to a single project/customer/use-case however, so they are not necessarily something that will ever be standardized.

RFC 6648, at least by my reading, explicitly allows this non-standardization path and simply asks someone implementing to follow some steps to attempt to avoid things like naming collisions.

So to me, it seems like we shouldn't require any new PREVIEW algorithms to "MUST be registered with IANA" - instead, simply make sure they are semantically correct (ABNF), and follow agreed-upon conventions (RFC 6648), and that is all we ask.

michael

---

From: Alexey Melnikov <[email-masked]>
Date: Wed, 23 Jan 2019 15:03:30 +0000

Hi Michael,

> One of the goals of this spec is to allow flexibility for local implementers to expand the PREVIEW functionality as they see fit.  (We are already thinking about at least one additional algorithm w…
While this is true, it would still be useful to register these to avoid 
name collisions. This is probably the main point for having an IANA 
registry.
> RFC 6648, at least by my reading, explicitly allows this non-standardization path and simply asks someone implementing to follow some steps to attempt to avoid things like naming collisions.
For this one typically need to carve a namespace which has different 
registration rules (e.g. "First Come First Served"). It is possible to 
have different policies for such namespace and usual (intended to be 
standardized) registrations.
> So to me, it seems like we shouldn't require any new PREVIEW algorithms to "MUST be registered with IANA" - instead, simply make sure they are semantically correct (ABNF), and follow agreed-upon co…

Personally, I prefer everything to be registered, even if it is not 
standardized. Maybe "SHOULD be registered with IANA"? What do other 
people think?

Best Regards,

Alexey

---

From: Alexey Melnikov <[email-masked]>
Date: Wed, 23 Jan 2019 15:11:32 +0000

Hi Michael,

1 small comment on the latest version:

>>> <[email-masked]> wrote:
  [snip]
>>> That looks correct to me.

You forgot to include this change in the latest revision of the document.

Thank you,

Alexey