Hi Daniel,
thank you for the explanation, please see inline.
Hi Valery,
Thanks for your review. We will promptly address your comment and update the
draft. Please let me respond to your questions/reviews below.
Yours,
Daniel
On 2025-01-16 08:11, Valery Smyslov wrote:
Hi,
I reviewed draft-ietf-ipsecme-ikev2-diet-esp-extension.
Summary: I don't think that the document is ready.
With the current text I have trouble reading it as implementer.
Issues:
1. Section 2.
Certain AfRG have already been
established during the SA negotiation process through IKEv2. This
extension facilitates the agreement on the remaining AfRG through
IKEv2.
>From my reading of this text, the negotiation of some AfRGs is defined
elsewhere. I don't think this is correct.
<mglt>
I beleive this assertion holds true, as certain parameters of the AfRG have
already been negotiated through IKEv2, such as TS or IPsec mode, for instance.
While these parameters are not explicitly categorized as AfRG, they serve as
inputs for defining the compression. Since we refer to these inputs as AfRG for
the purpose of defining compression, we consider these parameters as being
negotiated via IKEv2.
The AfRGs are referenced in draft-ietf-ipsecme-diet-esp, specifically in Table
1. Only six parameters require a specific negotiation, which is achieved by
this extension.
We will focus on refining and clarifying the language used.
</mglt>
I believe that all this should be explained in the draft. Moreover, I
think that there should be an explanation
of how _each_ of the remaining parameters is getting its value. For
example:
“The TV for ts_port_src_start is taken from the “Start Port” field of
the Traffic Selector of TSi payload
in the request message.”
This should be spelled out for each implicitly negotiated parameter,
currently implementers
have to guess this. I’d also like to see the FID, MO and CDA values for
each parameter.
Perhaps a table similar to Table 1 in diet-esp draft, but with more
columns, would help.
2. Section 3.
I think that it is better to replace the term "Proposal Payload" with just
"Proposal"
or "Proposal Substructure" to avoid confusion with IKEv2 payloads.
<mglt>
Thanks for the feed back. To prevent any misunderstanding regarding the
Proposal Substructure as outlined in RFC7296 Section 3.3.1, we opted for the
term "Proposal Payload." However, we acknowledge your concerns and are
receptive to alternative designations. Personally, and at the time I am
reading your comment, I have a preference for "Proposal," and "HCP Proposal"
seems also serve as a viable option.
We will give this careful consideration.
</mglt>
3. Section 3.
Nevertheless, it is anticipated that the responder will provide an
explanation for rejecting all HCP Proposals. If the reason pertains
to an AfRG with an unacceptable value, the responder SHOULD reply
with an HCP_UNSUPPORTED Notify Payload. This Notify Payload SHOULD
include one or more acceptable Proposal Payloads to guide the
initiator.
This is not how the parameters negotiation is used to be done in IKEv2.
Currently, the initiator sends list of all acceptable for it parameters
and the responder selects the subset that is acceptable for it.
With the approach in the draft the responder would send back the value
that is not presented by the initiator. Either this value is unacceptable
for initiator (and thus no reason to send it back except for logging)
or the initiator didn't send *all* acceptable for it values.
This is a major change to the way parameters are generally
negotiated in IKEv2 and thus it should be justified with a very good reason.
I currently fail to see such, but perhaps I'm missing something.
<mglt>
Thank you for your comment; we will likely need to provide further
clarification on this matter. The HCP_UNSUPPORT explicitly states that none of
the proposals can be accepted, and there is no requirement for additional
information to be supplied. I concur that the message may not be essential,
which is why we categorized it as a SHOULD.
The rationale behind this message is our anticipation that certain
implementations will exclusively support Diet-ESP with a specific set of
parameters, and not even support the uncompressed ESP at all. Consequently, we
want such implementations to have the capability to indicate a negotiation
failure due to their support for Diet-ESP with very particular parameters. If
this indication is provided, it is expected to be generic in nature. We only
expect the initiator to log this information. We appreciate your concern and
will clarify that we are not altering the IKEv2 negotiation process or the
reasons for introducing this Notification Payload.
</mglt>
Why do you add new notification for this? We have NO_PROPOSAL_CHOSEN,
which has the following meaning:
NO_PROPOSAL_CHOSEN 14
None of the proposed crypto suites was acceptable. This can be
sent in any case where the offered proposals (including but not
limited to SA payload values, USE_TRANSPORT_MODE notify,
IPCOMP_SUPPORTED notify) are not acceptable for the responder.
This can also be used as "generic" Child SA error when Child SA
cannot be created for some other reason. See also Section 2.7.
Why this is not acceptable for your case? Except that it does not
provide logging.
But I’m not sure it justifies adding a new notification for the very
specific case.
4. Section 3.
HCP_UNSUPPORTED is a very bad name. My first perception was that
it means that HCP is not supported at all by the responder, in which case
the responder would not have known anything about this notify too. After
re-reading
I realized, that it means that no provided parameters are acceptable,
thus the better name would be HCP_NO_PROPOSAL_CHOSEN.
<mglt>I concur that your proposal is a better wording, particularly since a
single node transmitting that message indicates its support for HCP. Thank you
for presenting the proposal.
</mglt>
In addition, it is defined as a status type notify, but it should be
an error type notify (not a fatal one), since it indicates an error
in creating Child SA.
And I see no need for it at all, if HCP negotiation is performed
as it is done in IKEv2 for all other parameters.
<mglt>I concur that it is preferable to categorize this as an error. I believe
I clarified the rationale behind the creation of that message in 3.</mglt>
5. Section 3.
In cases where the AfRG was not explicitly stated, the
responder will provide the AfRG unless it defaults to a standard
value.
I wonder what default values are and where they are defined.
Table 1 in draft-ietf-ipsecme-diet-esp lists only possible values
for each parameter and doesn't mark any of them as default.
<mglt>The division we have established is that draft-ietf-ipsecme-diet-esp
outlines the methodology for compression as derived from AfRG. It is presumed
that all AfRG have been clearly defined. The present draft is tasked with
detailing the process by which the AfRG are reached, and a default value has
been introduced to facilitate negotiations. This rationale underpins our
decision to elaborate on this matter within this document, specifically in
Section 7. </mglt>
6. Section 3.
There is no discussion whether HCP parameters should
match other values negotiated by IKEv2. For example,
is it allowed that values in ts_* parameters don't
match TSi/TSr content? What peers need should do in this case?
Am I missing something?
<mglt>I believe this is a valid observation. While I anticipate
HCP_UNSUPPORTED, it has not been explicitly stated. We will ensure this is
clarified. Thank you for your feedback.
It is important to note that in draft-ietf-ipsecme-diet-esp, we have made
certain assumptions regarding the TS. However, I concur that this aspect also
requires explicit handling by IKEv2.
"""
The compression of the Inner IP Packet is based on the attributes
that are derived from the negotiated Traffic Selectors TSi/TSr, as
described in [RFC7296], Section 3.13. The Traffic Selectors may
result in a quite complex expression, and this specification
restricts that complexity. In particular, Diet-ESP restricts the
Traffic Selector to a single type of IP address (i.e., IPv4 or IPv6),
a single protocol (such as UDP, TCP, or not relevant), a single port
range, and multiple DSCP numbers. Such simplification corresponds to
the expression of an individual Traffic Selector Payload [RFC7296],
Section 3.13.1.
"""
</mglt>
7. Section 4.
In description of Proposal Payload:
EHCP Name (2 octets): The identifier of the EHCP Name. (see Table 2)
Typo: s/2 octets/1 octet
<mglt> We will correct this. Thanks.</mglt>
8. Section 5.
As far as I understand, the attribute range_afrg_proposal is used to
provide the range of supported compression parameters.
It is not clear how the identifiers of these parameters are
represented - e.g. how many octets an identifier occupies.
I presume it occupies two bytes, as IKEv2 attribute type, but this is not
spelled out.
Perhaps I'm badly missing, the text is not clear for me.
<mglt>
The range_afrg_proposal consists of two attributes, namely AfRG_min and
AfRG_max. Both AfRG_min and AfRG_max are classified as attributes, and
consequently, the overall length encompasses the lengths of both AfRG_min and
AfRG_max.
The potential misunderstanding may arise from our reference to AfRG_min and
AfRG_max as values. While these represent the values for the range, they are
indeed attributes. It would be beneficial to enhance our description by
explicitly stating that AfRG_min and AfRG_max are attributes. Additionally, we
should outline the procedure for managing the range, specifically ensuring that
both attributes are of the same type. We will make the necessary clarifications
in this section. Thank you for bringing this matter to our attention.
</mglt>
I think this all should be clarified. I’d also like to see few
examples.
9. Section 7.
I wonder why the attributes for the listed AfRGs are encoded in TLV format,
and not in TV format? They all contain one-octet value, so they all
would fit in TV format. In my humble opinion, the goal of this specification
is to make IPsec packets smaller, and at the same time less-than-optimal
encoding is used for these attributes.
<mglt>
It is accurate that the attributes established for the Diet-ESP EHCP possess a
fixed length. Furthermore, in our specific instance, these attributes share the
same length. Nevertheless, this should not be anticipated for every profile,
(determined by the HCP Name attribute). We needed to strike a balance between
flexibility and bandwidth optimization, and thus far, we have opted to
prioritize flexibility. Additionally, it is worth noting that this format is
commonly used to describe attributes in IKEv2. It may be beneficial to include
a note regarding the rationale behind this decision.
</mglt>
I suspect you missed my point. Let me be more specific.
Let’s take for example the alignment attribute. Table 3 states that it
has associated data, which meant that it is encoded on the wire as TLV
attribute format.
The data, that this attribute contains, is always 1 octet in length
(since it is a codepoint defined
in Table 8.5.4, with a 255 as a maximum value). This doesn’t depend on
any future profile, this is fixed in your profile.
Thus, your current encoding on the wire takes 5 octets. But since the
attribute data is always 1 octet in length, you might use TV encoding
for this attribute (set Has Associated Data to NO), which would result
in 4-octet encoding on the wire.
I agree that this is not a big saving, but it seems to me that the
IOT-targeted document should
use any possibility to save, no?
10. Section 7.
Text regarding alignment:
* Default Value: the default value is set to "32 bit", which
corresponds to the standard IPv6 bit alignment
This contradicts to what RFC 4303 specifies (Section 2.3):
Note that the beginning of the next layer protocol header MUST be
aligned relative to the beginning of the ESP header as follows. For
IPv4, this alignment is a multiple of 4 bytes. For IPv6, the
alignment is a multiple of 8 bytes.
Perhaps different alignments are meant, but with no clarification in the draft
this is unclear for me.
<mglt> This is a valid observation. We will provide clarification on this
matter. While we may adopt IPv6 alignment as the default, we will ensure that
the text is made clear. Thanks for providing the note.</mglt>
11. Section 7.
s/Security Policy Index/Security Parameter Index
<mglt>ok.Thanks.</mglt>
12. Section 7.
This section provide registration information only for 6 AfRG out of 23 listed
in Table 1
of draft-ietf-ipsecme-diet-esp, that are needed to implement EHC. I wonder
where can I get information about the others.
<mglt>Table 1 lists all 23 AfRGs. The reference column specifies the sources
where these parameters are defined. For 18 of these, the AfRG is a parameter
outlined in either RFC7296, RFC4301, or a draft concerning DSCP values. All
these AfRGs are established through the creation of a CHILD SA via IKEv2. When
explicitly negotiated by IKEv2 with a specific payload, we referred to RFC7296
or RFC4301 was utilized. Six of the parameters are defined in
draft-ietf-ipsecme-diet-esp, which details the negotiation process for these
six parameters.</mglt>
Please see my comment above. There must be more details of how all
other parameters are getting their values.
It is not sufficient to reference RFC 7296 or RFC 4301 – they both are
silent about AfRGs and implementers
have to guess from what fields AfRGs should take their values from.
I also think that there must be Operational Considerations section,
which should help implementers
to correctly select some values. For example, when selecting
esp_sn_lsb, implementations
should consider the maximum number of packets that is expected to be
send over a Child SA
being negotiated before its rekeyng (thus, this is an Child SA-wide
parameter), while
for selecting esp_spi_lsb, implementation should consider the total
number of Child SAs,
that are expected to be active at any given time, (thus this is a
system-wide parameter).
I suspect that all these nuances currently have to be guessed by
implementers with no help
from the authors L
Regards,
Valery.
13. Section 8.3.
I wonder why the registry is called "IKEv2 Header Compression", while
the profile is about ESP header compression.
<mglt>I am not oppose to changing the terminology; however, I am willing to
clarify the rationale behind our chosen designation. We are quite receptive to
adopting an alternative designation if necessary.
Section 8.3 outlines a registry that pertains to the profile for which
parameters (AfRGs) are negotiated. This registry is exclusively utilized by
IKEv2, which is why IKEv2 is included in the registry's name. It is important
to note that this does not imply that the compression concerns IKEv2, and we
could opt to remove IKEv2 from the title, thereby referring to the registry as
the "Header Compression Profile."<mglt>
14. Section 8.2.
range_afrg is undefined. Perhaps this is a typo, the body of the draft
mentions range_afrg_proposal.
<mglt>Thanks. This is a typo yes.</mglt>
15. Section 8.4.
The same question as in issue 12 - what about other AfRG needed for ECH?
Only 6 out of 23 have codepoints.
<mglt>I tried to answer in question 1 and 12.</mglt>
Regards,
Valery.
This will start two week WGLC for the
draft-ietf-ipsecme-ikev2-diet-esp-extension [1]. This last call will end at
2025-01-23.
If you have any comments about the draft send them to the WG list.
[1]
https://datatracker.ietf.org/doc/draft-ietf-ipsecme-ikev2-diet-esp-extension/
--
[email protected] <mailto:[email protected]>
_______________________________________________
IPsec mailing list -- [email protected] <mailto:[email protected]>
To unsubscribe send an email to [email protected]
<mailto:[email protected]>
_______________________________________________
IPsec mailing list -- [email protected] <mailto:[email protected]>
To unsubscribe send an email to [email protected]
<mailto:[email protected]>
_______________________________________________
IPsec mailing list -- [email protected]
To unsubscribe send an email to [email protected]