On Tue, 2 Nov 2021, Valery Smyslov wrote:

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.

This is true, even if only because those are the only valid TS_TYPEs
that exist in RFC 7296. I could omit it to better match the old text,
but I find that a bit confusing. The goad of 1.2 is to point out the
problem if RFC 7296 using "Traffic Selector" to mean both the selector
and the selector set / payload. If you insist, I can remove:

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


Why it is repeated here?

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

OK.

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.

We do need to take implementations and common implementation mistakes
into consideration when designing a protocol.
But also, a security label of "nothing" isn't really a valid security
label selector. It is too ambiguous with "no security label". It makes
common sense to me to simply not allow it. And comparing a 0 size NULL
with a zero byte also seems just way to risky. Ideally, I would have
stated "a minimum length of one non-zero octet".

I guess we need to get other people's opinion on this.


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?

In our implementation you can define two "conns", one with and one
without. And choosing the right one will work. On the responder, there
is a connection switching mechanism that can switch to the better
matching connection once the TS payloads are read from IKE_AUTH. So
first you add all the conns with sec_label. And after a while, you can
just remove all the conns without sec_label.

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 mean to say, if you pick a TS with TS_SECLABEL, you MUST have a
security label as part of your SPD selector mechanism. You cannot
decide to ignore it and only use IP address based SPD selectors.

In our implementation, using the selector means _setting_ the SElinux
context (sec_label) on the decrypted packet before releasing it to the
OS for further processing by the application. The application might make
security decisions based on the sec_label, so not "applying" the sec_label
negotiated would be a security violation of the negotiated policy.

That is, things might appear to work but in fact might be working
without sec_label security and the remote peer has no way of detecting
this. It is trusting the peer to apply the security policy to the
packets.

If you have a suggested text you prefer, please let me know.

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

I avoided talking about IPsec / Child SA because different ones can have
different negotiated TS_SECLABELS - eg they can be independent from the
exchange we are talking about. (I think we are saying the same thing?)

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

sure, if that makes you happy. I have no objection.

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.

Yes, so such a new TS_TYPE will have to Update: this document, just like
this one has to Update: 7296. This is exactly the kind of issue Update:
is designed for. The generality is there and we are updating it for
specific TS_TYPEs.

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.

It does. If both endpoints have an "nfs-client" and "nfs-server" role.

But you are now getting very close to "interpreting sec_labels", which
the IKE stack should not do :)

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.

Your understanding is right. It is just that the "nfs-server" might
not have any "nfs-client" traffic since it is a server and not a client.

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.

traffic covered has 1 seclabel. If the initiator sends multiple
acceptable ones, the responder picks one. I thought this was clear
in the document?

In our implementation, this never happens and there is only ever
one sec_label the initiator proposes. It receives a seperate
ACQUIRE for each sec_label and sets up individual Child SA's for
each sec_label. But that is our implementation. I again did not
want to exclude other implementations with different mechanisms
on their sec_label to be limited to our personal requirements.
It just seemed that allowing multiple sec_labels within one
Child SA seemed dangerous, and one would be better of defining
a third type of sec_label value that incorporations the two
base types of sec_label one wants to include. Eg if you want
the connection to accept SECPOL1 and SECPOL2, define a label
SECPOL3 that means "SECPOL1 or SECPOL2", or even call it
"SECPOL1|SECPOL2".

If the WG believes we should be more generic and allow picking
multiple ones, this can be changed.

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.

I don't understand what you mean?

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.

I'll make that change.

Paul

_______________________________________________
IPsec mailing list
IPsec@ietf.org
https://www.ietf.org/mailman/listinfo/ipsec

Reply via email to