Hi Valery,
I apologize for the delay I missed this email. Thanks you very much for
the deep review. We have updated the draft accordingly. We do have one
remaining question regarding the 64 bit (or 32 bit) alignment for the
IPv6 ESP extension header (and IPv4 equivalent). I have the impression
it does not really apply to ESP, in which case we could omit that AfRG
completely. Please find below some response comments.
Yours,
Daniel
On 2025-01-16 08:11, Valery Smyslov wrote:
Hi,
I reviewed draft-ietf-ipsecme-ikev2-diet-esp.
Summary: I don't think that the document is ready.
There are many issues (or various severity) that need to be fixed.
Issues.
1. Section 4.3.
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.
This text is problematic. Section 3.13.1 describes Traffic Selector
(substructure)
and not a Traffic Selector payload (which is defined in Section 3.13).
In addition, traffic selectors, as defined in RFC 7296, cannot express DSCP,
thus I don't know how to interpret the above requirement.
I believe the text tries to say that there MUST be only
one Traffic Selector (Section 3.13.1) in both TSi and TSr payloads
(described in Section 3.13). The TS Type MUST be
either TS_IPV4_ADDR_RANGE or TS_IPV6_ADDR_RANGE.
(I don't know what to do with DSCP. The protocol and the TS type
are always single-valued in a Traffic Selector substructure).
Perhaps my interpretation is not correct, but the current text needs to be
revised anyway.
<mglt>
You are correct that "Traffic Selector Payload" should be substituted
with "Traffic Selector Substructure" or possibly "Traffic Selectors,"
which aligns more closely with the terminology used in RFC 7296.
Additionally, it is accurate that "and multiple DSCP numbers" should be
omitted. The inclusion of DSCP likely stems from initial considerations
during the drafting of what ultimately became
draft-mglt-ipsecme-dscp-np. We concur that these elements do not belong
to the Traffic Selector.
We originally aimed for the description of the compression to be
separate from the negotiation process of AfRGs. However, in light of
your additional suggestions, we are now incorporating more detailed
information regarding the negotiation of each AfRG through IKE. Our
objective is to potentially limit the description of the HCP IKE
extension to only those AfRGs that are actually negotiated by the HCP
IKE extension. We have thoroughly reviewed the text and propose the
following updates to our local copy.
"""
The Traffic Selectors may result in a quite complex expression, and this
specification restricts that complexity.
This specification restricts the resulting TSi/TSr to a single type of
IP address (IPv4 or IPv6), a single protocol (e.g., UDP, TCP, or ANY), a
single port range for source and destination. This specification
presumes that the Traffic Selectors can be articulated as a result of
CREATE_CHILD_SA with only one Traffic Selector {{!RFC7296, Section
3.13.1}} in both TSi and TSr payloads (as described in {{!RFC7296,
Section 3.13}}). The TS Type MUST be either TS_IPV4_ADDR_RANGE or
TS_IPV6_ADDR_RANGE.
"""<mglt>
2. Section 4.3, Table 1.
For ts_ip_version parameter possible values listed as "IPv4-only IPv6-only".
Should they be "IPv4-only" "IPv6-only" instead?
<mglt>You are correct, we replaced "IPv4-only IPv6-only" by "IPv4-only"
"IPv6-only" for ts_ip_version"</mglt>
Also s/IP4/IPv4 (two cases)
<mglt>You are correct, we replaced the two occurences of "IP4" by
"IPv4". </mglt>
Why "64 bit" parameter denotes its own empty row? Should it be along with
possible alignment values? The same question for ANY, ... and
"generated", "zero" empty rows. Perhaps they should be merged with the above
cells.
.
<mglt>It seems a issue that happen during the markdown conversion. We
will look into that. </mglt>
3. Section 4.3
This specification limits the expression of the Traffic Selector to
be of the form (IP source range, IP destination range, Port source
range, Port destination range, Protocol ID list, DSCP list). This
limits the original flexibility of the expression of TS, but provides
sufficient flexibility.
This is not consistent with the text above, since the text above
requires that there be only one protocol and not the list.
And again, DSCP values cannot be expressed in IKEv2 Traffic Selectors.
<mglt>You are correct. There is no more Protocol ID list or DSCP in the
TS and the text has been updated.</mglt>
4. Section 4.3.
It is not very clear for me what iipc_profile is for. In other words,
if the value is "diet-esp", then what it means? Does it mean
that only those parameters, defined in Table 1 for IIPC must be used?
Should the application verify this? Should all these parameters be supported
or it is allowed to support only some of them? I'd like to see more
discussion of this in the draft. Perhaps external reference to
some SCHC document is needed.
<mglt>
The iipc_profile specifies the method of compressing the inner packet,
which can either be "uncompressed" or "diet-esp". This document outlines
the Diet-ESP compression protocol, which provides two options for
compressing the inner IP packet: "diet-esp" and "uncompress." These
iipc_profiles can be regarded as configuration parameters. An
implementation that adheres to this protocol is anticipated to support
both "diet-esp" and "uncompress." The details of the compression process
are elaborated in section 5.1, which explains how the values are managed
by Diet-ESP. To enhance clarity we referred to iipc_profiles- that can
be seen as sub profiles - with "iipc_diet-esp" and "iipc_uncompress".
</mglt>
5. Section 4.3.
Please, expand the text for possible values for dscp_cda and ecn_cda
instead of sending readers to flow_label_cda by "(See flow_label_cda)".
The list of values is different, so it is better to repeat their meaning
for each parameter.
<mglt>We expanded the text to explain the values. We also expanded the
text of the variable themselves.</mglt>
6. Section 4.3.
The explanations for dscp_cda and ecn_cda both start with the text
"indicates how the DSCP values of the inner IP packet are generated.".
Unless I'm badly missing something, this should be s/generated/compressed /
decompressed
<mglt>You are correct, your wording seems more appropriated. We changed
it as suggested.</mglt>
7. Section 4.3
ts_ip_dst_start, ts_ip_dst_end, ts_port_dst_start, ts_port_dst_end are not
explained in the text.
<mglt>Correct. We added them. These have probably been lost as we used
to have ts_ip_src/dst_start. </mglt>
8. Section 4.3
Please, expand LSB and MSB on first use (I presume Least /Most Significant
Bits).
<mglt>We expanded them.</mglt>
9. Section 4.3.
In the text related to ts_proto - why RFC 5996 is referenced? It is obsoleted
by RFC 7296.
<mglt> The reason for keeping 5996 is that ANY is not defined in 7296.
This is something we noticed and had updated to The representation of
"ANY" is given in {{!RFC4301, Section 4.4.4.2}}. on our local copy. </mglt>
10. Section 4.3.
The text related to dscp_list parameter informally references the expired draft
draft-mglt-ipsecme-dscp-np.
In my reading of this text, it is impossible to implement this draft without
reading
draft-mglt-ipsecme-dscp-np (at least this is my impression). Thus, reference
to draft-mglt-ipsecme-dscp-np should be normative, whichm in my understanding,
makes impossible
publication of this document unless draft-mglt-ipsecme-dscp-np becomes RFC.
Alternatively, please rewrite the text so, that draft-mglt-ipsecme-dscp-np is
not needed
for implementing this document.
<mglt>
You are correct that the relationship to draft-mglt-ipsecme-dscp-np
necessitates further discussion, as it has not yet been adopted in the
charter. The draft-mglt-ipsecme-dscp-np outlines the process by which
peers reach an agreement on DSCP code points through an IKEv2 Notify
Payload.
We need to see if there is a way to move draft-mglt-ipsecme-dscp-np to a
standard track otherwise, we will have to include the negotiation in
draft-ietf-ipsecme-diet-esp-ikev2-extension. Our preference would be to
leverage on draft-mglt-ipsecme-dscp-np, as we believe that such DSCP
negotiation could be beneficial for various other applications. However
we remain open to suggestions. </mglt>
11. Section 4.3.
The text related to alignment parameter is very problematic. I cannot figure
out what it is all about.
For example, "indicates the byte alignement supported by the OS for the ESP extension."
What is "supported by OS"?
What is "ESP extension"? What should be aligned?
Why 32-bit alignment is for IPv6? As far as I remember, the IPv6 header should
be 64-bit aligned,
32-bit alignment is used for IPv4. Am I missing something?
Since AES-CCM is mentioned, I presume that alignment in fact means not a
requirement
for the start of payload alignment, but instead the requirement for the total
size of the payload
to be encrypted to be multiple of the encryption transform block size. If this
is the case,
then this should be spelled out more accurately. In addition, the block size
can be different
from 128 bits (for example, for GOST "Magma" it is 64 bits, there may also be
defined
encryption transforms with 256-bit block size in future), so this should be
expressed more accurately,
referencing the requirements imposed by encryption transform.
By the way, possible values for alignment do not include "128 bits", so the text
related to AES-CCM "a 128 bit alignment is overwritten by the block size"
cannot be parsed by me.
BTW, a typo: s/alignement/alignment
<mglt>
We appreciate you bringing this issue to our attention. It appears there
has been a discrepancy in the numbers for the IP4 and IPv6 extension
header alignment. This alignment parameter is not related to the block
size of the encryption algorithm, but solely to the alignment of the ESP
extension header. As you mentioned, there is another alignment
associated with the block cipher block length, which we have linked to
the ESP encryption algorithm (esp_encr). Importantly, the ESP Padding
and Pad Length must be generated to meet both (1) the requirement that
the Clear Text ESP matches the block cipher size (block len(Payload
Data) + len(Padding) + 1 = 0 %block_size) and (2) the requirement that
the entire ESP extension meets the alignment specifications (compressed
ESP Header + len(IV) + x block_size = 0 % alignment). I am addressing
these discussions separately, and here I am focusing solely on the
alignment, as I believe this is potentially something we can remove. The
discussion regarding the block_size is covered in the esp_encr section
related to comment 13.
In light of RFC8200 and RFC4303, I am curious whether the 64-bit
alignment for IPv6 is applicable to the Encapsulating Security Payload
(ESP). Given that generic extension headers and options express their
lengths in 8-octet units, they must be aligned to 64 bits. This
alignment allows us to potentially bypass the current option or proceed
to the next option without the need to parse the extension header or option.
RFC8200 section 4 states:
"""
Each extension header is an integer multiple of 8 octets long, in
order to retain 8-octet alignment for subsequent headers. Multi-
octet fields within each extension header are aligned on their
natural boundaries, i.e., fields of width n octets are placed at an
integer multiple of n octets from the start of the header, for n = 1,
2, 4, or 8.
"""
Conversely, ESP is a mandatory component of IPv6 and is always
positioned as the last extension header. Consequently, IPv6 must be
capable of parsing ESP, and there are no subsequent headers to consider.
Furthermore, the length of the ESP extension is determined by the IPv6
Payload Length field, which is a 16-bit integer representing bytes,
rather than an 8-byte word. Based on RFC8200, I am uncertain whether
this restriction is applicable.
Upon further examination of RFC4303, the payload data section indicates:
"""
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.
"""
The definition of the next layer protocol header is somewhat ambiguous.
If it refers to the layer that follows the ESP payload, then there is
none, as ESP is the final extension header, and thus the alignment
condition does not apply. Alternatively, if the next layer protocol
pertains to the Payload Data indicated by esp.next_header, then the
condition seems to imply that the Security Parameter Index (SPI) and
Sequence Number (SN) and IV must be 8 bytes aligned. This situation
appears rather peculiar. Therefore, unless I am overlooking something,
it seems to me that we could entirely eliminate the constraint.
It is essential to assess the current status of IPv4. However,
concerning IPv6, I believe we can disregard any previous considerations
related to alignment.
</mglt>
12. Section 4.3.
Text related to tunnel_ip states that it "can be an IPv4 or IPv6 address".
However,
in the Table 1 only IPv6 address is allowed.
< mglt> You are correct, we added IPv4 as well in the Table 1.</mglt>
13. Section 4.3.
Text related to esp_encr - I fail to understand why knowing the encryption
algorithm is needed
for compression. Perhaps this should be clarified.
BTW, a typo: s/compresse/compress
<mglt>We clarified this in the section defining the AfRG as follws:
esp_encr:
: designates the ESP Encryption Algorithm - also designated as Transform
1 in {{!RFC7296, Section 3.3.2}}. The algorithm is needed to determine
whether the ESP Payload Data needs to be aligned to some predefined
block size and if the ESP Pad Length and Padding fields can be
compressed. For the purpose of compression it is RECOMMENDED to use
algorithms that already compressed their IV {{!RFC8750}}.
</mglt>
14. Section 4.3.
s/Security Policy Index/Security Parameter Index
<mglt>Thanks. we updated the text.</mglt>
15. Section 4.3.
Text for esp_sn_lsb: "and is defined by this specification". Is this
clarification needed for this parameter?
I believe many parameters listed above are also defined by this specification,
but it is only mentioned for esp_sn_lsb.
In addition - why possible values for this parameter are 0-64?
SN in the ESP header is always 32 bits in length, for 64-bit ESN the upper 32
bits are never transmitted.
<mglt>you are correct, we removed "and is defined by this specification"
and possible values are limited to 32 as we only compress bits on the
wire. The virtual ones do not take too much bandwidth.;-)</mglt>
16. Section 5.1.1
If you add some table similar to Table 1, with FL, MO, CDA values for each
parameter,
it will make reading easier. Alternatively, place description of compressing
each field to its own paragraph.
The current text is difficult to read.
<mglt>The challenge we face is that the FL and MO CDA outline the
compression methods for fields within the inner IP packet. These
compression guidelines are contingent upon the different combinations of
the AfRG parameters detailed in Table 1. The SCHC parameters are more
pertinent when derived from IP fields and specific AfRGs, resulting in a
considerable number of potential combinations. Annex A.1 appears to
offer an example of a rule set that can be formulated for a particular
scenario. We will consider this, although I anticipate that achieving a
more lucid representation than a textual description may prove
difficult.</mglt>
17. Section 5.1.3.
In the text "If the IP Header is an IPv6 Header" perhaps "tunnel" is missing before
"IP"?
<mglt>You are correct. Thanks!.<mglt>
18. Section 5.1.3.
Please expand "IHL" (Internet Header Length).
<mglt> We expanded it.</mglt>
19. Section 5.3.
SCHC Compression efficiently compresses the Next Header field,
reducing overhead and aligning the packet to byte boundaries using
SCHC Padding. After this, the SCHC Pad Length field is added.
It is not clear for me what is the sequence of applying various actions.
>From my understanding the SCHC Padding is applied before CTEC compression,
but reading this text it seems that it is applied after it too. Is it applied
twice?
<mglt> No. It is implemented subsequent to the compression of the inner
packet. The rationale behind this is to ensure that the Data Payload is
represented in bytes to be encrypted. We significantly rewrote the
section 5.2 that explains the SCHC padding and it role over the ESP
Payload Data as well as Section 5.3 which details the clear text ESP
.<mglt>
20. Section 5.3.
The ESP Padding and Pad Length fields are reduced by the SCHC
compression rule.
I fail to understand how this can be done. The padding is included
into the ICV computation, I don't know how you are going to elide these bytes.
<mglt> We remove them before the encryption when encryption makes it
possible to remove them - that is we do not need any alignment.</mglt>
21. Section 5.3.
Isn't IPv6 require 64-bit alignment and not 32?
<mglt>yes we need to clarify this over the two drafts. This is related
to the discussion of comment 11. I currently believe it does not impact
our design. </mglt>
22. Section 5.4.
I fail to understand why there is a need to send SN uncompressed
in case of implicit IV. In any case, you *must* be able to decompress
SN to an original value *before* decrypting the packet. If there is a chance
SN is decompressed incorrectly, then the packet will be dropped anyway
(due to ICV verification failure). As a consequence - the esp_encr
parameter is not needed.
<mglt>This is correct. We need to clarify this in the text. We actually
do not consider the case SN are omitted. </mglt
23. Section 5.4.
The IPv6 Next Header field or the IPv4 Protocol field that contains
the "ESP" value is changed to "SCHC".
What is the value for "SCHC"? I failed to find it in IANA registry
https://www.iana.org/assignments/protocol-numbers/protocol-numbers.xhtml
<mglt>The current document describing the SCHC protocol number is the
one below. As the document is also requesting port number which seems a
bit more controversial.
https://datatracker.ietf.org/doc/draft-ietf-schc-protocol-numbers/
I am currently thinking making it only optional as it is only could be
useful if we want to differentiate the traffic of the Compressed ESP
from the one non-Compressed ESP.
</mglt>
24. Section 7.
Where the mentioned IANA registries are situated?
I failed to find any "IIPC Profile" registry on IANA website.
<mglt>In Section 7. We request the creation of that registry. </mglt>
Regards,
Valery.
This will start two week WGLC for the draft-ietf-ipsecme-diet-esp [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-diet-esp/
--
kivi...@iki.fi
_______________________________________________
IPsec mailing list --ipsec@ietf.org
To unsubscribe send an email toipsec-le...@ietf.org
_______________________________________________
IPsec mailing list --ipsec@ietf.org
To unsubscribe send an email toipsec-le...@ietf.org
_______________________________________________
IPsec mailing list -- ipsec@ietf.org
To unsubscribe send an email to ipsec-le...@ietf.org
