[Top] [All Lists]

Re: WGLC on draft-ietf-sieve-managesieve-01.txt (review)

2008-11-06 06:21:26


Alexey Melnikov wrote:
The Working Group Last Call for this document starts on November 3rd and will end on November 21st (so it is almost 3 weeks, so that the IETF meeting in Dublin is covered).

Please send any comments to the Sieve mailing list or directly to me. If you chose to do the latter, please CC Cyrus Daboo <cyrus(_at_)daboo(_dot_)name>. Reviews that found no issues are also welcomed, so if you review the document and find it acceptable, please let the mailing list/authors+chairs know as well.

Since this is the last call, I decided to do a full review of the document. Thus far I have not implemented any of the recent changes/additions to the document, so I may find other issues when I do (hopefully before the 21st of November).

My review is structured in a per-section manner. Comments include typos, but also requests for clarification:


- Use of example with 'the QUOTA/MAXSCRIPTS response' code is confusing. MAXSCRIPTS is not part of the standard, but the example does not mention that explicitly. Also, I can imagine that such detailed response can have some merit. So, why is it not included in the standard along with something like MAXSIZE?

- Typos in explanation of NONEXISTENT and ALREADYEXISTS : s/references/referenced.


- Example at the end of this section lists STARTTLS after authentication, which is strange considering that STARTTLS is only valid in a non-authenticated state.

- Explain the purpose of the OWNER capability. Common question would be: clients already know their identity, so why advertise it as a capability? Also, in my mind, listing status information in a capability response seems hardly appropriate.


- From text: "When both [TLS] and SASL security layers are in effect, the TLS encoding MUST be applied (when sending data) after the SASL encoding, regardless of the order in which the layers were negotiated." Isn't this order fixed correctly already by the fact that the STARTTLS command is only valid before authentication(== SASL and any security layer it may install). I mean that I do not see how the layers can be negotiated in a different order.

- From text: "To ensure interoperability, client and server implementations of this extension MUST implement the [SCRAM] SASL mechanism." This sentence seems out of place. What extension? Why SCRAM?


- Item 1, first sentence: s/than/then


- Clarify what is meant by syntax checking. In the strictest sense it could mean that any script that complies with the basic grammar of the Sieve language would pass. Command syntax, i.e. which arguments are required/allowed, would then be ignored. Also, wouldn't it be helpful/useful/desirable to include contextual checks here as well? E.g. unknown/invalid comparators etc. I first encountered this distinction when discussing the ihave extension to the Sieve language. I implicitly interpreted this requirement as a full script check (i.e. as would be done for full compilation), which seems overly restrictive when reading this standard more carefully.


- Naming of CHECKSCRIPT extension is not consistent with RENAME extension. To be consistent CHECK would be a better choice. (Yes, I am nitpicking, sorry)


- For clarity, mention that the CHECKSCRIPT command is in effect identical to the PUTSCRIPT command when in ANONYMOUS Sieve script verification mode.


- NOOP: why expect a NO response from older servers? It is advertised as a capability; if it is not advertised, don't issue the NOOP command.


- Explain the use and merit of the UNAUTHENTICATE command. The only merit I can think of is that reconnecting can cause some extra overhead for setting up the connection and re-negotiating security/TLS layers. But is this time optimization worth complicating the protocol? Discussion item: who would be willing to implement this? I personally have no real problems with the addition of this command, but I cannot imagine that I will ever want to implement it. Perhaps I am overlooking something?
- The described protocol is both referred to as "ManageSieve" and "Manage Sieve" (e.g. IANA Considerations)

I hope this review is useful. I'll try to update my implementation to match the most recent changes in the coming week or so.

Kind regards,

Stephan Bosch