IGP Extension for Path Computation Element Communication Protocol (PCEP) Security Capability Support in PCE Discovery (PCED)
draft-ietf-lsr-pce-discovery-security-support-13

        A receiving entity MUST NOT interpret invalid UTF-8 sequences.

What must it do then, when encountering invalid UTF-8 ?


        In any case, an implementation SHOULD [...]

So not in ANY case then? :-)

nits:
                        1                   2                   3
    0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |              Type = 6         |             Length            |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |    KeyID      |                 Reserved                      |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

      Type: 6

      Length: 4


Why does it not say "Length = 4" like it says "Type = 6"  ?

Thank you to Yaron Sheffer for the SECDIR review.

** Section 7.  In the spirit of inclusive language, s/enable a man-in-the-middle attack/enable an on-path attack/.

I started ballotting DISCUSS on this, but, surprisingly, "You made Warren sad" isn't actually one of the DISCUSS criteria, and so I'm (grudgingly and with bad grace) balloting NoObj instead.

----
6.  Management Considerations

   A configuration option may be provided for advertising and
   withdrawing PCEP security capability via OSPF and IS-IS.
----

This section seems more than pointless to me - it seems (admittedly very slightly!) harmful.
It doesn't actually *say* anything useful, but the very act of it showing up in the index / table of contents gives the impression that there may be actually Management Considerations text somewhere below.
This initially made me all excited, and set my heart a flutter -- only to be crushed when I actually read it.

Please consider ripping the section out - AFAICT, it doesn't accomplish anything, other than leading to false hope...

# Éric Vyncke, INT AD, comments for draft-ietf-lsr-pce-discovery-security-support-11

CC @evyncke

Thank you for the work put into this document. 

Please find below some non-blocking COMMENT points (but replies would be appreciated even if only for my own education).

Special thanks to Acee Lindem for the shepherd's detailed write-up including the WG consensus *and* the justification of the intended status, but we miss the WG reaction on the IPR disclosure (see below).

Please note that Suzanne Woolf is the Internet directorate reviewer (at my request) and you may want to consider this int-dir reviews as well when Suzanne will complete the review (no need to wait for it though):
https://datatracker.ietf.org/doc/draft-ietf-lsr-pce-discovery-security-support/reviewrequest/16328/

I hope that this review helps to improve the document,

Regards,

-éric

## COMMENTS

### IPR

The shepherd's write-up rightfully states that IPR disclosures were done (e.g., https://datatracker.ietf.org/ipr/5027/). But, the write-up says nothing about the WG reaction on a licensing scheme that it rather ambiguous `Reasonable and Non-Discriminatory License to All Implementers with Possible Royalty/Fee` as the "possible royalty/fee" could hinder the deployment and use of this I-D.

What was the WG reaction ?

### Section 1

The first paragraph mentions privacy, which is important but I would have assumed that integrity was even more important. Should integrity be mentioned ?

It is probably obvious, but should the change of registry names be linked to supporting IS-IS as well ?

### Section 3.2.1 (and 3.3.1)

The section would benefit of a simple figure showing the TLV structure, even if only to be consistent with section 3.2.2

### Normative references

Unsure whether RFC 5925, 5926, and others are really normative as I would qualify them as informative.

## Notes

This review is in the ["IETF Comments" Markdown format][ICMF], You can use the
[`ietf-comments` tool][ICT] to automatically convert this review into
individual GitHub issues. 

[ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md
[ICT]: https://github.com/mnot/ietf-comments

# GEN AD review of draft-ietf-lsr-pce-discovery-security-support-11

CC @larseggert

## Comments

### Inclusive language

Found terminology that should be reviewed for inclusivity; see
https://www.rfc-editor.org/part2/#inclusive_language for background and more
guidance:

 * Term `master`; alternatives might be `active`, `central`, `initiator`,
   `leader`, `main`, `orchestrator`, `parent`, `primary`, `server`
 * Term `man`; alternatives might be `individual`, `people`, `person`

## Nits

All comments below are about very minor potential issues that you may choose to
address in some way - or ignore - as you see fit. Some were flagged by
automated tools (via https://github.com/larseggert/ietf-reviewtool), so there
will likely be some false positives. There is no need to let me know what you
did with these suggestions.

### URLs

These URLs in the document can probably be converted to HTTPS:

 * http://www.unicode.org/unicode/reports/tr36/

### Grammar/style

#### "Abstract", paragraph 1
```
 for OSPF and IS-IS respectively. However these specifications lack a method
                                  ^^^^^^^
```
A comma may be missing after the conjunctive/linking adverb "However".
(Also elsewhere.)

#### Section 1, paragraph 5
```
ry" instead of the "IGP registry" where as [RFC8623] and [RFC9168] uses the
                                  ^^^^^^^^
```
Did you mean "whereas"?

#### Section 3.2.2, paragraph 3
```
 string to be used to identify the key chain. It MUST be encoded using UTF-8.
                                   ^^^^^^^^^
```
This word is normally spelled as one. (Also elsewhere.)

#### Section 5, paragraph 4
```
 enable a man-in-the-middle attack. Thus before advertising the PCEP security
                                    ^^^^
```
A comma may be missing after the conjunctive/linking adverb "Thus".

## Notes

This review is in the ["IETF Comments" Markdown format][ICMF], You can use the
[`ietf-comments` tool][ICT] to automatically convert this review into
individual GitHub issues. Review generated by the [`ietf-reviewtool`][IRT].

[ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md
[ICT]: https://github.com/mnot/ietf-comments
[IRT]: https://github.com/larseggert/ietf-reviewtool

Discuss cleared, thanks for accommodating my concerns.