[ippm] Re: AD Review of: draft-ietf-ippm-encrypted-pdmv2

["[e-mail]" <[e-mail]>]

Warren,
Sorry, I am to blame.  We have a new version.  I need to do a final review on it & plan to submit it by Friday morning.
Where does the time go?
Thanks,

Nalini Elkins
CEO and Founder
Inside Products, Inc.
https://www.insidethestack.com
PresidentIndustry Network Technology Councilhttps://www.industrynetcouncil.org 

    On Thursday, June 6, 2024 at 11:34:50 AM PDT, Warren Kumari <[e-mail]> wrote:  
 
 Hi there authors (& WG),

It's been a few weeks - I just wanted to confirm that y'all had received this and were working on it…

W

On Wed, May 22, 2024 at 6:03 PM, Warren Kumari <[e-mail]> wrote:


Hi there, authors (and WG),



Thank you for writing draft-ietf-ippm-encrypted-pdmv2 - "IPv6 Performance and Diagnostic Metrics Version 2 (PDMv2) Destination Option".


Unfortunately I think that that the document still needs some work before I'm ready to advance it to IETF LC / IESG Eval. 

I think that the largest issue is that the ordering of the sections doesn't make the document very easy to read —- the majority of Section 4 and Section 5 don't make much sense until and unless you have read Section 6. I think that much of my concern can be addressed by simply moving Section 6 before Section 3. 


Nits / comments: 
Section 1  - Introduction
1: "The current PDM is an IPv6 Destination Options header…"
Can you please expand PDM on first use? Yes, I know it is expanded in the title, but I think it would be good it include it in the narrative as well. 



2: "PDM metrics can help an attacker understand about the type of machine and its processing capabilities."
Which machine, and what about it? (e.g : something like "...can provide information to an attacker about the operational limits and network capabilities of the machine sending the metrics" or similar?)

3: I think that the following sentence can be dropped, or the two following sentences can be combined into one…

4: "... it can mislead the person showing there are …" 
I think you are missing a word after "person" - perhaps "by"? In addition, perhaps "the person" isn't great — much of this might be consumed by tooling, so perhaps just drop "the person".

Section 3 - Terminology
5: "pkX  (Public Key) and skX (Private Key).  Where X can be, any client or  any server."
This doesn't seem very clearly written - perhaps something like "In this document, the Public Key is represented as pkX and the Private Key as skX (where X can be any client or server)." or similar.

6: You are using the terminology "SharedSecret" and "SessionTemporaryKey" further down in the document, but haven't included it here. You claim to be using "Session Key", but it doesn't appear anywhere in the document other than in the "Terminology" section.


Section 4.1.  Client - Server Negotiation
This section says:
"Each Client and Server derive a "SessionTemporaryKey" by using HPKE Key Derivation Function (KDF), using the following inputs: [...]"
and then as the end of the section there is:
"The SessionTemporaryKey using a KDF with the following inputs:
   *  SrcIP, SrcPort, DstIP, DstPort, Protocol, SharedSecret, Epoch."
I'm assuming that the second one is just left over from earlier edits?


Misc:
I'm confused by:
"4.2.2.  Use Case 2: Server does not allow PDM or PDMv2

   If a client sends a packet with PDM and the network policy is to only
   allow encrypted or unencrypted PDMv2, then the PDM / PDMv2 header
   MUST be ignored and processing continue normally."
I'm really unclear what this is trying to say — there feels like a disconnect in the section header and the text? Even if that is the case, I think that the use of the term "PDM / PDMv2 header" here is confusing — would it be OK to just say something like "the PDM option"? 

This is made more confusing with the next paragraph:
" The server SHOULD log such occurrences but MUST apply rate limiting to any such logs.  The implementor should be aware that logging or returning of error messages can be used in a Denial of Service reflector attack.  An attacker might send many packets with PDM / PDMv2 and cause the receiver to experience resource exhaustion."
But the text above says that the "the network policy is to only  allow encrypted or unencrypted PDMv2…"


I found the text describing the "Global Pointer" in Sec 6.2 unclear — it took me quite a few readings to work out (and I'm still unclear!) that there are two Global Pointers, one for LL and one for Global addresses — I suggest rewording the "Unlike PDM fields, Global Pointer (GLOBALPTR) field in PDMv2 is defined for the SADDR type.  Following are the SADDR address types considered:" text.
Even saying something like "defined per SADDR type" would make this clearer (unless I'm completely wrong, I'm still unclear on the scope of this). 




I'm also somewhat confused by:
"Epoch
12-bit unsigned number.
Epoch field is used to indicate the valid SessionTemporaryKey.

Packet Sequence Number This Packet (PSNTP)
      16-bit unsigned number.
      This field is initialized at a random number and is incremented sequentially for each packet of the 5-tuple.
      This field is also used in the Encrypted PDMv2 as the encryption nonce.  The nonce MUST NOT be reused in different sessions."

and:
"The Epoch MUST be incremented when the Packet Sequence Number (PSN)
   This Packet (PSNTP) is rolled over.  It MAY be incremented earlier, depending on the implementation and the security considerations.

   The sender MUST NOT create two packets with identical PSNTP and Epoch."

I suspect that I'm missing something very fundamental here, but even if the Epoch is not 'incremented earlier', after 2^(12+16) packets in a flow, this will wrap and you will generate packets with identical PSNTP and Epoch. This is only 286 million packets — what am I missing?
 
I am also unclear what exactly "rolled over" means in:
"The Epoch MUST be incremented when the Packet Sequence Number (PSN) This Packet (PSNTP) is rolled over." I'm **assuming** the obvious "when the 16 bit counter exceeds 2^16 and overflows", but PSNTP is initialized at random — so, if it happens to be initialized at 65535 should it roll over after 1 packet, or do you mean when it next reaches 65535? I assume the former, but it's unclear…

Please let me know, LOUDLY, once you have had a chance to address these and I'll progress the document..

W

_______________________________________________
ippm mailing list -- [e-mail]
To unsubscribe send an email to [e-mail]