Hi Antony,
thank you for updating the draft. More comments inline.
> Hi Valery,
> Thank you for the detailed review. Speficic responses are inline.
>
> I am trying to submit -04 to the datatracker. The revision also incorporates
> the use case from draft-ietf-ipsecme-ikev2-reliable-transport. I am on
> vaction, hopefully get around datatracker soon.
>
> One terminology question: the draft currently uses "ESP packet" and "ESP
> message" interchangeably in older version. Which is more appropriate in
> this context?
>
> IKEv2 is clearly message-based. What is ESP? and also with IP-TFS in mind —
> aggregation, fragmentation, and padding — I am not sure "packet" is always
> the right abstraction. Is there an established convention or WG preference
> here?
My personal preference is "ESP packet" and "IKE message".
I don't think AGGFRAG influences this a lot - the content of ESP can vary
anyway.
But perhaps I don't feel some nuances here (as a non-native speaker).
> > 2. Section 1
> > "The IPsec peer may announce its capability to support Encrypted ESP
> > Ping using an IKEv2 Notification Status
> Type."
> > I wonder why "may"? If peers do not negotiate support for this feature,
> > then how the result of the encrypted
> ping
> > can be reliably interpreted in case no response is received?
>
> Redesigned to use two explicit modes of operation, each requiring its own
> negotiation. In Mode 1, peers that have negotiated USE_AGGFRAG [RFC9347] use
> the existing Congestion Control payload. In Mode 2, peers negotiate
> ENCRYPTED_PING_SUPPORTED via IKEv2 and use the new payload format defined in
> this document. In both modes a negotiation is required, so once negotiated
> lack of response can be reliably interpreted as path failure.
> Section 1 has been rewritten to describe both modes explicitly.
So, if peers negotiate Mode 2 (ENCRYPTED_PING_SUPPORTED), then the full support
of AGGFRAG
is not required? This is a good idea, I support it.
That said, the draft should be more clear on this. Currently readers have to
guess this.
Also "Mode 1" and "Mode 2" appear in the text with no prior explanation what
these "modes" are.
> > 19. Section 4.3
> > "However,the responder MAY respond to the peer
> > using its default Security Parameter Index (SPI)."
> > What is the "default SPI"?
>
> Fixed. Changed to: "However, the responder MAY respond using the SPI of the
> SA on
> which the request was received."
It is impossible since ESP SAs are unidirectional. The responder has to find
the corresponding
outgoing SA (since IKE always creates ESP SAs in pairs) that will most probably
have a different SPI than incoming SA.
More comments.
After re-reading the draft, I don't see a need for the R bit. It only
complicates
processing of ESP ping packets. For example, what if R bit is set in the
ESP-ECHO-RESPONSE packet?
Is this a valid packet or not? How the receiver should interpret this?
I propose to make the following changes:
1. Remove the R bit.
2. Define two ESP ping formats - one for ESP-ECHO-REQUEST and the other for
ESP-ECHO-RESPONSE,
with the only difference of a presence of the Return path SPI field: it is
present in request
and is missing in response. Thus, the ping initiator always provides the
return path SPI.
If it does not care, fill it with zeroes.
This would simplify processing of the packets by eliminating invalid
combinations of the values.
Opinions?
Then, what is a purpose of having both Identifier and Sequence number?
As far as I understand, they are just copied from ICMP Echo and Echo Reply
messages.
However, in ICMP they may have more value, since ping is host-wide and
scoping makes sense (you may have several instances of ping applications
running).
ESP pings will be implemented by an IPsec module, that will also handle ESP SA
processing.
Thus, I see no need for the both of these fields - it is sufficient to have a
single field to match
responses with requests (e.g. Identifier). Am I missing something?
Then, do we need Data Length? The size of ESP packet is known, so what is its
purpose?
To mark a margin between the Encrypted ping and possible TFC padding?
But the Data field in the ESP ping is actually a filling and thus is no
different
from TFC padding (in my understanding). Am I missing something?
And finally - do we need a Reserved field? Just for alignment?
Some text should also be added about possible replays. If replay protection for
ESP is active, this must not be an issue. However, if a peer receives an echo
request
over an ESP SA with disabled replay protection, then it must rate limit sending
responses. For example, no more than one response per second.
There are still nits in the draft, like Figure formatting is still broken
or erroneous dot at the end of ENCRYPTED_PING_SUPPORTED in the IANA
considerations.
Regards,
Valery.
> Thanks again for the thorough review!
>
> Antony
>
> PS: the authors hopping to merge this with ietf-ipsecme-esp-ping.
_______________________________________________
IPsec mailing list -- [email protected]
To unsubscribe send an email to [email protected]
