Subject: [Gen-art] review of draft-ietf-pkix-authorityclearanceconstraints-02.txt

From: Francis Dupont <[email-masked]>
Date: Wed, 12 Aug 2009 13:07:08 +0200

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

Please resolve these comments along with any other Last Call comments 
you may receive. 

Document: draft-ietf-pkix-authorityclearanceconstraints-02.txt
Reviewer: Francis Dupont
Review Date: 2009-08-10
IETF LC End Date: 2009-08-14
IESG Telechat date: unknown

Summary: Not Ready

Major issues: the I-D is too hard to read

Minor issues: 
 - IMHO a transition paragraph is needed at the end of the Introduction
  in order to introduce technical dependencies:
  * clearance attribute is in fact from 3281bis (this is obvious when
  one reads the ASN.1 module appendix but it should be mentioned as
  soon as possible)
  * the processings augment the RFC 5280 section 6 (so the text is
  understable only with this section in mind)
 The whole idea is to prepare a first reader (IMHO it is a problem when
 a document needs to be read more than once to get a good idea about
 what it specifies :-).

 - another issue is the multiple values in a Clearance attribute.
  The Clearance attribute syntax of section 2 is in fact for an
 AttributeValue type and doesn't include multiple values (only
 multiple SecurityCategory). Of course the Attribute in AC can
 contains multiple values, so the text often uses the term "value"
 in a very ambiguous way.

Nits/editorial comments: 
 - Abstract page 2: TA and AA abbrevs are useless, CA abbrev must be
  expanded. Note abbrevs are useless in abstracts if they aren't
  possible keywords or very common (the RFC Editor has a list of
  common abbrevs with very common, cf PS). In no case the Abstract
  can introduce an abbrev for the body.

 - ToC page 3: Author's Addresses -> Authors' Addresses

 - Introduction 1: please introduce abbrevs (PKC, PKI, CA, AA, TA)

 - 3 page 6: I don't understand this statement:
  "In 
   addition, each Clearance attribute in the SEQUENCE must not contain 
   more than one value."
  perhaps SEQUENCE should be sequence (of AuthorityClearanceConstraints)?

 - 4.1.1.2 page 8: ,, -> ,

 - 4.1.1.2 page 8: can't understand:
   If any of the Clearance attributes in the permitted-clearances 
   contains more than one value

 - 4.1.1.3 page 8: 6. -> 6 (for uniformity among the text)

 - 4.1.1.5.1 page 9:
  in "If the permitted-clearances has special value of all-clearances, exit 
  with success." what about the effective-clearance (unchanged?)

 - 5.1.1 page 11: [RFC5280 -> [RFC5280]

 - 8 page 15: what is id-TBSL?

 - Author's Addresses page 19:
  Author's Addresses -> Authors' Addresses
  EMail/Email: choose one of them (EMail seems to be the standard)

Regards

[email-masked]

PS: http://www.rfc-editor.org/rfc-style-guide/abbrev.expansion.txt
PPS: it should be fine to get an implementation report (not required
for Proposed Standards but in this case it should show there is no
trivial mistakes in the processing stuff).

---

From: Sean Turner <[email-masked]>
Date: Wed, 12 Aug 2009 13:46:33 -0400

Francis,

Thanks for your review.  Responses inline.

spt

>   one reads the ASN.1 module appendix but it should be mentioned as

I agree that's why it's in the last sentence of the 1st paragraph in the 
intro ;)

>   understable only with this section in mind)

It says this in section 4.1 and 5.1, but we could move something to the 
intro to explain that we're augmenting the 5280/3281bis processing 
rules. How about:

This document augments the certification path validation rules for PKCs 
in [RFC5280] and ACs in [RFC3281bis].

>  contains multiple values, so the text often uses the term "value"

We restrict the number of times clearance can be included in a 
certificate to one or zero. We also restrict it to be single valued.

Are you referring to the Authority Clearance Constraint extension that 
can include multiple values?

>   can introduce an abbrev for the body.

We'll expand abbreviations on their first occurrence in the main body. I 
think they're all expanded by the end of the Introduction

>  - ToC page 3: Author's Addresses -> Authors' Addresses

Fixed.

>  - Introduction 1: please introduce abbrevs (PKC, PKI, CA, AA, TA)

Fixed.

>   perhaps SEQUENCE should be sequence (of AuthorityClearanceConstraints)?

SEQUENCE refers to the ASN.1 construct for the extension.  We didn't 
capitalize sequence previously so we'll switch it to lower case.  Note 
the must ought to be MUST.

>  - 4.1.1.2 page 8: ,, -> ,

Fixed.

>    contains more than one value

This check makes sure there is only one value in permitted-clearances.

>  - 4.1.1.3 page 8: 6. -> 6 (for uniformity among the text)

Fixed.

>   with success." what about the effective-clearance (unchanged?)

There's no need to set this value as it's the special case where it 
matches all.

>  - 5.1.1 page 11: [RFC5280 -> [RFC5280]

Fixed.

>  - 8 page 15: what is id-TBSL?

It stands for To Be Supplied Later.  I guess now would be a good time ;) 
  I need to get an OID from Russ we'll add this in the next version.

>   EMail/Email: choose one of them (EMail seems to be the standard)

Fixed.

---

From: Francis Dupont <[email-masked]>
Date: Sun, 16 Aug 2009 18:44:11 +0200

=> well, I missed it (:-).

   
   
=> fine

   
=> this is what I understood but there are some places in the document
where the term value is used about the clearance AttributeValue type,
for instance inside AuthorityClearanceConstraints.

=> an extension can contain only one value embedded inside an OBJECT
STRING, and it is forbidden (RFC 5280 4.2) to have multiple extensions
with the same OID. So IMHO extensions and multiple values are exclusive.

   
=> but a AuthorityClearanceConstraints contains clearances, no clearance
attributes, so what does mean the more than one value?

   
=> the permitted-value is either all-clearances or a
AuthorityClearanceConstraints, so there is no Clearance attributes
in it, nor a value...

   
=> the output is both the effective-clearance (with the - everywhere,
including in 4.1.1.6) and success/failure, so all exists must specify
both.

   
=> until you'll get one IMHO a comment should explain this
(only TBD is recognized :-).

To fix my main concern about the term "value", as ASN.1 is not LISP (:-)
I propose to typecheck all types where the term is used and to keep
it when the type can contain a value (typically contain an attribute),
remove it if it can't or replace it by SecurityCategory (or other?)
if it is what you'd like to mean.

PS: occurences of value:

Abstract
   The Authority Clearance Constraints certificate extension values in a 
   Trust Anchor (TA), CA public key certificates, and an Attribute 
   Authority (AA) public key certificate in a public key certification 
   path constrain the effective Clearance of the subject.   

=> correct

1
   Organizations that have implemented a security policy can issue 
   certificates that include an indication of the clearance values held 
   by the subject.

=> correct

1
   Some organizations have multiple TAs, CAs, and/or AAs and these 
   organizations may wish to indicate to relying parties which clearance 
   values from a particular TA, CA, or AA should be accepted.

=> correct

1
   a security policy has been defined for Amoco with three security 
   classification values (HIGHLY CONFIDENTIAL, CONFIDENTIAL, and 

=> correct

1
   Cross-certified domains can also make use of the Authority Clearance 
   Constraints certificate extension to indicate which clearance values 
   should be acceptable to relying parties. 

=> correct

2
   If the Clearance attribute 
   is present, it must contain a single value. 

=> correct

2
     SecurityCategory ::= SEQUENCE { 
       type  [0]   
           TYPE-IDENTIFIER.&id({SupportedSecurityCategories}), 
       value [1] 
           EXPLICIT TYPE-IDENTIFIER.&Type 
                                 ({SupportedSecurityCategories}{@type}) 
     } 

=> correct

2
     - classlist identifies the security classifications. Six basic 
      values are defined in bit positions 0 through 5 and more may be 
      defined by an organizational security policy. 

=> correct

3
   The syntax for Authority Clearance Constraints certificate extension 
   contains Clearance values that the CA or the AA asserts.

=> correct but IMHO misleading. I propose to remove the word values.

3
   In 
   more than one value. 

=> incorrect (no attribute, lower case SEQUENCE, upper case must,
no multiple values).

4
   When 
   processing begins, permitted-clearances is initialized to the user 
   input value or special value all-clearances if Authority Clearance 
   Constraints user input is not provided.

=> correct

4
   When processing the end PKC, the value in the Clearance attribute in 
   the end PKC is intersected with the permitted-clearances state 
   variable. 

=> correct (note it is a case the value can be multiple)

4.1.1.2
  If user input includes AuthorityClearanceConstraints, set the 
   permitted-clearances to the input value, otherwise,, set the 
   permitted-clearances to special value all-clearances. 

=> correct

4.1.1.2
   If any of the Clearance attributes in the permitted-clearances 
   contains more than one value, set effective-clearance to an empty 
   list, set error code to "multiple values in input", and exit with 
   failure. 

=> incorrect (no attribute, no multiple value)

4.1.1.5.1
   Examine effective-clearance and verify that it does not contain more 
   than one value.  If effective-clearance contains more than one value, 
   set effective-clearance to an empty list, set error code to "multiple 
   values", and exit with failure. 

=> correct (the effective-clearance contains Clearance attribute)
but ambiguous: the effective-clearance is a list so it can have more
than one element (even there is no operation described in the document
which allows more than one element, IMHO it will be far better to
make effective-clearance a Clearance attribute or the special value
no-clearance (in place of empty list)).

4.1.1.5.1
   If the permitted-clearances has special value of all-clearances, exit 
   with success. 

=> correct

4.1.1.5.1
   Assign those classList bits in effective-clearance a value of one (1) 
   that have a value of one (1) both in effective-clearance and in the 
   clearance structure in permitted-clearances associated with policyID 
   X.  Assign all other classList bits in effective-clearance a value of 
   zero (0). 
   If none of the classList bits have a value of one (1) in effective-
   clearance, set effective-clearance to an empty list and exit with 
   success. 

=> correct

5
   When processing begins, permitted-clearances 
   is initialized to user input or special value all-clearances if 
   Authority Clearance Constraints user input is not provided.

=> correct

5
   When processing the AC, the value in the Clearance attribute in the 
   AC is intersected with the permitted-clearances state variable. 

=> correct (possible multiple values)

6
   If any of the Clearance attributes in the 
   AuthorityClearanceConstraints extension contains more than one value, 
   set effective-clearance to an empty list, set error code to "multiple 
   values", and exit with failure. 

=> incorrect (no attribute, etc)

6
   If the AuthorityClearanceConstraints certificate extension is not 
   present in the PKC, no action is taken, and the permitted-clearances 
   value is unchanged.

=> correct

6
   If the AuthorityClearanceConstraints certificate extension is present 
   in the PKC and permitted-clearances contains the all-clearances 
   special value, then assign permitted-clearances the value of the 
   temp-clearances. 

=> correct

6
   If the AuthorityClearanceConstraints certificate extension is present 
   in the PKC and permitted-clearances does not contain the all-
   clearances special value, take the intersection of temp-clearances 
   and permitted-clearances by repeating the following steps for each 
   clearance in the permitted-clearances state variable: 

=> correct

6
      -- For every classList bit, assign the classList bit a value of 
          one (1) for the policyID in permitted-clearances state 
          variable if the bit is one (1) in both the permitted-
          clearances state variable and the temp-clearances for that 
          policyID; otherwise assign the bit a value of zero (0). 

=> correct

7
     2. If there is an element in SET B with the same Type OID and value 
        as in the element in SET A, carry out the following steps: 

          a) Add an element containing the Type OID and the value to the 
             SET temp-set. 

          b) Delete all elements with the same Type OID and the same 
             value from the SET B. 

=> correct (securityCategories)

7
     4. Perform Type OID specific intersection of the value in the 
        element in SET A with the values in the applicable elements in 
        SET B with the same Type OID. 

=> correct (securityCategories)

7
    5. If the intersection is not empty, add and element containing the 
        Type OID and intersection result as value to temp-set. 

=> correct (BTW and element -> an element)

8
  SecurityCategory ::= SEQUENCE { 
       type  [0]  OID id-TBSL, 
       value [1]  BIT STRING 
     }  

   Note that Type specific intersection of two values for this Type will 
   be simply setting the bits that are set in both values.  If the 
   resulting intersection has none of the bits set, the intersection is 
   considered empty. 

=> correct

A
   --   value [1] 

=> correct

BTW the permitted-clearances type is loose, IMHO it is the sum of
AuthorityClearanceConstraints considered as a list of clearances
and the special value all-clearances. As it is not a certificate
extension, "set the permitted-clearances to AuthorityClearanceConstraints
certificate extension" is at least a short-cut (:-).

Regards

---

From: Sean Turner <[email-masked]>
Date: Thu, 20 Aug 2009 16:37:15 -0400

> => well, I missed it (:-).

Well I missed your response.  Somebody had to tell me to respond so 
we're  even ;)

> with the same OID. So IMHO extensions and multiple values are exclusive.

Okay now I get it.  We'll work on a revision to address this.

> attributes, so what does mean the more than one value?

The sentence in question is going to get deleted.

> AuthorityClearanceConstraints, so there is no Clearance attributes

I think this will end up getting deleted too.

> including in 4.1.1.6) and success/failure, so all exists must specify

I don't think we're going to change this one.  effective-clearance has 
no meaning when it's set to all-clearances.

> (only TBD is recognized :-).

Fair enough.

> if it is what you'd like to mean.

We'll go through these to fix the ones that make sense after we 
incorporate the changes for the value text.

> [email-masked]

I delete the ones we got right ;)

> => correct but IMHO misleading. I propose to remove the word values.

Agreed

> => incorrect (no attribute, lower case SEQUENCE, upper case must,

Sentence to be deleted.

> => incorrect (no attribute, no multiple value)

Sentence to be deleted.

> => incorrect (no attribute, etc)

Sentence to be deleted.

---

From: "Santosh Chokhani" <[email-masked]>
Date: Thu, 20 Aug 2009 16:16:10 -0400

Francis,

Good catch on Authority Clearance Constraints not asserting clearance as
an attribute.

We are fixing the I-D. 

> -----Original Message-----
> From: [email-masked] [mailto:[email-masked]] 
> Sent: Sunday, August 16, 2009 12:44 PM
> To: Sean Turner
> Cc: [email-masked]; Santosh Chokhani; [email-masked]
> Subject: Re: review of 
> draft-ietf-pkix-authorityclearanceconstraints-02.txt 
> 
> => this is what I understood but there are some places in the 
> document where the term value is used about the clearance 
> AttributeValue type, for instance inside 
> AuthorityClearanceConstraints.
> 
> => an extension can contain only one value embedded inside an 
> OBJECT STRING, and it is forbidden (RFC 5280 4.2) to have 
> multiple extensions with the same OID. So IMHO extensions and 
> multiple values are exclusive.
> 
> => but a AuthorityClearanceConstraints contains clearances, 
> no clearance attributes, so what does mean the more than one value?
> 
> permitted-clearances 
> AuthorityClearanceConstraints, so there is no Clearance 
> attributes in it, nor a value...
> 
>    >  - 4.1.1.5.1 page 9:
> => the output is both the effective-clearance (with the - 
> everywhere, including in 4.1.1.6) and success/failure, so all 
> exists must specify both.
> 
> To fix my main concern about the term "value", as ASN.1 is 
> not LISP (:-) I propose to typecheck all types where the term 
> is used and to keep it when the type can contain a value 
> (typically contain an attribute), remove it if it can't or 
> replace it by SecurityCategory (or other?) if it is what 
> you'd like to mean.
> 
> => incorrect (no attribute, lower case SEQUENCE, upper case 
> must, no multiple values).
> 
> 4
>    When 
> => correct (the effective-clearance contains Clearance 
> attribute) but ambiguous: the effective-clearance is a list 
> so it can have more than one element (even there is no 
> operation described in the document which allows more than 
> one element, IMHO it will be far better to make 
> effective-clearance a Clearance attribute or the special 
> value no-clearance (in place of empty list)).
> 
> 4.1.1.5.1
> BTW the permitted-clearances type is loose, IMHO it is the 
> sum of AuthorityClearanceConstraints considered as a list of 
> clearances and the special value all-clearances. As it is not 
> a certificate extension, "set the permitted-clearances to 
> AuthorityClearanceConstraints certificate extension" is at 
> least a short-cut (:-).
> 
> Regards
>