Subject: Re: [IPsec] WGLC of draft-ietf-ipsecme-labeled-ipsec

From: Paul Wouters <[email-masked]>
Date: Mon, 25 Oct 2021 17:10:06 -0400 (EDT)

On Wed, 25 Aug 2021, Valery Smyslov wrote:

> sorry for being late, the GGLC is already over, but I was really busy to review the draft before.

And sorry for my late response..

> 1. Section 1.2.
> The last para is erroneous here, since the immediately following the first para of Section 1.3
> states exactly the same with more details. Either remove it or rephrase and move to 1.3 (e.g.
> to keep provided example).

It is actually different. Section 1.2 only clarifies content of RFC
7296. Sectin 1.3 describes to update to RFC 7296.

> 2. Section 2.2.
>   If the TS_SECLABEL is present in a TSi/TSr, at least one Traffic
>   Selector of type TS_IPV4_ADDR_RANGE or TS_IPV6_ADDR_RANGE MUST also
>   be present in that TSi/TSr.
>
> I think this requirement was already spelled out in more generic form in 1.3.
> Why it is repeated here?

Defense in depth against sloppy implementers? I removed this one :P

> 3. Section 2.2
>   A zero length Security Label MUST NOT be used.  If a received TS
>   payload contains a TS_TYPE of TS_SECLABEL with a zero length Security
>   Label, that specific Traffic Selector MUST be ignored.  If no other
>   Traffic Selector of TS_TYPE TS_SECLABEL can be selected, a
>   TS_UNACCEPTABLE Error Notify message MUST be returned.  A zero length
>   Security Label MUST NOT be interpreted as a wildcard security label.
>
> I wonder why zero-length TS_SECLABEL cannot be used to represent "no security label"?
> This would significantly simplify negotiation when using security labels are optional
> for initiator. Currently it is proposed that initiator performs two attempts
> to establish Child SA in this case - with and without TS_SECLABEL.
> If zero-length TS_SECLABEL meant "no security label", then a single CREATE_CHILD_SA
> would be sufficient. It would also simplify the IKE state machine for this case.
>
> Are there any reasons I'm not aware of?

If new implementations start using this, it would be incompatible with
old implementations that would reject any TS set with unknown TS_TYPE,
and would also lead to two attempts. We also wanted to avoid ambiguity
with NULL or the empty string. And wanted a clear signal for wanting
a TS_SECLABEL to not signify "no sec label". I am nervous that
implementations would fail to insist on a label because it got an empty
label. So again, defense in depth against sloppy implementers.

Note that in all cases I am aware of, there is no use case of "optional
security labels" and both ends always need to configure the exact same
label. So there is not really a cause of "retrying without security
label". I know this might look useful upgrading existing deployments
from no label to sec label, but in practise, I have not seen this.
People start with insisting proper labels.

> 4. Section 2.2
>   If multiple Security Labels are allowed for a given IP protocol,
>   start and end address/port match, multiple TS_SECLABEL can be
>   included in a TS payload.
>
> This sentence is unclear for me. If the initiator includes multiple TS_SECLABEL,
> does it mean that the responder must select exactly one of them or not?
> If yes, then can you please clarify, that the presence of  multiple TS_SECLABEL
> means that selection any of them is acceptable for the initiator.

Changed to:

       If multiple Security Labels are allowed for a given IP protocol,
       start and end address/port match, the initiator includes all of the
       acceptable TS_SECLABEL's and the responder MUST select one of them.

> 5. Section 2.2:
> What is "TS_set", which is mentioned twice in the last para?
> This acronym was never used before in the dicument.

Changed. Old text:

    A responder that selected a TS with TS_SECLABEL MUST use the Security
    Label for all selector operations on the resulting IPsec SA.  It MUST
    NOT select a TS_set with a TS_SECLABEL without using the specified
    Security Label, even if it deems the Security Label optional, as the
    initiator TS_set with TS_SECLABEL means the initiator mandates using
    that Security Label.

New text:

    A responder that selected a TS with TS_SECLABEL MUST use the Security
    Label for all selector operations on the resulting TS. It MUST
    NOT select a TS_SECLABEL without using the specified Security Label,
    even if it deems the Security Label optional, as the initiator has
    indicated (and expects) that Security Label will be set for all
    traffic matching the negotiated TS.

(I avoided talking about IPsec SA or Child SA, as one of those _may_
include multiple different TS'es)

(If you find the term TS confusing, see Section 1.2 where I blame the WG
  for an unclear definition in RFC 7296 :)

> 6. Section 3.
>   Each TS payload (TSi and TSr) MUST contain at least one TS_TYPE of
>   TS_IPV4_ADDR_RANGE or TS_IPV6_ADDR_RANGE.
>
> This is repeated for the third time in the document. Can you please keep the only
> instance of this requirement?

Okay, I removed the middle one as it really talked about TS_LABEL
properties only. I left the "update RFC 7296" one and this one which is
actually about the negotiation.

> 7. Section 3.
>   A responder MUST create its TS response by selecting one of each
>   TS_TYPE present in the offered TS by the initiator.  If it cannot
>   select one of each TS_TYPE, it MUST return a TS_UNACCEPTABLE Error
>   Notify payload.

Changed to:

       A responder MUST create each TS response by creating one of more
       (narrowed or not) TS_IPV4_ADDR_RANGE or TS_IPV6_ADDR_RANGE entries,
       plus one of each further TS_TYPE present in the offered TS by the
       initiator. If this is not possible, it MUST return a TS_UNACCEPTABLE
       Error Notify payload.

> and later
>
>   Some TS_TYPE's support narrowing, where the responder is allowed to
>   select a subset of the original TS.  Narrowing MUST NOT result in an
>   empty selector for that TS_TYPE.

I removed this paragraph.

> This is wrong with regard to TS_IPV4_ADDR_RANGE or TS_IPV6_ADDR_RANGE.
> The responder may return any subset of TSs with TS_IPV4_ADDR_RANGE or TS_IPV6_ADDR_RANGE.
> E.g, if the initiator included 5 TSs with TS_IPV4_ADDR_RANGE and 3 with TS_IPV6_ADDR_RANGE,
> then the responder is free to return (just as example) only 2 TS_IPV4_ADDR_RANGE and no TS_IPV6_ADDR_RANGE,
> and all of them may have different content (due to narrowing).

Agreed.

> 8. Section 3.2:
>   It would be unlikely that the traffic for TSi and TSr would have a
>   different Security Label, but this specification does allow this to
>   be specified.
>
> Can you please provide some examples of possible semantics of
> different TS_SECLABELs in TSi and TSr? Sending different
> security labels in different directions? Just for curiosity.

I cannot, but I wanted to ensure not to accidentally forbid it. What
we do see with our implementation though, that we end up with two
IPsec Sa's where each is only used in one direction. For example,
image you have an SElinux context for nfs-server and nfs-client. What
happens is that the NFS client triggers one label on the outgoing
connection, triggers ACQUIRE, sets up an IPsec SA with a label
"nfs-client". The first packet hits the NFS server, which to reply
hits a different SElinux context of "nfs-server". So it will trigger
an ACQUIRE and setup a second IPsec SA with a label "nfs-server". Both
IPsec SA's will only be used in one direction. I did not add any
of this into the document, as it is very SElinux specific, where as
the draft is agnostic on the meaning of SEC_LABEL and allows for
other mechanisms that might not have this complexity.

> 9. Section 3.2:
>   Rekey of an
>   IPsec SA MUST only use identical Traffic Selectors, which means the
>   same TS Type and selectors MUST be used.  This guarantees that a
>   Security Label once negotiated, remains part of the IPsec SA after a
>   rekey.
>
> I think it is a too strong requirement. Traditionally in IKEv2 the rekey operation
> is equivalent to creating a new Child SA with full negotiation of algorithms and
> TSs.

I used the RFC 7296 language in the new text:

      The mechanism of narrowing of Traffic Selectors with
      TS_IPV4_ADDR_RANGE and TS_IPV6_ADDR_RANGE does not apply to
      TS_SECLABEL as the Security Label itself is not interpreted and
      cannot be narrowed. It MUST be matched exactly. Since a rekey
      MUST NOT narrow down the Traffic Selectors narrower than the scope
      currently in use, the only valid choice of TS_SECLABEL for a rekey
      is the identical TS_SECLABEL that is in use by the Child SA being
      rekeyed. If the TS_LABEL is missing from the TS during the rekey
      negotiation, the negotiation MUST fail with TS_UNACCEPTABLE.

diffs available at:

https://www.ietf.org/rfcdiff?url2=draft-ietf-ipsecme-labeled-ipsec-06.txt

Paul