Re: Protocol: control specifications.

[Hallvard B Furuseth <h.b.furuseth@usit.uio.no>]

I believe this message answers the clarifying message as well as the
one I'm replying to.

Kurt D. Zeilenga writes:
> Hallvard,
> 
> I believe you are missing a few aspects of the RFC 2251 text:
>    This document does not define any controls.  Controls may be defined
>    in other documents.  The definition of a control consists of:
>      ...
>      - whether the control is always noncritical, always critical, or
>        critical at the client's option,
> 
> Note that the bullet is part of a statement of practices to be followed
> in writing extension documents.  That is, this bullet is not stating a
> requirement upon implementations of LDAP (including those implement
> any control).  Someone implementing LDAP can completely ignore this
> paragraph as it simply does not pertain to implementations.

Yes, of course.  But that leads to control specifications which say
that the criticality for a control is <something>.  What we are
talking about is how to implement <the LDAP spec + such a control
spec>.  And given that combined spec, and interpreting criticality
in the text you quoted as the criticality protocol element and not
the criticality given in the control spec (which seems the natural
way to read rfc2251 to me, given the context of that text), it is a
user error to provide the other criticality value.

Which is why I at the very least want servers to continue to be
allowed to return an error for it.

> Second, your interpretation of this text as an implementation requirement
> is counter to the previous text detailing the protocol:
>    The criticality field is either TRUE or FALSE.                      
>                              
>    If the server recognizes the control type and it is appropriate for
>    the operation, the server will make use of the control when
>    performing the operation.

That text says how to perform such an operation if it is performed
at all.  Then the RFC goes on to list, among other things,
circumstances when _not_ to perform the operation.

At least, that's how I'm reading it.

> This text, as been interpreted by most implementors as meaning that,
> regardless of the value of criticality presented, a server is to make use
> of a control when they recognize the control type and that control type
> is appropriate for the operation.

Yes, I agree with that.

We disagree about
- whether or not to perform the operation at all in various
  circumstances,
- how the various parties should get to participate in that
  decision: LDAP spec, control spec, server, and/or client, and
- what it is desirable that implementors choose to do about it.

> [Moving this part of your message further down - will need to
> address a few other points first]

> Your primary argument to make the change is that servers should check
> user input.  I think it can be reasonable be argued that checking user
> input in this case is primarily the responsibility of the client (under
> the clients "should be strict in what they send" principle).

That helps, but I'm afraid s/user/user or client programmer/ in my
arguments better expresses what I meant to say.

It's a good point, though.  A statement that clients SHOULD NOT (or
MUST NOT) send controls with wrong criticality would help.  However,
that's incompatible with telling control specs to merely supply
'guidance' about what criticality to send, like you suggest.

>At 09:17 AM 1/9/2004, Hallvard B Furuseth wrote:
>>Kurt D. Zeilenga writes:
>
>>> - guidance as to what value the sender should provide for the
>>>   criticality field
>>
>>That still has the problem that it removes the server's license to
>>treat the 'unadvised' criticality as an error.
> 
> Again, the RFC 2251 text which this replaced does not specify any
> implementation "license" as it simply did not pertain to implementors.

Again, the LDAP spec + such a control spec does.  Or rather, can be
interpreted to do.  If you read it as, the control spec may say what
the client SHALL set the criticality to, then it's a client error to
set it differently.  If you read it as saying what the criticality
of the control as considered by the server is, then it is not a
client error, but OTOH all the other text talks about how the server
shall react not to the spec's value but the criticality _field_.

Your proposal redefines what was previously client bugs - and
potentially quite harmful ones - to be features.  This solution does
not appeal to me, to put it mildly.

>> My preference is this text from protocol-19:
>>
>>   - whether the criticality field should be always set to TRUE,
>>     always set to FALSE, or sender's choice
> 
> As previously noted, this language could be viewed as limiting the
> kinds of guidance which an extension specification could give.  For
> instance, it does not allow an extension from saying:
>         Criticality should be TRUE when X is TRUE.
> or
>         Criticality should be FALSE when Y is FALSE.

I must have missed that.  I agree, allowing that is good.  However,
it can be done while retaining the spirit of the current text:

    - when to set the criticality field to TRUE, when to set it
      to FALSE, and when the sender may choose

(Or if going back to the rfc2251 text, reword to not talk about
'setting' the 'field', so people may interpret it either way.  But
if you see no point in that, then I certainly prefer text based on
the current [Protocol] text.)

That should also take care of this point in your clarifying message,
unless I've missed some implications of it:

>         5) it will limit how one control could extend
>         an operation extended by other control(s).


> The intent of this original text was to encourage extension authors
> to discuss the circumstances of when particular criticality values
> are appropriate.

I wouldn't know.  If that was the intent, it's strangely worded -
strangely enough that I suspect you may be wrong about the intent,
however certain you sound.  It isn't what it says at all.

As for X.511 which is mentioned elsewhere in this thread, looking
that up left me rather confused.  A little below the text in section
7.3.1 (Critical extensions) which you quoted, it says:

  If the criticality of an extension is defined to be critical, the
  DUA shall set the corresponding bit in criticalExtensions. If the
  defined criticality is non-critical, the DUA may or may not set
  the corresponding bit in criticalExtensions.

i.e. the criticality in the spec is a mandate, not just a guidance.
However, it later says:

  The extensions, their identifiers, the operations in which they
  are permitted, the recommended criticality, and the clauses in
  which they are defined are shown in Table 1.

which indicates that e.g. the targetSystem's criticality=TRUE in
Table 1 is merely recommended, not mandated.  Then, in the
definition of targetSystem in section 11.1.2 (Add Entry arguments),
it reverts to saying:

  If the [targetSystem] argument is present, the targetSystem bit in
  the criticalExtensions parameter in CommonArguments shall be set,
  indicating that this extension is critical.

i.e. criticality=TRUE is mandated.

So I guess X.511 can be read either way, but it seems to lean
towards mandated criticality, not mere guidance.

There doesn't seem to be any text about which criticality to assume
if the client sends FALSE when TRUE is mandated, unless (a) you read
X.500 as saying the criticality is mandated, so this would be a
client error, and (b) there is text somewhere which requires the
server to catch such errors.

BTW, one note: Unlike LDAP, X.511 does not seem to allow the control
spec to mandate that the criticality is FALSE.

> The text I proposes attempts clarify the text
> to that intent while removing ambiguity that some have raised that
> the text could be viewed as stating an implementation requirement
> (instead of a document profile). 

However, it also removes the possibility of interpreting the text as
saying the wrong criticality is a user error.  So any server which
reads rfc2251 as I do and returns an error, will be in violation of
the new LDAP spec.

> [Snip]
>
>>Also, I'm still hoping for:
>>
>>  If the criticality field for a supported control does not match
>>  the value required by the control specification, the server SHOULD
>>  NOT perform the operation, but instead return protocolError in the
>>  resultCode if the operation has a response.
>
> It would be inappropriate to make such a statement in text intended
> to detail a document profile.

Of course.  It would be in the text about the LDAP protocol, not
about the control spec.

> When considers this as an addition to the technical specification,
> absent the document profile, it becomes clear that you are asking
> for a change in protocol semantics.

I don't understand what you mean here.  I'm suggesting to change an
implicit 'MAY' - implicit if one reads rfc2251 as I do - to an
explicit 'SHOULD'.  That is no change in protocol semantics.  I
might even argue that it is an implicit 'SHOULD' in rfc2251 (when
reading it as I do), since good quality servers SHOULD catch
client/user errors.

> You suggested text is also problematic for a number of reasons:
>         1) servers don't, as far as I know, don't behave in the manner you
>         describe,

A SHOULD about that does not make such servers wrong, but will push
them into moving in that direction.

Such a move will be a problem for some clients, but only the ones
that are buggy according to rfc2251.  I seem to care less about
keeping buggy clients working than you do, at least when the bug can
cause the client to do significant harm if someone switches to
another server implementation than the client was designed for.

>         2) the suggestion is counter to "be liberal in what
>         they accept" principle, and

I've already said why I don't buy this argument.  This principle is
a guidance, not a command cast in stone.  I think it is harmful to
follow it here, and I see little advantage in following it.  If you
wish to argue that LDAP should always follow this principle, then
remove error returns from Stringprep, where LDAPbis has broken this
principle.

>         3) the suggestion hinders changes to guidance given (based upon
>         operational experiences or otherwise).

No, people can change the criticality in a new spec.  As I've said,
I think that is a major change in the semantics of the control, and
to make such a change rather than making a new control is a choice
to consider very carefully.

After all, the main effect of criticality is when the server
_doesn't_ implement the control, not when it does, so modifying the
control spec isn't going to help there.  The client is still sending
an operation which that server should not perform because the
operation is wrong without performing the control.  E.g. the no-op
control.  Changing the control spec won't affect that client.

>>>   (note: the processing semantics of the
>>>   criticality field, which are defined above, should not be
>>>   altered in any way by the control's specification),
>>
>>I'd strengthen that:  s/should not/can not/.  Remove '(note: )'.
>>(Yes, I know.  Nobody is as fanatic as a convert:-)
> 
> I think it clearer as suggested.

Both seem clear to me, they just say different things.  Your 'should
not' allows implementations to go against this note, my 'can not'
does not.

However, I have to retract that suggestion, because I have bad news:
There already are control spec RFCs that give the criticality field
additional semantics.  Look:

RFC2649 An LDAP Control and Schema for Holding Operation Signatures

   1.1.  Audit Trail Mechanism

   The SignedOperation
   Control MAY be marked CRITICAL, and if it is CRITICAL then if the
   LDAP Server performs the LDAP operation, then must include the
   signature in the signedAuditTrail information.

   2.  Signed Results Mechanism

   This control MAY be marked as CRITICAL.  If it is marked
   as CRITICAL and the LDAP Server supports this operation, then all
   search results MUST be returned with a signature as attached in the
   SignedResult control if it is willing to sign results for this user.

RFC2891 LDAP Control Extension for Server Side Sorting of Search Results

   2.1 Behavior in a chained environment

   If a client submits a sort request to a server
   which chains the request and gets entries from multiple servers, and
   the client has set the criticality of the sort extension to TRUE, the
   server MUST merge sort the results before returning them to the
   client or MUST return unwillingToPerform.

That RFC also has this mess:

   2. Client-Server Interaction

   3 - If the server supports this sorting control but for some reason
       cannot sort the search results using the specified sort keys and
       the client specified TRUE for the control's criticality field,
       then the server SHOULD do the following: return
       unavailableCriticalExtension as a return code in the
       searchResultDone message; include the sortKeyResponseControl in
       the searchResultDone message, and not send back any search result
       entries.

   4 - If the server supports this sorting control but for some reason
       cannot sort the search results using the specified sort keys and
       the client specified FALSE for the control's criticality field,
       then the server should return all search results unsorted and
       include the sortKeyResponseControl in the searchResultDone
       message.

So, (3) the control has an effect (returning sortKeyResponseControl)
even when the operation fails and unavailableCriticalExtension is
returned, and (4) if the server implements the control, there is no
such thing as the control having no effect when the criticality is
FALSE - even if the real point which the control was designed for is
not achieved.

Maybe it's less messy - but still ugly - to restate the control spec
as follows:

  The server SHOULD do this:

  - the control always succeeds if it is supported.  Its effect
    is to _attempt_ to sort the entries (according to a point I
    didn't quote) and to set the sortKeyResponseControl field in the
    result.

  - if sorting failed, the effect of setting criticality=TRUE is to
    return no entries, and to return unavailableCriticalExtension
    even though the control succeeded.

    (Does that mean the control succeeded even though operation
    failed?  Or does it mean that the operation succeeded but
    returned an error anyway?  My head hurts...)

It's a bit more complicated because of the rest of section 2, but
not significantly, I believe.

-- 
Hallvard