[Top] [All Lists]

Review comments on draft-martin-managesieve-04.txt

2003-10-17 10:27:23

It was recently mentioned on the imapext list that it is time to get
the managesieve protocol spec moving forward. So I decided to take a look
at the specification as it stands. My comments are as follows:

(0) In keeping with IESG feedback on the sieve subaddress specification, I
    think the title needs to be changed to "A Protocol for Remotely Managing
    Sieve Email Filtering Scripts".

(1) The second paragraph of the introduction regarding ACAP should only
    appear in the introduction (see next point), not in the abstract.

(2) This specification really needs an introduction. Currently the
    introduction section is blank. I suggest an extended version of the
    abstract, something along the lines of:

  Scripts written in the sieve [SIEVE] language allow users to specify
  filters for the email they receive. Typically each user of a message
  store can have his or her own private sieve script or scripts.
  However, message stores are commonly sealed servers so users cannot
  log into them, yet users must be able to update their scripts on them.

  This document describes a protocol "sieve" for securely managing Sieve
  scripts on a remote server.  This protocol allows a user to have multiple
  scripts, and also alerts a user to syntactically flawed scripts.

  This an interim measure as it is hoped that eventually Sieve scripts
  will be stored on ACAP. This document is intended to proceed on the
  experimental track.

(3) Section 1.1 needs to be marked as needing to be removed prior to

(4) Sections 1.3 and later really don't belong as subsections of the
    introduction. I suggest moving all of these up a level.

(5) Section 1.3, first paragraph, second sentence. Three types of what?
    Tokens? And why is ATOMS capitalized?

(6) It seems to me section 1.3, Syntax, should provide a prose description
    of the syntax of responses as well as that for commands.

(7) The bit about the port number in section 1.3 really needs to be in a
    section other than one on syntax. Perhaps a section of its own is
    appropriate? It could contain the usual prose about how managesieve
    servers listen for connections on TCP port <tbd>. It also should be
    make it clear that the prose about port 2000 needs to be removed prior
    to publication.
(8) Section 1.7 and probably elsewhere. "UTF8" -> "UTF-8".

(9) Sieve script names are UTF-8 identifiers, and as such would seem to me
    to be subject to UTF-8 normalization concerns. The usual way this is
    dealt with is to specify an appropriate stringprep scheme to use. (Indeed,
    the specification appears to be silent on whether or not even ASCII
    script names are case sensitive.) I'm tempted to say that the right
    course of action is to simply reuse the nameprep stringprep profile
    defined in RFC 3491, but this assumes case-insensitivity is what you want.

(10) Section 1.8, paragraph prior to example. "any other capabilities given" ->
     "any capabilities given".

(11) Section 2.1, second example. There probably should be an ellipsis or
    something similar at the begining to indicate these aren't the first
    commands in a session.

(12) Section 2.8. Should make it clear any previously active script is
     deactivated. There's also the question of what happens to the currently
     active script when SETACTIVE is specified with a bogus script name.
     Does the currently active script remain active or is no script active
     after this is done? Need to be specific either way.

(13) Section 3, Sieve URL scheme. A couple of problems here. First, the
     function of this URL scheme needs to be stated in prose prior to the
     registration template. Second, given that the URL can either be
     used to identify either a managesieve server or a script on that
     server, doesn't the scriptname have to be an optional part of the
     URL? Third, I think the name "sieve" is too general for any URL
     scheme in this document -- it is entirely possible there will be other
     URLs that deal with sieve in some way. I suggest "managesieve" instead. 
     And finally, the usage section talks about a "sieve server". Isn't
     this a "managesieve" server?

(14) Section 4, ABNF. The rules iana-token and extension-data
     aren't defined in this specification and the only external rules that
     are inherited are the ones from the base ABNF specification. It looks
     to me like the reference for these rules that needs to be added is
     ACAP (RFC 2244). And there's also the issue of references to the
     sieveurl ABNF given in section 3. The simplest way to deal with this
     would be one more sentence saying it also refers to the sieveurl
     production defined in section 3.

(15) Section 5, second paragraph. Doesn't TLS, properly implemented, also
     provide protection against passive observation and MITM attacks?
     A pointer to the security considerations for sieve itself in RFC 3028
     would probably be a good thing to add as well.

(16) This document needs an IANA considerations section that tells IANA
     to register the URL scheme defined in section 3 and to assign a port
     for the managesieve protocol to use.

(17) Reference to [imap4rev1] needs to be updated from RFC 2060 to RFC 3501.

(18) References need to be split into normative and informative groups.

(19) Indentation of first reference doesn't match the rest.

(20) The required IPR boilerplate section is missing from this document.
     See, say, RFC 3598 section 8 for what this needs to contain.

That's it!