On Thu, 21 Jan 2021, Valery Smyslov wrote:

I wonder what's rational behind this restriction. From my point of view
zero length TS_SECLABLE can be used to express that using Security Labels
is optional. I.e. initiator can include zero length TS_SECLABEL Traffic Selector
along with other  TS_SECLABEL Traffic Selectors to indicate that responder
is free to not use security labels for the SA.

We did discuss this earier in the WG. There is no use case of "optional"
security labels. You either insist on these, or you don't use these.

Well, it contradicts what draft says (Section 2.2):

  If the Security Label traffic selector is optional from a
  configuration point of view, the initiator will have to choose which
  TS payload to attempt first.  If it includes the Security Label and
  receives a TS_UNACCEPTABLE, it can attempt a new Child SA negotiation
  without that Security Label.

So you do allow security label to be optional, but negotiate its usage this
with most complex and ineffective way. It's very strange.

Again, this was the result of discussion in the WG where you were very
insistent on how to interpret and send TS payloads. So I'm a little
confused about this being an issue now.

The text above shows how you could implement optional labels, which is
deemed a rare or not needed feature for most implementations. It tells
you to basicaly reconfigure on the fly. The advantage is that the whole
discusion of "optional" does not leak into the IKE TS negotiation
protocol. If you think it would be more clear if we don't describe
the above option, I'm fine with removing that text.

Implementing "optional" ones by using a zero length label is asking for
implementation problems. Especially when people consider these labels
as strings and run strlen() on the empty (but NULL terminated string)
and interpret that as wildcard.

Well, people will consider meaning of the security labels in accordance
to what is written in the draft. If we try to envision every possible 
mis-reading
of the documents we'd better not to start writing them...

It is not about misreading the document. It is to ensure there are less
conditions where there is ambiguity. Does a NULL TS_SECLABEL mean "no
sec label" or "optional sec label" or "wildcard sec_label". We can write
it clearly in the draft, but it is still a flawed concept. The clearest
solution is if you want no restructions on sec_label, to not include any
TS_SECLABEL traffic selector. Removing the zero length from being valid
makes it very clear this cannot be used for anything, including meaning
"a wildcard" or "nothing". If you want a wildcard, you can send a
TS_SECLABEL containing the string "*" (and then figure out whether it is
a single character label or has a string NUL termination :P )

And BTW zero length means that there are no any content, including NULL 
terminator :-)

I bring it up because it is a real danger this will get badly
implemented. And it already is. The Linux kernel takes these values in
a struct:

struct xfrm_user_sec_ctx {
        __u16                   len;
        __u16                   exttype;
        __u8                    ctx_alg;  /* LSMs: e.g., selinux == 1 */
        __u8                    ctx_doi;
        __u16                   ctx_len;
};

So you specify the label with ctx_len. Yet the "ip xfrm state" output
shows a trailing @ character, suggesting it is already treating this
as a NUL terminated string. Oddly enough, the output of "ip xfrm policy"
shows the same label without the trailing @.

This is why I originally had stated that the label could not contain a
NULL, but during the process, all this got removed for treating it as
an opaque blob. The libreswan implementation, which uses "strings" of
SElinux values, checks for embedded NULLs and rejects these. But that
is arguably part of the "label consumer" and not IKE.

Since there is no use case, and significant implementation danger, the
document makes an explicit case to reject these. Our implementation
will return TS_UNAVAILABLE if it encounters any zero length TS_SECLABEL.

No, the document does allow optional security labels, see above.

Not really. It says you first try negotiating "with labels" and if that
fails try to negotiate "without labels". That is different from
supporting a negotiation with optional labels.

Again, it was you who was very strongly of the belief that optional
traffic selectors should not be allowed and that one MUST pick one
of each TS type. And that you must be _selecting_ on something, and
you cannot select on nothing.

Additionally, an implementation accidentally outputting an empty label
into the TS_SECLABEL payload should not introduce a wildcard label in
my opinion. It is just a little safer for them to need to create a "*"
payload.

The case you describe is very unlikely. Either the endpoints wants a
label or doesn't want one. optional security is not security.

Then why text in 2.2 allows the initiator to choose between using them
and omitting them? Isn't it exactly what you just described as "not security"?

It just describes something you can do with any new Traffic Selector
that will ever be introduced to IKE. You can always try it with it
first, and without it as fallback. It is not a propery of TS_SECLABEL.

Another my concern is that draft allows security labels to differ
in TSi and TSr without giving any possible semantic interpretation for this.

Correct. But the label is clearly defines as opaque to the IKE layer.
The IKE layer should not be making any kindds of assumptions or
interpretations, but just relay this to the underlying label security
consumer (eg in our case the LSM of SElinux). Note that our
implementation only supports symmetrical labels so far, because
that is how SElinux labeling works. But the protocol can accept
other methods.

I still prefer _some_ words to be added about _possible_
semantics when seclabels are different in TSi and TSr.

Please provide text, but be aware that the document treats the label as
opaque to IKE.

This looks weird. I think that at least some possible interpretation
of such situation must be given, e.g. TS_SECLABEL in TSi is used
for traffic from initiator to responder and TS_SECLABEL in TSr
is used for the return traffic. In this case it is clear that
they can differ in some situations.

I'm not sure why this is not clear? TSi and TSr entries apply to
one direction.  TS_SECLABEL does not change that.

It's unclear what this case could mean:

        TSi = ((0,0,198.51.0-198.51.255),
               TS_SECLABEL1)

        TSr = (((0,0,203.0.113.0-203.0.113.255),
               TS_SECLABEL2)

It's true that IKE doesn't interpret seclabels, but it usually
does some sanity checks and as implementer I'm not sure whether
this is allowed (what does it mean?) or not.


It just tells you that both endpoints will apply the specific label
to the specific IP/proto/ports range when receiving these packets.

I mean, it could even mean a specific group, eg group "seclabel1" and
group "seclabel2". eg:

         TSi = ((0,0,198.51.0.0-198.51.0.255),
                "jeeps")

         TSr = (((0,0,203.0.113.0-203.0.113.255),
                "awacs-plane")

And the jeeps could only accept awacs-plane traffic, and the awacs-plane
could only accept jeeps traffic.

Sure, in my use case, these will be symmetric, eg: 
system_u:object_r:ipsec_spd_t:s0

How about this clarification:

        A responder MUST create its TS response by selecting at least
        one of each TS Type.  One exception is that a responder MAY omit
        a TS_IPV4_ADDR_RANGE or TS_IPV6_ADDR_RANGE as long as at least
        one TS_IPV4_ADDR_RANGE or TS_IPV6_ADDR_RANGE is selected.
        Furthermore, a narrowed TS_IPV4_ADDR_RANGE and TS_IPV6_ADDR_RANGE 
content
        set MAY be selected.

Much better, thank you. For best clarity I would have added a sentence that 
this draft
doesn't change a logic described in Section 2.9 of RFC 7296 for
types TS_IPV4_ADDR_RANGE and TS_IPV6_ADDR_RANGE, it only
defines additional rules for how TS_SECLABEL traffic selectors are negotiated.

The idea was to update the TS handling in general for new TS types, not
just TS_SECLABEL. So that new TS Types in the future do not need to
update RFC 7296. So how about this change:

        A responder MUST create its TS response by selecting at least
        one of each TS Type. For this selection, TS_IPV4_ADDR_RANGE
        and TS_IPV6_ADDR_RANGE are considered one (IP) type. That is,
        it is valid to choose 1 TS of type TS_IPV4_ADDR_RANGE and zero
        TS of type TS_IPV6_ADDR_RANGE. TS Types supporting narrowing
        may be narrowed down upon their selection, as it described in
        Section 2.9 of RFC 7296.

Paul

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

Reply via email to