Hi Stephan,
Thank you for your review.
I am quickly answering easy comments/questions and will followup on the
rest later on.
Stephan Bosch wrote:
Hello,
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:
1.3:
[...]
- Typos in explanation of NONEXISTENT and ALREADYEXISTS :
s/references/referenced.
Fixed.
1.7:
- 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.
Right. I can add a text that listing STARTTLS capability after
successful STARTTLS is a SHOULD NOT.
Unless people prefer MUST NOT?
- Explain the purpose of the OWNER capability. Common question would
be: clients already know their identity, so why advertise it as a
capability?
I will reply to this in a separate message.
Also, in my mind, listing status information in a capability response
seems hardly appropriate.
The list of currently available SASL mechanisms is also status
information (it might depend on whether TLS is on or not), so I think
your argument is not entirely valid.
2.1:
- 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.
Good point. This was cut&paste from another document.
I've deleted part of the sentence that starts with "regardless".
- 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?
This should have been "To ensure interoperability, client and server
implementations of the ManageSieve protocol ..."
Why SCRAM?
I will reply to this separately.
2.2.1:
- Item 1, first sentence: s/than/then
Fixed.
2.6:
- 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.
IMHO, invalid/unknown comparator would be covered by syntactic checks.
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.
You need to suggest specific text, especially if you want the document
to describe interaction with ihave in more details.
[...]
2.11.2:
- For clarity, mention that the CHECKSCRIPT command is in effect
identical to the PUTSCRIPT command when in ANONYMOUS Sieve script
verification mode.
I am not sure that would add much clarity.
I am almost tempted to add a reference in the other direction. The only
thing stopping me is the fact that CHECKSCRIPT is an extension.
[...]
2.11.4:
- 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.
Yes.
But is this time optimization worth complicating the protocol?
I think the answer is yes. For a management client that tries to update
Sieve scripts for several users the speed of reestablishing a
ManageSieve connection might be non trivial.
UNAUTHENTICATE command came out of discussion regarding SASL
reauthentication in Dublin. Here is the message that motivated the whole
discussion: <http://www.imc.org/ietf-mta-filters/mail-archive/msg04061.html>
Discussion item: who would be willing to implement this?
I am willing to implement this in Isode's server.
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?