Subject: [Gen-art] Gen-art LC review of draft-ietf-pce-of-05

From: Robert Sparks <[email-masked]>
Date: Thu, 18 Dec 2008 15:21:24 -0600

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-pce-of-05
Reviewer: Robert Sparks
Review Date: 18 Dec 2008
IETF LC End Date: 23 Dec 2008
IESG Telechat date: not known

Summary: Mostly Ready but some nits

Comments:

The security consideration section is pretty light (particularly given  
how long this set of documents appears to have been cooking). Are  
there really no additional considerations over what's in [PCEP]  
introduced by this document? A couple of sentences further supporting  
that (if it's the case) would go a long way.

This is probably a naive question, but what happens if a PCC sets the  
OF flag in a request, doesn't provide a OF object, and the OF object  
in the response has a function code the client has never heard of  
before? (If this is clear, where in the document suite is it captured?)

Also (and to check my understanding of how extensibility of the flags  
field in the RP object is supposed to work) - it's possible for the OF  
flag to be set in a request but for the response to not contain an OF  
object (if the PCE doesn't know about the extension in this draft it  
MUST ignore the non-zero flag bit according to the definitions on page  
30 of [PCEP]). Should this document explicitly mention this case?

This draft uses RBNF (draft-farrel-rtg-common-bnf). The proto writeup  
indicates that no tool was used to verify the RBNF in the draft. Has  
there been a careful review by a human? (There isn't much of it, but  
enough for a typo or dangling reference to slip in).

I'm probably being dense, but I can't make the literal "0x200" in the  
definition of the OF flag make sense. It doesn't seem to have anywhere  
to go in the registry that [PCEP] creates. Additionally, that registry  
asks for a "Capability Description" and this document only provides a  
"Name" (of OF).

Section 8.2 is, if I read it correctly, a note that more standards  
work needs to happen, not that an implementer needs to do something. I  
suggest not using 2119 language here.

The rest of these comments are minor:

- The abstract does not capture that the document defines/registers  
objective functions/metric types

- There's an extra closing parenthesis in the function on Objective  
Function Code 3: r(Lpi)) <-here

- It would make IANA's job easier to point to the registries this  
document is adding values to (rather than creating) by the name. For  
instance, 6.2.3 of this document could say the "PCEP-ERROR Object  
registry" (assuming that's the name that [PCEP] is trying to get IANA  
to use for the registry it creates in its section 9.12. The [PCEP]  
document itself might benefit from being more explicit with registry  
names it suggests to IANA.

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

---

From: "Adrian Farrel" <[email-masked]>
Date: Thu, 18 Dec 2008 22:20:55 -0000

Hi Robert,

Thanks for reading.

> how long this set of documents appears to have been cooking). Are  there 
> really no additional considerations over what's in [PCEP]  introduced by 
> this document? A couple of sentences further supporting  that (if it's the 
> case) would go a long way.

Sure. I think it is probably the length of cooking that led to the brevity 
of this section.

I guess if we added further text it would look something like...

PCEP security mechanisms are described in [PCEP] and are used to secure 
entire PCEP messages. Nothing in this document changes the message flows or 
introduces any new messages. This document introduces a single new object 
that may optionally be carried on PCEP messages and will be automatically 
secured using the mechanims described in [PCEP]. If a PCEP message is 
vulnerable to attack, for example because security mechanisms are not used, 
then the OF object could be used as part of an attack, however, it is likely 
that other objects will provide far more significant ways of attacking a PCE 
or PCC in this case.

We probably left this out because it was "obvious". Would it address you 
concerns if we added it?

> This is probably a naive question, but what happens if a PCC sets the  OF 
> flag in a request, doesn't provide a OF object, and the OF object  in the 
> response has a function code the client has never heard of  before? (If 
> this is clear, where in the document suite is it captured?)

The PCC is not required to process the OF object on the PCRep.
If the PCC gets a function code it doesn't understand it can:
- ignore it and use the response
- add the function code to its view of the PCE's repetoire for
  later request/exclusion
- discard the response and ask another PCE to compute for it
- discard the response and ask the same PCE to compute
   excluding the function used.

I'm not sure we need to tell implementers how to write code here.

> object (if the PCE doesn't know about the extension in this draft it  MUST 
> ignore the non-zero flag bit according to the definitions on page  30 of 
> [PCEP]). Should this document explicitly mention this case?

Right, this would be the behavior of a legacy node.
The PCC that set the OF flag would receive a PCRep carrying no OF object 
(i.e., in breach of this I-D).
The PCC would either:
- discard the PCRep because it *really* wanted the OF object returned
- shrug its shoulders and continue

Again, I'm not sure that we need to tell people how to write code. This 
certainly doesn't change the protocol behavior onthe wire.

> enough for a typo or dangling reference to slip in).

Did you find any problems?

I wrote RBNF and also reviewed the I-D as doc shepherd. I found some issues 
in an earlier review and these have been fixed. I see no other issues.

||        Has the
||        Document Shepherd personally reviewed this version of the
||        document and, in particular, does he or she believe this
||        version is ready for forwarding to the IESG for publication?
|
|     Adrian Farrel is the document shepherd.
|     He has personally reviewed the I-D and believes it is ready for
|     forwarding to the IESG for publication.

> definition of the OF flag make sense. It doesn't seem to have anywhere  to 
> go in the registry that [PCEP] creates. Additionally, that registry  asks 
> for a "Capability Description" and this document only provides a  "Name" 
> (of OF).

Yes, you're right. The IANA section of the PCEP spec has been updated 
several times in the last month and has left this I-D behind.

I think, based on Section 9.6 of [PCEP] we need to use Bit 25, but we need 
to check other I-Ds in the pipeline.

> Section 8.2 is, if I read it correctly, a note that more standards  work 
> needs to happen, not that an implementer needs to do something. I  suggest 
> not using 2119 language here.

Yes, I'm happy with that.
I'm not sure the Ops ADs are so comfortable :-)

> objective functions/metric types

Add...

This document defines objective function code types for six objective 
functions previously listed in the PCE requirments work.

> Function Code 3: r(Lpi)) <-here

Good catch

> registry" (assuming that's the name that [PCEP] is trying to get IANA  to 
> use for the registry it creates in its section 9.12. The [PCEP]  document 
> itself might benefit from being more explicit with registry  names it 
> suggests to IANA.

Well, that horse has bolted and we don't have names to point to.
So we will have to help IANA as the work goes forward.

Thanks for your review.
Adrian

---

From: Robert Sparks <[email-masked]>
Date: Fri, 19 Dec 2008 10:17:33 -0600

On Dec 18, 2008, at 4:20 PM, Adrian Farrel wrote:

> Hi Robert,
>
> Thanks for reading.
>
>> The security consideration section is pretty light (particularly  
>> given how long this set of documents appears to have been cooking).  
>> Are  there really no additional considerations over what's in  
>> [PCEP]  introduced by this document? A couple of sentences further  
>> supporting  that (if it's the case) would go a long way.
>
> Sure. I think it is probably the length of cooking that led to the  
> brevity of this section.
>
> PCEP security mechanisms are described in [PCEP] and are used to  
> secure entire PCEP messages. Nothing in this document changes the  
> message flows or introduces any new messages. This document  
> introduces a single new object that may optionally be carried on  
> PCEP messages and will be automatically secured using the mechanims  
> described in [PCEP]. If a PCEP message is vulnerable to attack, for  
> example because security mechanisms are not used, then the OF object  
> could be used as part of an attack, however, it is likely that other  
> objects will provide far more significant ways of attacking a PCE or  
> PCC in this case.
>
> We probably left this out because it was "obvious". Would it address  
> you concerns if we added it?

Yes.

>
>
>> This is probably a naive question, but what happens if a PCC sets  
>> the  OF flag in a request, doesn't provide a OF object, and the OF  
>> object  in the response has a function code the client has never  
>> heard of  before? (If this is clear, where in the document suite is  
>> it captured?)
>

No, but making it clear to an implementer that this (and the  
following) are cases they will to have to deal with would be a good  
thing.
Is the list you present above explicit (even if distributed) in other  
documents? - I'm definitely not well-versed in this protocol.
But I'm more interested in making sure it's clear to implementors that  
the conditions can arise than laying out what they have to do about it  
- the
conditions are subtle if you read this document alone.

>
>
>> Also (and to check my understanding of how extensibility of the  
>> flags field in the RP object is supposed to work) - it's possible  
>> for the OF flag to be set in a request but for the response to not  
>> contain an OF object (if the PCE doesn't know about the extension  
>> in this draft it  MUST ignore the non-zero flag bit according to  
>> the definitions on page  30 of [PCEP]). Should this document  
>> explicitly mention this case?
>
> The PCC that set the OF flag would receive a PCRep carrying no OF  
> object (i.e., in breach of this I-D).
> The PCC would either:
> Again, I'm not sure that we need to tell people how to write code.  
> This certainly doesn't change the protocol behavior onthe wire.
see above
>
>
>> This draft uses RBNF (draft-farrel-rtg-common-bnf). The proto  
>> writeup indicates that no tool was used to verify the RBNF in the  
>> draft. Has there been a careful review by a human? (There isn't  
>> much of it, but enough for a typo or dangling reference to slip in).
>

No, but I didn't try to either.

Spot-checking a couple of things:

Some productions use <SVEC-list> but the rule I think they're pointing  
to is <svec-list>. ([PCEP] is similar). Are the rule names supposed to  
be case-insensitive?

The rule you are updating from [PCEP] for PCReq has one of these:  
[<RRO>[<BANDWIDTH>]], which you are replacing with [<RRO>].
Is that intentional?

>
>
> I wrote RBNF and also reviewed the I-D as doc shepherd. I found some  
> issues in an earlier review and these have been fixed. I see no  
> other issues.
>
> ||        Has the
>> I'm probably being dense, but I can't make the literal "0x200" in  
>> the definition of the OF flag make sense. It doesn't seem to have  
>> anywhere  to go in the registry that [PCEP] creates. Additionally,  
>> that registry  asks for a "Capability Description" and this  
>> document only provides a  "Name" (of OF).
>
> Yes, you're right. The IANA section of the PCEP spec has been  
> updated several times in the last month and has left this I-D behind.
>
> I think, based on Section 9.6 of [PCEP] we need to use Bit 25, but  
> we need to check other I-Ds in the pipeline.
>
>> work needs to happen, not that an implementer needs to do  
>> something. I  suggest not using 2119 language here.
>
> This document defines objective function code types for six  
> objective functions previously listed in the PCE requirments work.
should the metrics also be called out?
>> document is adding values to (rather than creating) by the name.  
>> For instance, 6.2.3 of this document could say the "PCEP-ERROR  
>> Object registry" (assuming that's the name that [PCEP] is trying to  
>> get IANA  to use for the registry it creates in its section 9.12.  
>> The [PCEP]  document itself might benefit from being more explicit  
>> with registry  names it suggests to IANA.
>

---

From: "Adrian Farrel" <[email-masked]>
Date: Fri, 19 Dec 2008 16:32:00 -0000

Thanks Robert,
We're converging.

> No, but making it clear to an implementer that this (and the  following) 
> are cases they will to have to deal with would be a good  thing.
> the conditions can arise than laying out what they have to do about it  - 
> the conditions are subtle if you read this document alone.

OK.
These optional behaviors are not described anywhere.
They can be added as "suggested potential actions"

>> certainly doesn't change the protocol behavior onthe wire.

Ditto

> Some productions use <SVEC-list> but the rule I think they're pointing  to 
> is <svec-list>. ([PCEP] is similar). Are the rule names supposed to  be 
> case-insensitive?

Good catch.
Funny how you can stare at something for ages...

Using the same case would be sensible.
I have also flagged this to the RFC Editor for [PCEP].

> [<RRO>[<BANDWIDTH>]], which you are replacing with [<RRO>].

Hmmm.
Looks like PCEP gor updated since this I-D went through the working group.

Yes, we should update this I-D in line with the latest (final?) version of 
PCEP).

Thanks again.
Adrian