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

[Valery Smyslov <[e-mail]>]

Hi Paul,

> 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.

I disagree. Only the first two paras of 1.2 are clarifications. 
The last para of 1.2 is not a clarification, it is a new requirement for what
Traffic Selector Payload must contain, so it's an update of 7296 text and for 
this reason should belong to 1.3. But 1.3 has already contained this requirement.

Compare:

Last para of 1.2:

   A Traffic Selector payload (TS) is a set of one or more Traffic
   Selectors of the same or different TS_TYPEs, but MUST include at
   least one TS_TYPE of TS_IPV4_ADDR_RANGE or TS_IPV6_ADDR_RANGE.

1.3:

   This document updates the text to mean
   that the TSi/TSr payloads MUST contain at least one Traffic Selector
   of type TS_IPV4_ADDR_RANGE or TS_IPV6_ADDR_RANGE...

> > 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

OK.

> > 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. 

I don't buy this argument - with old implementations two attempts are unavoidable.

> We also wanted to avoid ambiguity
> with NULL or the empty string. 

What ambiguity? There is no ambiguity in representing this in the protocol - 
it's a TS with a type of TS_SECLABEL and no data. If you mean possible ambiguity
in representation this label inside the code, then I think that it's
out of scope of the protocol.

> 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.

My very deep believe is that you cannot stop some implementers from 
doing thing wrong (some are very creative in this regard) :-/
And I think that adding complexity to the protocol is a penalty
for those implementers, who strictly follow the spec.
So I can't buy this argument too, sorry.

> 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.

I wonder how they deploy this with more-or-less big system.
Simultaneously upgrade thousands of hosts with no service interruption?

> > 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.

Good, thank you.

> > 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'm a bit confused with the part of \new text:

"...all selector operations on the resulting TS."
What do you mean? Did you mean
"on the resulting Traffic Selector"?
Or am I missing something?
 
> (I avoided talking about IPsec SA or Child SA, as one of those _may_
> include multiple different TS'es)

So what? I think that the essence of this text is that if TS_SECLABEL
is used, then it is applied for the whole Traffic Selector for this Child SA.
Am I missing the essence?

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

No, I'm fine with TS.

> > 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.

I still think it's a repetition of a requirement. I believe that properly 
written standard must not contain repetitions of requirements.
If you really really want to keep it here (do defense against sloppy implementers?),
then change the text to something like:

    Section XX of this document updates definition of TS payload,
    so that it must contain at least one TS_TYPE of
   TS_IPV4_ADDR_RANGE or TS_IPV6_ADDR_RANGE.

(note lowercase "must" and reference to actual definition).

> > 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.

Better. thanks. I'm a bit concerned of the generality of this requirement.
If tomorrow somebody introduces new TS_TYPE, that will be allowed
to have multiple values in response, then the text "plus one of each further TS_TYPE"
will prevent responder from doing that. So this text is true for 
TS_SECLABEL, but is not necessarily true for not yet defined TS_TYPEs.

> > 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.

Fine.

> > 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.

I understand your example, but I failed to understand how it is aligned
with the current document. In particular, my understanding
of the negotiation process is that the initiator includes 
several possible seclabels and the responder selects one of them, 
which means that both agreed to use the selected seclabel
in both directions.

If my understanding is wrong and you meant that 
the initiator can use "any" of the proposed seclabels,
while responder can only use one of them - which
it has selected, then you must explain this in more details.
Moreover, I failed to understand what the responder
has to do in this situation if its policy allows initiator
to use only one seclabel. You describe a similar case 
for the initiator, but not for the responder.

So, I believe that unless more clarifications are
added to the document and all these cases are
described in details, this para must be removed,
because it describes the case that contradicts
to the rest of the document and cannot be 
implemented using this document.

> > 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.

I think that this part

     Since a rekey
      MUST NOT narrow down the Traffic Selectors narrower than the scope
      currently in use, the only valid choice of TS_SECLABEL

must be change to 

      According to RFC 7296  Section 2.9.2, a rekey operation 
      must not narrow down the Traffic Selectors narrower than the scope
      currently in use. For this reason the only valid choice of TS_SECLABEL

So, it's clear that this document doesn't impose any new requirement
here, it only refers to RFC 7296's one.

Regards,
Valery.


> diffs available at:
> 
> https://www.ietf.org/rfcdiff?url2=draft-ietf-ipsecme-labeled-ipsec-06.txt
> 
> Paul