Subject: protocol-27 comments #3

From: Hallvard B Furuseth <[email-masked]>
Date: Mon, 8 Nov 2004 11:59:28 +0100

Final (hopefully:-) bunch of protocol comments:

> 4.1.10. Referral

>    If the client wishes to progress the operation, it MUST follow the
>    referral by contacting one of the supported services.

"if the client wishes, then it MUST" sounds strange to me.
I would s/MUST/can/.

>    Protocol peers (...) MUST NOT repeatedly contact the same
>    server for the same request with the same target entry name, scope
>    and filter.

How about different controls (maybe due to LDAPURL extensions) and other
extensions?  (Same question to continuation references in section 4.5.3.)

How about non-search operations?  Maybe this text should just be

     MUST NOT contact the same server for the same request with the
     same parameters.

>    - If an alias was dereferenced, the <dn> part of the URL MUST be
>      present, with the new target object name.

I would prepend "Note that" to the following, and move it below the list
of instructions:

>      UTF-8 encoded characters
>      appearing in the string representation of a DN or search filter
>      may not be legal for URLs (e.g. spaces) and MUST be escaped using
>      the % method in [URI].

>    - If the <dn> part is present, the client MUST use this name in its
>      next request to progress the operation, and if it is not present
>      the client will use the same name as in the original request.

s/will/MUST/ for consistency.

>    - If the <scope> part is missing, the scope of the original search
>      is used by the client to progress the operation.

s/is/MUST be/.  (Or go the other way, and remove MUSTs in this list.)

> 4.1.11. Controls

>    For controls attached to
>    response messages and the unbindRequest, the criticality field SHOULD
>    be FALSE,

Well, the protocol _field_ should not be FALSE but absent (since default
values MUST be absent, by Section 5.2).  The abstract _value_ sent by
the client should be FALSE.  The same applies to the rest of the text.

>    A value
>    of TRUE indicates that it is unacceptable to perform the operation
>    without applying the semantics of the control and FALSE otherwise.

"and FALSE otherwise" looks strange in this sentence - is it left over
from a different wording?

>    - whether information is to be present in the controlValue field,
>      and if so, the format of the controlValue contents,

I think that should be

       whether the controlValue field is to be present, and if so, ...

since if it is present, it does contain information.

> 4.2. Bind Operation

>    Authorization is the decision of which access an operation has to the
>    directory. Among other things, the process of authorization takes as
>    input authentication information obtained during the bind operation
>    and/or other acts of authentication (such as lower layer security
>    services).

Authorization can also - or instead - take as input information which is
not normally considered acts of authentication, such as the IP number of
time of day.

> 4.4. Unsolicited Notification

>    The unsolicited notification is structured as an LDAPMessage in which
>    the messageID is zero and protocolOp is of the extendedResp form (See
>    Section 4.12).

Actually, 4.12 defines ExtendedResponse.  extendedResp is defined in
4.1.1.

>    - the format of the contents (if any) of the responseValue,

move "(if any)" to the end of the sentence (to address an absent value,
not a present but empty value).

>    - the circumstances which will cause the notification to be
>      returned, and
>
>    - the semantics of the operation.

s/returned/sent/.  I think 'returned' means it is some request's return
value.  "operation" seems strange for the same reason.  "message",
maybe?

> 4.4.1. Notice of Disconnection

>    This notification may be used by the server to advise the client that
>    the server is about to close the connection due to an error
>    condition.

s/error condition/exceptional condition/ or something?  I wouldn't call
controlled shutdown of the server or idletimeout of the connection an
error.  Or maybe the question should be avoided:

     ... server is about to close the connection on its own initiative.

> 4.5.1. Search Request

>         SubstringFilter ::= SEQUENCE {
>              type           AttributeDescription,
>              -- initial and final can occur at most once
                                
s/once/once each/?  (I.e. twice:-)

>    - scope: Specifies the scope of the search to be performed. The
>      semantics (as described in [X.511]) of the possible values of this
>      field are:
>    - derefAliases: An indicator as to how alias entries (as defined in
>      [Models]) are to be handled in searching. The semantics of the
>      possible values of this field are:

s/possible/defined/ in these two, since other values can be defined
later.

>         derefInSearching: While searching, dereference any alias entry
>         subordinate to the base object which is also in the search
>         scope. The filter is applied to the dereferenced object(s). If
>         the search scope is wholeSubtree, the search continues in the
>         subtree of any dereferenced object. Aliases in that subtree are
>         also dereferenced.

This seems to say that recursive dereferencing stops at alias entries
that are not subordinate to the base object, and that onelevel searches
do not recursively dereference aliases.

>         derefAlways: Dereference aliases both in searching and in
>         locating the base object of the search.

Add a blank line.

>      The present match evaluates to TRUE where there is an attribute or
>      subtype of the specified attribute description present in an
>      entry, and FALSE otherwise (including a presence test with an
>      unrecognized attribute description.)

s/.)/)./ ?  Not sure how () and punctuation interact in English.

>      Note that the AssertionValue in a substrings filter item conforms

s/AssertionValue/AssertionValues/.

>         If the matchingRule field is absent, the type field MUST be
>         present, and an equality match is performed for that type.

Which means just (:dn:=foo) is not allowed.  Maybe the draft should
suggest that one can use (1.1:dn:=foo).

Also, is it an error to violate this "MUST"?  Most other invalid filters
just return Undefined.

>         If the type field is present and the matchingRule is present,
>         the matchValue is compared against entry attributes of the
>         specified type. In this case, the matchingRule MUST be one
>         suitable for use with the specified type (see [Syntaxes]),
>         otherwise the filter item is Undefined.

This "MUST" looks a bit strange, since it's well-defined what the filter
returns.  There might be a good reason to use a matching rule which one
doesn't know is suitable for use with the type.

> 4.5.2. Search Result

>    The results of the search operation are returned as zero or more
>    searchResultEntry messages, zero or more SearchResultReference
>    messages, followed by a single searchResultDone message.

replace "messages, zero or more" with "and/or" to clarify that the
entries need not precede the references.

>    Each entry returned in a SearchResultEntry will contain all
>    appropriate attributes as specified in the attributes field of the
>    Search Request. Return of attributes is subject to access control and
>    other administrative policy.

I would s/. Return of attributes is/,/

> 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 one or more non-local entries,
>    the server may return one or more SearchResultReference entries, each
>    containing a reference to another set of servers for continuing the
>    operation.

It's not clear to me if "SearchResultReference entries" - which is wrong
since they are protocol elements, not entries - means to imply that each
SearchResultReference represents one entry.  Can NSSRs or something like
them be returned?  (Would that be if one has an entry - maybe a shadow
copy - but does not search (or return) any of its children?)

>    - If the originating search scope was singleLevel, the <scope> part
>      of the URL will be "base".
>
>    - It is RECOMMENDED that the <scope> part be present to avoid
>      ambiguity.

The text doesn't seem to say that the scope defaults to the original
value.  (Nor other aspects below, but I guess that would depend on these
aspects' definitions.)

>    - Other aspects of the new search request may be the same as or
>      different from the search request which generated the
>      SearchResultReference.

> 4.5.3.1. Examples

>    It
>    knows that either LDAP-capable servers (hostb) or (hostc) hold
>    <OU=People,DC=Example,DC=NET>

s/either...or/both...and/, I think.

>      SearchResultReference {
>        ldap://hostb/OU=People,DC=Example,DC=NET??sub
>        ldap://hostc/OU=People,DC=Example,DC=NET??sub }
>      SearchResultReference {
>        ldap://hostd/OU=Roles,DC=Example,DC=NET??sub }
>      SearchResultDone (success)

I suggest you change an example (and the matching text) so a DN on one
host is different from the original DN.

> 4.6. Modify Operation

>    - object: The name of the object to be modified. The value of this
>      field contains the DN of the entry to be modified.

Doesn't this say the same thing twice?  Mimicing the wording from add &
co, it should be

     - object: The name of the entry [not object] to be modified.

If you wish to imply that the name is a DN, I suggest 'The name (DN)
of...' both here and elsewhere.

>            delete: delete values listed from the modification attribute,
>            removing the entire attribute if no values are listed, or if
>            all current values of the attribute are listed for deletion;

Suggest /if no values are listed, or if/if either values are listed or/.

> 4.7. Add Operation

>    - attributes: (...)

>      Clients MUST
>      include the 'objectClass' attribute, and values of any mandatory
>      attributes of the listed object classes. Clients MUST NOT supply
>      NO-USER-MODIFICATION attributes such as the createTimestamp or
>      creatorsName attributes, since the server maintains these
>      automatically.

The same applies to Modify Operation.  Maybe factor that out and replace
with a reference to [Models], similar to what has been done with some
Modify Operation text?

> 4.9. Modify DN Operation

>    - newrdn: (...) If the operation moves the entry
>      to a new superior without changing its RDN, the value of the old
>      RDN is supplied for this parameter.

This sort of implies that Something (the server?) modifies this
parameter if one does that operation, instead of the other way around.
Maybe:

       To move the entry to a new superior without changing its RDN, the
       client can supply the value of the old RDN for this parameter.

> 4.13. IntermediateResponse Message

>    - the format of the contents of the responseValue, and

s/responseValue/responseValue (if any)/.

> 6. Security Considerations

>    This version of the protocol provides facilities for simple
>    authentication using a cleartext password, as well as any [SASL]
>    mechanism. Installing SASL

and TLS  [or did I say that before?]

>    layers can provide integrity and other
>    data security services.

> A.1 Non-Error Result Codes

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

s/is required/needs/, since the client is free to not follow referrals.

-- 
Hallvard