more protocol-19 comments

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

draft-ietf-ldapbis-protocol-19.txt says:

> 4.1.1.1. Message ID 

>   For operations that do 
>   not return responses (unbind, abandon, and abandoned operations), the 
>   client SHOULD assume the operation is in progress until a subsequent 
>   bind request completes. 

Maybe 'bind request or TLS closure', since outstanding operations do not
get responses after TLS closure.  Or could reusing these IDs be
troublesome for servers if the operations continue running after the
closure, just without sending responses?

> 4.1.11. Controls 

>   The criticality field is either TRUE or FALSE and only applies to 
>   request messages that have a corresponding response message. For all 
>   other messages (such as abandonRequest, unbindRequest and all 
>   response messages), the criticality field SHOULD be FALSE. 

Why?  What's wrong with a critical control which makes the abandon mean
'abandon this operation only if something or other succeeds'?

Besides, two paragraphs down, the draft does care for criticality=TRUE
in requests withohut response:

>   If the server does not recognize the control type or it is not 
>   appropriate for the operation, and the criticality field is TRUE, the 
>   server MUST NOT perform the operation, and for operations that have a 
>   response, MUST set the resultCode to unavailableCriticalExtension. 


>   This document does not specify any controls. Controls may be 
>   specified in other documents. The specification of a control consists 
>   of: 
>   (...)
>   - whether the control is always non critical, always critical, or 
>     optionally critical, 

A request fails if this condition is violated?

> 4.3. Unbind Operation 

>   Any outstanding operations 
>   on the server are, when possible, abandoned, and when not possible, 
>   completed without transmission of the response. 

Should probably have a separate section which says the same happens if
the connection is lost (e.g. if the client abruptly closes the
connection).

> 4.4.1. Notice of Disconnection 

>   Upon transmission of the UnbindRequest, each protocol peer is to 
>   consider the LDAP association terminated, MUST cease transmission of 
>   messages to the other peer, and MUST close the connection. 

This is either misplaced or should say "Notice of Disconnection" instead
of "UnbindRequest".  I object to the latter:  The client should not be
required to immediately (or at all) detect a Notice of Disconnection.

> 4.5.1. Search Request 

>             -- at least one must be present, 

This comment for 'substrings' is superfluous, since 'substrings'
contains  'SIZE (1..MAX)'.

>     The matching rule for greaterOrEqual and lessOrEqual filter items 
>     is defined by the ORDERING matching rule for the attribute type. 

True for greaterOrEqual, which matches if (! (ORDERING)).
lessOrEqual is defined by EQUALITY + ORDERING: (| (EQUAL) (ORDERING)).
See e.g. [Syntaxes] 4.2.8 (caseIgnoreOrderingMatch).

> 4.5.3. Continuation References in the Search Result 

   -    The <dn> part of the URL MUST be present, (...)
   -    It is RECOMMENDED that the <dn> part be present (...)

Delete one of these.  I don't know which one?

>   Other kinds of URIs may be returned. The syntax and semantics of such 
>   URIs is left to future specifications. Clients ignore URIs that they 
>   do not support. 

Not sure if this needs to be mentioned, but: should it be treated as an
error if a continuation reference consists of only unsupported URIs?

> 4.9. Modify DN Operation 

>   - entry: (...) Note that the server SHALL NOT 
>     dereference any aliases in locating the entry to be changed. 

I think this sentence should be moved to after the argument list,
and apply to both entry and newSuperior.

> 4.11. Abandon Operation 

>   Clients MUST NOT send abandon requests for the same operation 
>   multiple times,

Why not, since the server is required to handle it anyway:

>   Servers MUST discard abandon requests (...) for 
>   operations which have already been abandoned. 


------------------------------------------------------------------------

Editorial comments:

> 4.1.2. String Types 

>   <numericoid> given in Section 1.3 of [Models]. 

Section 1.4.

> 4.1.9. Result Message 

>   The resultCode enumeration is extensible as defined in Section 3.5 of 
>   [LDAPIANA].

Section 3.6.

> 4.2. Bind Operation 

>     string) for the purposes of anonymous binds ([AuthMeth] Section 7) 

Section 6.1.

>     or when using Simple Authentication and Security Layer [SASL] 
>     authentication ([AuthMeth] Section 4.3). Server behavior is 

This is all over [AuthMeth], but not in section 4.3.

>   - authentication: information used to authenticate the name, if any, 
>     provided in the Bind Request. This type is extensible as defined 
>     in Section 3.6 of [LDAPIANA].

Section 3.7.

> 4.2.1. Processing of the Bind Request 
    
>    Before processing a BindResponse,
                         ^^^^^^^^^^^^
                         BindRequest, I think

>   For some SASL authentication mechanisms, it may be necessary for the 
>   client to invoke the BindRequest multiple times.
                     ^^^
delete "the".

>   clients may
>   either unbind (which results in the underlying connection being 
>   closed) or by issuing a bind request and then examining the 
>   BindResponse returned by the server. 

Replace "by issuing (...) examining" with "issue (...) examine".

> 4.5.3. Continuation References in the Search Result 

>   If the server was able to locate the entry referred to by the 
>   baseObject but was unable to search all the entries in the scope at 
                                                                     ^^
>   and subordinate to the baseObject,
    ^^^

"at and"?

> 4.6. Modify Operation 

>   -   operation: Used to specify the type of modification being 
>   -   modification: A PartialAttribute (which may have an empty SET of 

These two paragraphs are indented too much after the '-'.

> 4.7. Add Operation 

>   A response of success indicates that the new entry is present in the 
>   Directory. 

No, that it was added to the directory.  entryAlreadyExists also
indicates presence.

> 4.10. Compare Operation 

>   In the event that the attribute or subtype is not present in the 
>   entry, the resultCode field is set to noSuchAttribute. If the 
>   attribute is unknown, the resultCode is set to 
>   undefinedAttributeType.

Please mention that compareTrue or compareFalse are returned on success.

> 4.12. Extended Operation 

>   - the semantics of the operation, 
                                    ^
".", not ",".  Unless something is missing below.

> 4.13.2. StartTLS Response 

>   response field is absent.  
    ^^^^^^^^
responseValue field.

> 4.13.2.1. "Success" Response 

>   If the StartTLS Response contains a result code of success, this 
>   indicates that the server is willing and able to negotiate TLS. Refer 
>   to Section 5.3 of [AuthMeth] for details. 

Section 4.

> 4.13.2.2. Response other than "success" 

>   -  operationsError:  operations sequencing incorrect; e.g. TLS is 

Indented one space too much after the "-".

>   The server MUST return operationsError if the client violates any of 
>   the StartTLS extended operation sequencing requirements described in 
>   Section 5.3 of [AuthMeth]. 

Section 4, I think.

> 6. Security Considerations 

>   issues are addressed are application 
>   and/or implementation specific. 

Spurious newline.

> 8. Normative References 

>   [AuthMeth] Harrison, R., "LDAP: Authentication Methods and Connection 
>             Level Security Mechanisms ", draft-ietf-ldapbis-authmeth-

Remove space in front of <">.

>   [URI]     Berners-Lee, T., Fielding, R., and L. Masinter Uniform 
>             Resource Identifiers (URI): Generic Syntax", RFC 2396, 

Missing <, "> in front of "Uniform".

> Appendix A - LDAP Result Codes 

>   LDAP result code enumerated in Section 4.1.10. 

4.1.9.

> A.1 Non-Error Result Codes 

>   The success, compareTrue, and compare result codes indicate 
                                  ^^^^^^^
                                compareFalse

>   The referral and saslBindInProgress result codes indicate the client 
>   is required to take additional action to complete the operation 
                                                                  ^^^
Missing period.

> A.2 Result Codes 

>           For example, this code is returned if the client attempts to 
>           StartTLS [RFC2246] (...)

That RFC is TLS 1.0, not StartTLS.

>        referral (10) 
>           Indicates that a referral needs to be chased to complete the 
>           operation (see Section 4.1.11). 

Section 4.1.10.

>        unavailableCriticalExtension (12) 
>           Indicates that the server is unable or unwilling to perform a 
>           critical extension (see Section 4.1.12). 
                     ^^^^^^^^^
                      control       Section 4.1.11.

> C.1.14 Section 4.2 

>   - Required servers to not dereference aliases for bind. This was 
>     added for consistency with other operations and to help ensure 
>     data consistency 
                     ^^^
Missing period.

> C.1.23 Section 4.6 

>   - Removed restriction that required an equality match filter in 
                                           ^^^^^^^^^^^^^^^^^^^^^
                                           EQUALITY mathcing rule

> C.1.26 Section 4.11 
 
>   - Explained that since abandon returns no response, clients hould 
                                                                ^^^^^
                                                                should

-- 
Hallvard