Subject: [Gen-art] Gen-art review of draft-zeilenga-ldap-ext-09.txt

From: Elwyn Davies <[email-masked]>
Date: Wed, 04 Jan 2006 13:13:47 +0000

I was selected as General Area Review Team reviewer for this specification
(for background on Gen-ART, please see
http://www.alvestrand.no/ietf/gen/art/gen-art-FAQ.html).

Document: draft-zeilenga-ldap-ext-09.txt
Intended Status: BCP (Individual Submission)
Shepherding AD: Ted Hardie
Review Trigger: IETF Last Call, ending 4 january 2006

Summary:
Assuming that numbers of extensions to LDAP are expected, this seems a 
useful piece of work.  I am not an LDAP expert so it is difficult for me 
to judge the correctness or degree of completeness of the work but it 
'feels' like it covers the field.

In general, the document appears ready to go to the IESG, but I think it 
would be worth reviewing the prescriptive items, which fall into three 
separate categories - see the full review, and using different language 
for each category.  Amongst these prescriptions, there are a few 
constraints that really belong in the protocol specification in my opinion.

Review:

[This next long winded bit is basically a way of saying that (mainly) 
SHALL and SHALL NOT are overloaded to mean subtly different things and 
it might be better if they were differentiated.]

In my view the guidelines could be classified into
1. Motherhood and apple pie both for general protocol design and IETF 
specifications,
2. Advice on preferred routes where there might be multiple choices,
3. Attempts to forbid specific really stupid ideas,
4. Requirements on designers to document reasoning (e.g., para 2 in each 
of s3.1.1, s3.1.2), and
5. Outright constraints.

The desirability of using RFC2119 requirements language capitalization 
for describing advice in a meta-specification is arguable.  However, 
types 3, 4 and 5 all use the prescriptive SHALL/SHALL NOT and MUST/MUST 
NOT language which is a little confusing as they are requiring 
qualitatively different things.

In general terms I have no problem with types 1, 2 and 3. 

Regarding the type 5, these items are so prescriptive of extension 
specification or behaviour that I think they belong in the main protocol 
definition rather than in this document (although reference could 
obviously be made here).  As is said in s1, LDAP is intended to be 
extensible, and if there are things that extensions MUST or SHALL do 
then the definitive statement does't belong in the BCP, which is about 
choices.  For example, s3.3:

  Extensions SHALL use IntermediateResponse messages instead of
  ExtendedResponse messages to return intermediate results.

Similarly para 2 of s4.1, first half of last para of s4.3 and s6.2.

Type 4 occurs, for example, in para 2 of s3.1.1 and s3.1.2:

  Specifications detailing controls extending the Bind operation SHALL
  detail that the Bind negotiated security layers do not protect the
  information contained in these controls and SHALL detail why this
  protection is not needed.

This is about documenting design decisions (a meta-meta-specification 
maybe?). I think using 'needs to explain' rather than 'SHALL detail' is 
more helpful.

The remaining Type 3 (don't do this because it is really stupid) items 
might just be better said as 'Do not do X' rather than using 'SHALL 
NOT'.  Ultimately you can't prevent it if stupid designers and dumb 
reviewers conspire to allow it. A few words of justification might help 
in a couple of cases.

Couple of other minor points:

s2.2, para 1: 'Designers SHOULD consider...'.  I would have said if 
there was ever a case for MUST it is here!

s4.4
  Designers SHOULD avoid introducing extensions which rely on
  unsuspecting implementations to ignore trailing components of SEQUENCE
  whose tags they do not recognize.
I don't understand what this is trying to forbid.

Editorial:

s1: Expand acronyms URL, ASN (yawn).

s1.1, para 2: s/client/a client/, s/server generated/a server generated/

s2.3, para 1: s/discover/discovery/
s2.3, para 2: s/this discover/this discovery/

s2.4: s/repitor/repertory/

[Aside: s2.4, para 2: Does the update of RFC3066  have any implications 
for RFC3866?]

s2.5: s/is described/are described/
s2.5: s/specific contains/specific contents/

s2.7: Expand acronym DN.

s2.8: s/SHALL extended/SHALL extend/

s3, para 1: I think the last piece 'extended operation instead' is 
confusing.  I suggest s/existing operations/existing operations wherever 
it is appropriate/, s/extended operation/extra, new operation using the 
extended operations mechanism/

s3.1, para 1: s/Controls... is/Controls... are/
s3.1, para 2: s/unless unless/unless/

s3.1.1, last para: s/controls semantics/controls' semantics/

s3.1.3, last para: s/apply in./apply./, s/finding phasing/finding phase/

s3.1.4, para 1: s/concistency/consistency/
s3.1.4, para after bullets: s/wether/whether/, s/or separate/or a separate/

s4, para 1: s/to made/to be made/

s4.1, last para:s/server/the server/

s4.2: s/protocolOp/protocol/

s5.2, last para: s/In additional/In addition/

s6.1: s/hyphen/hyphens/

_______________________________________________
Gen-art mailing list
[email-masked]
https://www1.ietf.org/mailman/listinfo/gen-art

---

From: "Kurt D. Zeilenga" <[email-masked]>
Date: Wed, 11 Jan 2006 12:16:56 -0800

Elwyn,

Thanks for the review.   My comments below indicate how I believe
the specific issues you raise should be addressed.

-- Kurt

At 05:13 AM 1/4/2006, Elwyn Davies wrote:
>Assuming that numbers of extensions to LDAP are expected, this seems a useful piece of work.  I am not an LDAP expert so it is difficult for me to judge the correctness or degree of completeness of the work but it 'feels' like it covers the field.
>
>In general, the document appears ready to go to the IESG, but I think it would be worth reviewing the prescriptive items, which fall into three separate categories - see the full review, and using different language for each category.  Amongst these prescriptions, there are a few constraints that really belong in the protocol specification in my opinion.
>
>Review:
>
>[This next long winded bit is basically a way of saying that (mainly) SHALL and SHALL NOT are overloaded to mean subtly different things and it might be better if they were differentiated.]
>
>1. Motherhood and apple pie both for general protocol design and IETF specifications,
>4. Requirements on designers to document reasoning (e.g., para 2 in each of s3.1.1, s3.1.2), and
>5. Outright constraints.
>
>The desirability of using RFC2119 requirements language capitalization for describing advice in a meta-specification is arguable.  However, types 3, 4 and 5 all use the prescriptive SHALL/SHALL NOT and MUST/MUST NOT language which is a little confusing as they are requiring qualitatively different things.
>
>Regarding the type 5, these items are so prescriptive of extension specification or behaviour that I think they belong in the main protocol definition rather than in this document (although reference could obviously be made here).  As is said in s1, LDAP is intended to be extensible, and if there are things that extensions MUST or SHALL do then the definitive statement does't belong in the BCP, which is about choices.

There are, I believe, reasonable justification for including
each of these imperatives.

In particular, in LDAPBIS WG discussions, the WG determined that
providing "guidance to extension designers" was viewed as generally
beyond the scope of the revised LDAP TS.  It was felt that the
protocol technical specification should not itself restrict
the scope of extensions and, as practices change over time,
they should be detailed independently.

>For example, s3.3:

The revised LDAP TS is clear that IntermediateResponse messages
are to be used to return intermediate results.  However, because
the older LDAP TS stated ExtendedResponse messages were to be
used to return intermediate results, this statement was included
in the guidelines to ensure that all, including extension
designers following (for whatever reason) the older LDAP TS
instead of the revised LDAP TS, adhere to the requirements of
the revised LDAP TS (assuming the adhere to this I-D's guidance).
That is, we didn't want to rely on extension designers ability
to see that the older LDAP TS is obsoleted by the revised LDAP
TS.

>Similarly para 2 of s4.1,

This is stating a practice, not a protocol requirement.
 
Extensions are allowed to alter the semantics of the protocol
in most any way they see fit, including overloading the
semantics of result codes.   But as overloading result
code semantics can lead to problems, such as when combining
multiple extensions which rely on differing result code
semantics, this is considered a bad practice.

>first half of last para of s4.3 and s6.2.

The LDAP TS allows new messages to be added by extension.
s4.3 states that extensions specifications should generally
avoid adding new messages, a practice.  As noted above,
the LDAP WG concluded that such practices should be
stated in a document separate from the LDAP TS.

s6.2 states that extensions are to use authzIds to represent
authorization identities in LDAP extensions as it is
LDAP's extension identity form.  This is not a protocol
restriction as the protocol itself just carries extension
information, but a best practice.  And as it possible that
in the future some other practice might be considered best,
we detail them independently of the protocol specification
so that they can be more easily updated or worked around.

>This is about documenting design decisions (a meta-meta-specification maybe?).

This paragraph is about documenting a particular security consideration
(which may involve design decisions).

>I think using 'needs to explain' rather than 'SHALL detail' is more helpful.

I think the following might be more clear as to the intent:
 Specifications detailing controls extending the Bind operation SHALL
 detail that the Bind negotiated security layers do not protect the
 information contained in these controls and SHALL detail how     
 the information in these controls is to be protected or why the           
 information does not need protection.

That intent being that bind control extension specifications
need to:
        1) clearly state that Bind negotiated security layers
        do not protect Bind controls,
        2) clearly state either:
          a) how information carried in the Bind
          controls are to be protected,
          b) why information carried in the Bind controls needs
          no protection.

>The remaining Type 3 (don't do this because it is really stupid) items might just be better said as 'Do not do X' rather than using 'SHALL NOT'.  Ultimately you can't prevent it if stupid designers and dumb reviewers conspire to allow it. A few words of justification might help in a couple of cases.

I would specific instances to take action.

>s2.2, para 1: 'Designers SHOULD consider...'.  I would have said if there was ever a case for MUST it is here!

The reason why this is a SHOULD, not a MUST, is that a MUST
could easily abused by those opposing publication of an
extension by requiring designers to consider various
ill-conceived or not-well-specified extensions.

For instance, I was once asked to consider "multi-master
replication" extension in a document detailing LDAP transactions.
As X.500/LDAP is explicitly single master, I choose not to
consider multi-master replication extension interaction.

>I don't understand what this is trying to forbid.

Avoid not forbid (it's a SHOULD not a MUST).

The revised LDAP TS says:
  Because of the implied extensibility,
   clients and servers MUST (unless otherwise specified) ignore trailing
   SEQUENCE components whose tags they do not recognize.

which opens the door to extensions to use such trailing
components.  However, as it's known that some implementors
in the past have not properly ignored such trailing
components, the current best practice in designing new
extensions is not to rely on such trailing components.

>Editorial:

okay.

>[Aside: s2.4, para 2: Does the update of RFC3066  have any implications for RFC3866?]

If it does, it would be an issue for a future RFC3066bis effort.

>s4.2: s/protocolOp/protocol/

protocolOp is correct (protocolOp refers a particular CHOICE
in the ASN.1).