Subject: Re: [I2nsf] Some comments / questions on draft-abad-i2nsf-sdn-ipsec-flow-protection-03

From: Rafa Marin-Lopez <[email-masked]>
Date: Mon, 23 Oct 2017 11:59:58 +0200

Hello Paul:

Sorry for this delayed answer. We are now revising our I-D and we want to apply your comments and suggestions.

Please see our comments inline.

> El 17 sept 2017, a las 19:03, Paul Wouters <[email-masked]> escribió:
> 
> 
> I had another look at the draft.
> 
> I am not in favour or against adoption of this draft.
> 
> However, I am wondering if the two use cases are not better split into
> two separate documents, as the requirements, deployment and security
> considerations are very different. It would also allow implementors to
> more clearly indicate supporting one or both use cases by referencing
> RFC numbers.

[Rafa] Chairs already answered this question.

> 
> Specific comments on text follow below.
> 
> 
> In section 6.2, I see a difference in ah-sa and esp-sa. The document
> has:
> 
> +--rw ah-sa
>   +--rw integrity-algorithm?
>   +--rw key?
>   +--rw esp-sa
>   |     |  +--rw encryption
>   |     |  |  +--rw encryption-algorithm?   encryption-algorithm-t
>   |     |  |  +--rw key?                    string
>   |     |  |  +--rw iv?                     string
>   |     |  +--rw integrity
>   |     |  |  +--rw integrity-algorithm?   integrity-algorithm-t
>   |     |  |  +--rw key?                   string
>   |     |  +--rw combined
>   |     |     +--rw combined-algorithm?
> 
> This seems inconsistent between ah-sa and esp-sa. More consistent would
> be:
> 
> +--rw ah-sa
>   +--rw-integrity
>   |  +--rw integrity-algorithm?
>   |  +--rw key?
>   +--rw esp-sa
>   |     |  +--rw encryption
>   |     |  |  +--rw encryption-algorithm?   encryption-algorithm-t
>   |     |  |  +--rw key?                    string
>   |     |  |  +--rw iv?                     string
>   |     |  +--rw integrity
>   |     |  |  +--rw integrity-algorithm?   integrity-algorithm-t
>   |     |  |  +--rw key?                   string
> 
> That is two changes:
> 
> 1) Give rw ah-sa a subsection rw-integrity, even if it is the only subsection.

[Rafa] Done.

> 2) Don't define rw combined on its own. It is an rw encryption-algorithm.
>   (also, otherwise rw combined needs rw key and rw iv)
>   (alternatively rw combined could be a bool)
>   (also see below)

[Rafa] So you propose to add rw combined bool under rw esp-sa, correct? I think having encryption-algorithm on its own should be enough.

> 
> The rw encap entry should add an entry for rw espintcp to indicate
> support for RFC 8229.

[Rafa] Done.

> 
> I'm not sure why the rpcs ro authentication and ro encryption entries
> have "max-bits" and "min bits" entries. The idea that we would have
> algorithms with variable key length was only ever valid for the
> (basically abandoned) CAST and (never adopted) blowfish/twofish
> algorithms. These days we have a discrete set of bits, at the moment
> 128/192/256 only. (libreswan IKE has removed some scar tissue based
> on this, where it had concepts of min/default/max that were removed)

[Rafa] Yes, that was looking at the structure returned by register in PF_KEYv2. We can simply add a key_length value, instead.

> 
> I'm confused by the SAD entries have sa-lifetime's of "soft" and "hard".
> The counters in the SPD sure need to have these, but the counters in
> the SAD are representing the actual counters themselves, so there is
> no hard or soft.

[Rafa] 

Section 4.4.2.1.  Data Items in the SAD in RFC 4301 mentions:

"
There SHOULD be two kinds of lifetime -- a soft lifetime that
         warns the implementation to initiate action such as setting up
         a replacement SA, and a hard lifetime when the current SA ends
         and is destroyed.”

When you do an "ip -s x state list" you can see the soft and hard values are in the IPsec SA as well (besides the current value as you mentioned)

Also you can use "ip x xfrm add" to specify those limits, right?

> 
> The notifications could use an entry for sadb_bad-spi, as it would
> be useful to somehow notify that packets with unknown SPI's are
> received, which indicated one of the two endpoints rebooted but the
> other end isn't aware and still sending encrypted packets that
> can no longer be decrypted.

[Rafa] In my opinion, this is interesting. However, I was thinking that NSF would generate many of those notifications one per packet that have a bad SPI, am I correct? I think we should limit the notification to the first packet of a flow that has a bad spi.
> 
> It seemds IPCOMP has been left out everywhere. I'm not sure if that
> was by design or not. (I think IPCOMP should die, so I'm fine with it)

[Rafa] It was not by design. But if most people agree we can leave it as it is right now.

> 
> Section 6.4
> 
> the rw autostartup is a boolean, but we tend to have three states here,
> alwayson, initiate-on-demand, or respond-only.

[Rafa] Perfect, thank you. Could you provide a short description so we can add it to the I-D. By now, we have defined these three values.

 typedef type-autostartup
  {
  type enumeration {
      enum ALWAYSON { description " ";}
      enum INITIATE-ON-DEMAND {description " ";}
      enum RESPOND-ONLY {description " ";}
  }
  
  }

> 
> nat-traversal should probably be expanded from a boolean, due to the
> ESPinUDP and ESPinTCP and ESPinTLs options. It should also have a port
> value since TCP/TLS could run on different ports.

[Rafa] Correct. Now we have the following: nat-traversal is still a boolean but in case it is true we add a container con encapsulation information.

leaf nat-traversal {
    				type boolean;
    				default false;
    				description "Enable/Disable NAT traversal";
    			}
                
container encap { 
                    when "../nat-traversal = 'true'";
                    description "Encapsulation container";
                    leaf espencap { type esp-encap; description "ESP in TCP, ESP in UDP or ESP in TLS";}
                    leaf sport {type inet:port-number; description "Encapsulation source port";}
                    leaf dport {type inet:port-number; description "Encapsulation destination port"; }
                    leaf oaddr {type inet:ip-address; description "Encapsulation Original Address ";}
                }

> 
> It seems for IKE, combined algorithms were not seperated from enc+integ
> algorithms (see previous comment) as was done for IPsec.

[Rafa] Your suggestion for IPsec above makes sense.
> 
> If I read the syntax correctly, there seems to only be one PFS group
> possible? Often IKE proposals allow more then one.

[Rafa] Now it is a leaf-list structure.

> 
> traffic selectors seem also to be limited to only single IP-to-IP without
> port or protocol selectors?

[Rafa] Traffic selectors are defined in SPD model as specified in RFC 4301. In particular, as you can observe there is: 

rw traffic-selector-list* [ts-number]
		|     |   +--rw ts-number              uint32
              |     |     +--rw direction?             ipsec-traffic-direction
              |     |     +--rw local-addresses* [start end]
              |     |     |  +--rw start    inet:ip-address
              |     |     |  +--rw end      inet:ip-address
              |     |     +--rw remote-addresses* [start end]
              |     |     |  +--rw start    inet:ip-address
              |     |     |  +--rw end      inet:ip-address
              |     |     +--rw next-layer-protocol*   ipsec-next-layer-proto
              |     |     +--rw local-ports* [start end]
              |     |     |  +--rw start    inet:port-number
              |     |     |  +--rw end      inet:port-number
              |     |     +--rw remote-ports* [start end]
              |     |     |  +--rw start    inet:port-number
              |     |     |  +--rw end      inet:port-number
              |     |     +--rw selector-priority?     uint32

The asterisks you can see reflects that there are several IP addresses, several next layer protocols , ports, etc…

Are we missing something?

> That seems to conflict with an earlier value
> in Section 6.2 indicating support for Populate From Packet (pfp) which
> would create multiple IPsec SA's limited by protocols and ports.

[Rafa] In reality, no traffic selector is specified in the IKEv2 configuration since all that information is in the SPD model. In fact, SPD model is based on the information we found in RFC 4301. Therefore, we think the confusion comes from 

+--rw local-addrs        inet:ip-address
+--rw remote-addr        inet:ip-address

But basically those are the endpoints of the IKEv2 SA.

> 
> phase2-lifetime seems to only have a uint32 value. I don't understand
> what unit this is in? And what happend to the policies for maxtime,
> maxpackets and maxbytes?

[Rafa] True, that must be corrected. In fact, phase2-lifetime would not make any sense here since those values are already established in the SPD model. 

That also makes me think that phase1-auth is redundant since it should be in the PAD. In the PAD model we have auth-method-grouping that includes auth-method-type

> 
> Section 7.1
> 
> In use case 2 it states "In this case the execution of IKE is not necessary
> in the controller". I would say that you MUST NOT run an IKE daemon in
> that case because otherwise the Security Controller and the IKE daemon
> are going to fight each other for the SAD/SPD states.

[Rafa] Considering the case 2 and a single SDN controller... why should an IKE daemon be run in the controller?

> 
> I'm still a little confused about what is meant by "gateway-to-gateway"
> since I don't see any policies being able to be defined for "site to
> site" IPsec policies. Perhaps the context of gateway here does not mean
> "IPsec gateway" ? Although it then goes on to talk about VPN connections
> between two branch offices. Is this document only targeting that one
> host in one branch talks to another host in another branch using
> host-to-host and that the two subnets are connected by gateways that
> are not in scope for this document? If site-to-site is in scope, then
> the entries in Section 6.2 and 6.4 need updating to allow such policies
> (SPD) and states (SADs)

[Rafa] gateway-to-gateway refers to IPsec gateway : host——(IPsec gateway)— (IPsec gateway)- host as it happens in site to site.

In fact SPD model has a way to configure a tunnel between two gateways where the endpoints of the tunnel local and remote . 
container tunnel {
          when "../mode = 'TUNNEL'";
          uses tunnel-grouping;
          description "tunnel grouping container";
        }
grouping tunnel-grouping {
  description "Tunnel mode grouping";
    leaf local{ type inet:ip-address; description "Local tunnel endpoint"; }
    leaf remote{ type inet:ip-address; description "Remote tunnel enpoint"; }
    leaf bypass-df { type boolean; description "bypass DF bit"; }
    leaf bypass-dscp { type boolean; description "bypass DSCP"; }
    leaf dscp-mapping { type yang:hex-string; description "DSCP mapping"; }
    leaf ecn { type boolean; description "Bit ECN"; } /* RFC 4301 ASN1 notation. Annex C*/
  }

Also in the SAD. For example this is the xml file sent by NETCONF for the SPD

<spd-entry>
		<rule-number>10</rule-number>
		<priority>0</priority>
		<names>
			<name>in/10.1.0.2/10.0.0.2</name>
		</names>
		<condition>
			<traffic-selector-list>
				<ts-number>102</ts-number>
				<direction>INBOUND</direction>
				<local-addresses>
					<start>10.1.0.2</start>
 					<end>10.1.0.2</end>
				</local-addresses>
				<remote-addresses>
					<start>10.0.0.2</start>
					<end>10.0.0.2</end>
				</remote-addresses>
				<next-layer-protocol>TCP</next-layer-protocol>
				<local-ports>
					<start>0</start>
					<end>0</end>
				</local-ports>
				<remote-ports>
					<start>0</start>
					<end>0</end>
				</remote-ports>
			</traffic-selector-list>
		</condition>
		<processing-info>
			<action>PROTECT</action>
			<ipsec-sa-cfg>
				<security-protocol>esp</security-protocol>
				<mode>TUNNEL</mode>
        		<tunnel>
          			<local>192.168.0.2</local>
          			<remote>192.168.0.1</remote>
        		</tunnel>
			</ipsec-sa-cfg>
		</processing-info>
	</spd-entry>

Are we missing anything?

> 
> Section 8
> 
> I'm not sure if PF_KEYv2 should be recommended for new things. It seems
> an old interface that people have moved away from.

[Rafa] We were thinking in BSD systems. They seem to use PF_KEYv2 and extensions for SPD management. 

Many thanks again for your really valuable review.

> 
> Paul
> 
> _______________________________________________
> I2nsf mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/i2nsf

-------------------------------------------------------
Rafa Marin-Lopez, PhD
Dept. Information and Communications Engineering (DIIC)
Faculty of Computer Science-University of Murcia
30100 Murcia - Spain
Telf: +34868888501 Fax: +34868884151 e-mail: [email-masked]
-------------------------------------------------------