Re: [I2nsf] [IPsec] WGLC and IPR poll for draft-ietf-i2nsf-sdn-ipsec-flow-protection-04

Paul Wouters <paul@nohats.ca> Fri, 24 May 2019 18:38 UTC

Return-Path: <paul@nohats.ca>
X-Original-To: i2nsf@ietfa.amsl.com
Delivered-To: i2nsf@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id E8BAA1202EA; Fri, 24 May 2019 11:38:09 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.999
X-Spam-Level:
X-Spam-Status: No, score=-1.999 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (1024-bit key) header.d=nohats.ca
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id tuaVPXoiAB8K; Fri, 24 May 2019 11:38:07 -0700 (PDT)
Received: from mx.nohats.ca (mx.nohats.ca [IPv6:2a03:6000:1004:1::68]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 72EE312004B; Fri, 24 May 2019 11:38:07 -0700 (PDT)
Received: from localhost (localhost [IPv6:::1]) by mx.nohats.ca (Postfix) with ESMTP id 459Zqx0kYJzBT; Fri, 24 May 2019 20:38:05 +0200 (CEST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nohats.ca; s=default; t=1558723085; bh=Ft6LYGlUf8kgkzVjK+apMrkZOQq/U6gdhk/oNwfc9PY=; h=Date:From:To:cc:Subject:In-Reply-To:References; b=N1lXuNdfFFiFq3mhwLJh8uK2v2ULlbVYvadEqhn7ylgYJZT+LUfPycIftuihj6x3B 0IKxVjshU32a487ZPpM2TOVIcLtVvCYzxGcFXiHA5L1x7eXDf9Cohhng/RY3YaRfKv ryKNBbSoFMJL7ywV9G1jEexhUkIntwXBPXiZWkg8=
X-Virus-Scanned: amavisd-new at mx.nohats.ca
Received: from mx.nohats.ca ([IPv6:::1]) by localhost (mx.nohats.ca [IPv6:::1]) (amavisd-new, port 10024) with ESMTP id JX88BdpYJ1Hg; Fri, 24 May 2019 20:38:01 +0200 (CEST)
Received: from bofh.nohats.ca (bofh.nohats.ca [76.10.157.69]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx.nohats.ca (Postfix) with ESMTPS; Fri, 24 May 2019 20:38:00 +0200 (CEST)
Received: by bofh.nohats.ca (Postfix, from userid 1000) id 0053A898; Fri, 24 May 2019 14:37:57 -0400 (EDT)
DKIM-Filter: OpenDKIM Filter v2.11.0 bofh.nohats.ca 0053A898
Received: from localhost (localhost [127.0.0.1]) by bofh.nohats.ca (Postfix) with ESMTP id DC212401B15B; Fri, 24 May 2019 14:37:57 -0400 (EDT)
Date: Fri, 24 May 2019 14:37:57 -0400 (EDT)
From: Paul Wouters <paul@nohats.ca>
To: Rafa Marin-Lopez <rafa@um.es>
cc: =?ISO-8859-15?Q?Fernando_Pere=F1=EDguez_Garc=EDa?= <fernando.pereniguez@cud.upct.es>, "i2nsf@ietf.org" <i2nsf@ietf.org>, Gabriel Lopez <gabilm@um.es>, Linda Dunbar <linda.dunbar@huawei.com>, "ipsec@ietf.org WG" <ipsec@ietf.org>
In-Reply-To: <ED73306E-F807-42A4-B063-D45E133B8419@um.es>
Message-ID: <alpine.LRH.2.21.1905241401320.3939@bofh.nohats.ca>
References: <4A95BA014132FF49AE685FAB4B9F17F66B3869DE@sjceml521-mbs.china.huawei.com> <DBBD75C3-9FB3-473F-A627-062DB3F5C32D@um.es> <alpine.LRH.2.21.1904210811200.1903@bofh.nohats.ca> <ED73306E-F807-42A4-B063-D45E133B8419@um.es>
User-Agent: Alpine 2.21 (LRH 202 2017-01-01)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8BIT
Archived-At: <https://mailarchive.ietf.org/arch/msg/i2nsf/O6BMPEKtePpsEyEFbjtqdf8fB84>
Subject: Re: [I2nsf] [IPsec] WGLC and IPR poll for draft-ietf-i2nsf-sdn-ipsec-flow-protection-04
X-BeenThere: i2nsf@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: "*I2NSF: Interface to Network Security Functions mailing list*" <i2nsf.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/i2nsf>, <mailto:i2nsf-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/i2nsf/>
List-Post: <mailto:i2nsf@ietf.org>
List-Help: <mailto:i2nsf-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/i2nsf>, <mailto:i2nsf-request@ietf.org?subject=subscribe>
X-List-Received-Date: Fri, 24 May 2019 18:38:10 -0000

On Fri, 24 May 2019, Rafa Marin-Lopez wrote:

> Sorry for the delayed answer. Our answer took us more time than expected preparing. Thank you very much again. Please see our comments inline:

No problem, thanks for addressing the issues!

> [Authors] We have rephrased as:
> 
> “In principle, IKE case is easier to deploy than IKE-less
> case because current gateways typically have an IKEv2/IPsec 
> implementation. Moreover hosts can install easily an IKE implementation."

Looks good.

>         The main
>         reason is that the Security Controller is not able to observe any
>         session keys generated for the IPsec SAs because IKEv2 is in charge
>         of negotiating the IPsec SAs.
>
>       I think you mean "because the nodes, not the Security Controller, is
>       generating the session keys". I would not use the word IKEv2, because
>       it is unclear if that IKEv2 would be running on the SG or the node.
> 
> 
> [Authors] That’s right. Let’s only write this sentence:
> 
> "The main reason is that the NSFs are generating the session keys and not the Security Controller”.

Looks good.

>         For IKE case, the rekeying process is carried out by IKEv2, following
>         the information defined in the SPD and SAD.
>
>       That is not the whole story though? Isn't is the Security Controller
>       that decides when to terminate a connection? I guess it is implied here
>       that we are talking about connections that are instructed by the SG to
>       stay up long enough to need rekeying?
> 
> 
> [Authors] The Security Controller may decide to terminate a connection but, in principle, if nothing unexpected happens after applying IKE configuration
> will allow IKE to operate as configured. So, yes, it is expected that connections will live unless something different is required by the administrator or
> the SC detects something wrong. 

Agreed. no modifications needed.

>       Section 5.3.1 talks about a bundle of two inbound SA's. That is a little
>       confusing as traditionally we talk about the bundle being an inbound and
>       outbound SA from the perspective of one of the endpoints. I understand
>       you are doing this so you can talk about installing inbound on both ends
>       first. Perhaps talk initially about the inbound and outbound bundle,
>       then state you are handing out what is the inbound sa from the point of
>       view of each endpoint first ?
> 
> 
> [Authors] We can add the following paragraph. How about this one?:
> 
> "Traditionally, during a rekey process of the IPSec SA using IKE, a bundle of inbound and outbound SAs, from the perspective of one of the NSFs, is taken
> into account. For example, if the inbound SA expires both the inbound and outbound SA are rekeyed at the same time in that NSF. However, when IKE is not
> used, we have followed a different approach to avoid any packet loss during rekey: the Security Controller installs first the new inbound SAs in both NSFs
> and then, the outbound SAs."

Looks good.

>         It is worth noting that if the IPsec implementation
>         can itself detect traffic on the new IPsec SA, and it can delete
>         the old IPsec SA itself without instruction from the Security
>         Controller, then this step 3 is not required.
>
>       Technically, they can never know this because IKEv2 allows installing
>       multiple IPsec SA's with the exact same policies.
> 
> 
> [Authors] We are talking here about IKE-less case so there is no IKEv2. This paragraph was added after , as far as I remember, your comment about this
> possibility. I think if the IPsec SAs have different SPIs the SDN controller can distinguish among them

Yes, but that was after I was pointed to this text in RFC 7296:

    Note that IKEv2 deliberately allows parallel SAs with the same
    Traffic Selectors between common endpoints.  One of the purposes of
    this is to support traffic quality of service (QoS) differences among
    the SAs (see [DIFFSERVFIELD], [DIFFSERVARCH], and Section 4.1 of
    [DIFFTUNNEL]).  Hence unlike IKEv1, the combination of the endpoints
    and the Traffic Selectors may not uniquely identify an SA between
    those endpoints, so the IKEv1 rekeying heuristic of deleting SAs on
    the basis of duplicate Traffic Selectors SHOULD NOT be used.

So two IPsec SA's might be the same _and_ one of these might not see
much if any traffic. Still it should not get deleted. So I would now
say my original suggestion is wrong, and we should remove the paragraph
that starts with "It is worth noting that if the IPsec implementation"

> [Authors] Then the step 3 still applies. Most probably removing the paragraph should be enough, right?

Yes just remove it

>       It can also instruct the affected NSF to send IKEv2 INITIAL_CONTACT.
>
>       I think this is something the IKEv2 implementation would determine on its own.
>       I would remove the sentence or change it to say the node can determine it
>       needs to send INITIAL_CONTACT. Also, this removes the need for the SC to
>       have to relay this option to the IKEv2 NSF.
> 
> [Authors] We have observed some implementations (e.g. libreswan) has this variable initial-contact as well in the ipsec.conf. That is why we decide to
> keep it. Is it not useful then?

We added it because sending or not sending it might change te behaviour
of the other endpoint. libreswan as implementation ignores the payload
completely, because it has all the known information/state needed.
Although based on the above new finding of multiple IPsec SA's, this
might change.

I guess my point was more to the fact that instructing the NSF to send
an INITIAL_CONTACT seems to mixup instructions from the SC. If the SC
says "bring tunnel X to GW Y up", and then later says "bring tunnel Z
up to GW Y and send INITIAL_CONTACT", it is implicitly sending a message
to the NSF to bring tunnel X to gateway Y down. Now the NSF and SC might
be out of sync if the SC wasn't expecting this result.

>         In IKE-less case, if the Security Controller detects that a NSF has
>         lost the IPsec SAs (e.g. it reboots) it will delete the old IPsec SAs
>         of the non-failed nodes established with the failed node (step 1).
>         This prevents the non-failed nodes from leaking plaintext.
>
>       Wouldn't doing that before installing a new IPsec SA not actually _cause_
>       leaking plaintext? Unless the non-failed nodes, upon receiving the delete
>       would put in an SPD policy that would catch packets and trigger an acquire.
> 
> 
> [Authors] This is an interesting question. We must state that, for security reasons, any NSF should have a default DISCARD policy. A NSF restarting should
> not allow any traffic unless SC says so. In other words, Since the NSF needs information coming from the SC, if that information is not in place yet the
> safest option is DISCARD action.

Sure. I think adding some text that says a connection in the "configured
but not up" state is expceted to drop or hold onto the packets, that
would make it clear.

>       +--rw security-protocol?          ic:ipsec-protocol
>
>       What does this stand for? There is no "IPsec protocol" in the sense that we do not have
>       an IANA protocol entry for "IPsec" (only for ESP)
> 
> 
> [Authors] I understand. The intention was to say ipsec related protocol. Should we use something like ic:ipsec-related-protocol? or  ic:sec-protocol?

or maybe ipsec-protocol-parameters ?

>         NSFs MUST NOT allow the reading of these values once they have been
>         applied by the Security Controller (i.e. write only operations).
>
>       These are not really "applied by the Security Controller". Perhaps you
>       meant "obtained" instead of “applied"
> 
> [Authors] Since we were referring to IKE-less case the Security Controlles sends these keys to the NSF. In that case, the Security Controller applies the
> keys but it MUST NOT BE obtained (read) by anyone (including the SC).

Perhaps use:

 	NSF's receiving private key material from the SC, MUST NOT allow the reading .....

> [Authors] We are writing version -05 of these changes will be applied.
>
>          typedef ipsec-upper-layer-proto {
>                              type enumeration {
>                                      enum TCP { description "TCP traffic"; }
>                                      enum UDP { description "UDP traffic"; }
>                                      enum SCTP { description "SCTP traffic";}
>                                      enum DCCP { description "DCCP traffic";}
>                                      enum ICMP { description "ICMP traffic";}
>                                      enum IPv6-ICMP { description "IPv6-ICMP traffic";}
>                                      enum GRE {description "GRE traffic";}
>                              }
>                              description "Next layer proto on top of IP";
>                      }
>
>       I don't understand ipsec-upper-layer-proto? If this is encapsulation protocol, then
>       there is only TCP and UDP. If this is the protocol of the encrypted data, then there
>       are a lot more protocols. The document also nowhere explains the term "upper layer protocol"
>       I think you mean what I would call the "inner protocol" so that it is every number from the
>       IANA protocol registry.
> 
> 
> [Authors] Basically RFC 4301 mentions next layer protocols and we had that in previous versions but it was suggested that was not a good name. So we used
> this one. We can use inner protocol. Thus it is not encapsulation but the protocols in the next header after IP header.

Ok, so then I think inner protocol would be more clear.

> "Next Layer Protocol: Obtained from the IPv4 "Protocol" or the
>         IPv6 "Next Header" fields.”
> 
> So it seems  that every number from the IANA protocol registry should be there. So here we have again the same problem as crypto algorithms. However, we
> can do as RFC 8519 : 

This one did not get answered. Do you plan to link to the protocol
registry at IANA ?

>                      typedef pfs-group {
>                              type enumeration {
>                                      enum NONE {description "NONE";}
>                                      enum 768-bit-MODP {description "768-bit MODP Group";}
>                                      enum 1024-bit-MODP {description "1024-bit MODP Group";}
>                                      enum 1536-bit-MODP {description "1536-bit MODP Group";}
>                                      enum 2048-bit-MODP {description "2048-bit MODP Group";}
>                                      enum 3072-bit-MODP {description "3072-bit MODP Group";}
>                                      enum 4096-bit-MODP {description "4096-bit MODP Group";}
>                                      enum 6144-bit-MODP {description "6144-bit MODP Group";}
>                                      enum 8192-bit-MODP {description "8192-bit MODP Group";}
>                              }
>                              description "PFS group for IPsec rekey";
>                      }
>
>       This is the only entry still enumerating (partially!) an IANA registry.
> 
> 
> [Authors] Right. But I am not sure if there is a final conclusion about this dilemma. Personally, I still prefer to have a uint32 and refer Group
> Description (Value 4) in 
> 
> https://www.iana.org/assignments/ipsec-registry/ipsec-registry.xhtml

That's an old IKEv1 registry, you would want to link it to:

https://www.iana.org/assignments/ikev2-parameters/ikev2-parameters.xhtml#ikev2-parameters-8

And it would be a uint16, not a uint32.

>       Use "Configuration of IPsec Security Association (SA). Section 4.4.2.1 in RFC 4301"
>       To ensure it is clear this is not about an IKE SA. Same on the next line for
>       "for a particular SA"
>
>         leaf seq-number-overflow-flag { type boolean; description "The flag indicating whether overflow of the sequence number counter should
>       prevent transmission of additional packets on the SA, or whether rollover is permitted."; }
>
>       What is the source of this? I thought sequence numbers were never allowed to overflow?
> 
> 
> [Authors] According to RFC 4301 - 4.4.2.1.  Data Items in the SAD:
> 
> Sequence Counter Overflow: a flag indicating whether overflow of
>       the sequence number counter should generate an auditable event and
>       prevent transmission of additional packets on the SA, or whether
>       rollover is permitted.  The audit log entry for this event SHOULD
>       include the SPI value, current date/time, Local Address, Remote
>       Address, and the selectors from the relevant SAD entry.


Hmm, I have never heard of this so I should look into it, but I guess it
means you should have the option for it, but please default it to not
allow rollover, and it would need to forbid rollover for AEAD's.

Tero, do you know anything more about this feature? I don't think we
could ever negotiate it via IKE anyway?

>         leaf anti-replay-window { type uint16 { range "0 | 32..1024"; } description "Anti replay window size"; }
>
>       Why cap at 1024? That could be too low for 100gbps connections.
> 
> 
> [Authors] The truth is that RFC 4301 mentions:
> 
> Anti-Replay Window: a 64-bit counter and a bit-map (or equivalent)
>       used to determine whether an inbound AH or ESP packet is a replay.
> 
> So we can replace this with uint64. On the other hand, we have observed with uint16 should be enough in real cases. We can remove the restriction (1024) .
> what do you think?

Yes, I think the uint16 is more than enough, as long as you don't mention any cap like 1024.

> [Authors] Right.
>
>         leaf replay {type uint32; default 0; description "packets detected out of the replay window and dropped because they are replay packets";}
>
>       That should be: "inside the replay window”

> [Authors] Good catch. Changed.

Actually, now I am not sure anymore :)

Outside the replay window, if the number is too low, it should be
dropped. If it is too high, I think once decrypted successfully, the
window is updated. If the packet is within the window, we keep a list
of received / not-received sequence numbers, and drop duplicates ?

So I think I misled you and you were correct with the original text.

> [Authors] If integrity node does not appear in the configuration sent by the controller, the integrity interpretation is none. We can comment this. 

> Our comment is if the encryption container is sent to the NSF with an AEAD algorithm the container integrity is not sent , not required since AEAD
> algorithm provides integrity (and encryption)

Just to clarify, some (bad) implementations handle "no integrity
transforms" differently from "the integrity transform NONE" (value 0)

I know older libreswan versions did it wrong. However, I am not sure if
this needs to be exposed here. I think it is fine to that the model
states to not send any integrity transform, and that the implementation
properly handles it when it receives an ENCR transforms AES_GCM with
an INTEG transform NONE.

> [Authors] combined-enc-intr has been removed.

Great!

>       I see IPcomp is not used anywhere. I agree with that but I'm not sure if there is WG conensus for that.
> 
> [Authors] We did not include IPcomp from the beginning and until now, there has not been any comment in this regard.

Okay. I guess the model can always be extended in the future if needed.

Thanks!

Paul