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