Re: [IPsec] [I2nsf] Review of draft-ietf-i2nsf-sdn-ipsec-flow-protection-03

[Rafa Marin Lopez <[e-mail]>]

Hi Paul:

First of all, thank you very much for this impressive review. We are going to process all your comments as soon as possible by separating our answers in different e-mails so that the discussion is easier to follow.

In any case, we would like to answer first to your initial question and Yoav's

> El 18 nov 2018, a las 7:52, Paul Wouters <[e-mail]> escribió:
> 
> On Wed, 14 Nov 2018, Yoav Nir wrote:
> 
>> As discussed in the room, we need some reviewers for the sdn-ipsec-flow-protection draft ([1])
>> Thanks for these comments. Please see our response below.
>> While any comments on any part of the document are welcome, I would like people to concentrate on the following issues:
>> *  The YANG model in Appendix A
>>     +  Some of the crypto seems obsolete (example: DES). We would get into trouble in SecDir review.  OTOH ChaCha20-Poly1305 is missing..
> 
> Based on the introduction and abstract of the draft, this document does two things:
> 
> 1) Specify a yang model for use with SDWAN + IKE + IPsec
> 2) Define the desired modes and algorithms to use with 1)
> 
> It does not try to map the entire IKE/IPsec IANA registry into a yang model. Let me know if this is incorrect, because I use
> this as an assumption for the remainder of the review.

We must say that our I-D specifies 1) but being SDWAN one of the possible scenarios to operate so that the intent was to map the IKE/IPsec IANA registry. In any case we can change that approach if the WG consider is the right way to proceed.

> 
>> The TLS working group went quite far with TLS 1.3.  Only 2 ciphers remain: AES-GCM with 16-byte ICV, and ChaCha20-Poly1305. That’s it.  Specifically, they’ve deprecated everything
>> that isn’t an AEAD.
> 
> I think this can work for this SDWAN use case as well. While I don't think all systems
> are ready to support ChaCha20-Poly1305 for both IKE and ESP, I do believe all systems
> used for SDWAN should be able to do AES-GCM with 16-byte ICV for IKE and ESP. Although
> some IKE clients do not yet support AES-GCM for IKE (even while they support it for ESP)
> 
>> The IPsecME working group hasn’t gone that far yet.  But in practice pretty much nothing is used except 3DES, AES-CBC, and AES-GCM.  Perhaps ChaCha20-Poly1305 is starting to see
>> some use by now. We have RFC 8221, especially sections 5 and 6.  I think (although it’s up to the working group) that we should be fine defining only the MUSTs and the SHOULDs in
>> those sections.
> 
> Yes, with the exception of ENCR_NULL for encryption and AUTH_HMAC_SHA1_96 for integrity. (the latter is a MUST- only because of existing
> deployment, it is not recommended for new things)
> 
>> That brings another question. What is our plan for future expansions?  Suppose there’s some hot, new algorithm that everyone is implementing. How do you update the YANG model in
>> the future when you add new values to the enumerations?  Is it up to the administrator to make sure that the controller and NSFs are all on the “same page”?
> 
> I'm wondering about that too, since it is basically a snapshot of the
> IANA registry SHOULD/MUST entries which changes over time.

This is really a good question. Most probably one better way would be import another yang model that we could use. In fact, there is an related effort in

https://tools.ietf.org/html/draft-ietf-netconf-crypto-types-02

that we could use. In any case, we will follow your advise here.

> 
> 
> 
> General comments:
> I'd like to see "Case 1" and "Case 2" replaced with more descriptive terms. I keep losing track of which case is which.
> Perhaps call them IKE Case and IKE-less Case ?
> 
> 
> Section 1:
> 
>   Typically, traditional IPsec VPN concentrators and, in general,
>   entities (i.e. hosts or security gateways) supporting IKE/IPsec, must
>   be configured directly by the administrator.  This makes the IPsec
>   security association (SA) management difficult and generates a lack
>   of flexibility, specially if the number of security policies and SAs
>   to handle is high.
> 
> This is not entirely true. We have Opportunistic IPsec that automates
> this precisely for the same reasons. We also have packet triggers in
> combination with Traffic Selector narrowing to create multiple IPsec
> SA's on demand. I know this is all qualified by "typically" but I think
> this paragraph is not adding any real information. The idea is that we
> have an SDWAN situation with SDN controllers and nodes, and we want to
> automate the IKE/IPsec for that specific deployment use case.
> 
> 
>   The analysis of the host-to-gateway (roadwarrior) scenario is TBD.
> 
> Does this sentence mean this is to be done in this document or is it out
> of scope for this document (but it does allow it to be added later) ?
> 
> Section 3:
> 
>     It requires information about the
>     required authentication method (i.e. preshared keys), DH groups,
>     modes and algorithms for IKE SA negotiation, etc.
> 
> In the IKE world, we really try to not recommend preshared keys, because
> these keys mostly based on human readable low entropy content. If this
> document thinks raw RSA/ECDSA keys or X.509 certificates are also methods
> that will be implemented by SDN Controllers, please change the example of
> preshared keys to something else.
> 
> Section 5.1:
> 
> The diagram lists:
> 
> 	|   IKE    |      IPsec(SPD,PAD)  |
> 
> Shouldn't that be:
> 
>        | IKE(PAD) | IPsec(SPD, SAD)
> 
> IKE is using the PAD information to tell IPsec to establish SPDs and SADs) ?
> 
> 
> Section 5.1.1:
> 
> What does this mean:
> 
> 	Moreover, an east-west interface is required to exchange IPsec-related information.
> 
> Section 5.2:
> 
>   In this case, the NSF does not deploy IKEv2 and, therefore, the
>   Security Controller has to perform the management of IPsec SAs by
>   populating and monitoring the SPD and the SAD.
> 
> I would write:
> 
> In this case, the NSF does not deploy IKEv2 and, therefore, the
> Security Controller has to perform the IKE security functions and management
> of IPsec SAs by populating and managing the SPD and the SAD.
> 
> I added "IKE security functions" to make sure some entity is responsible
> for those items that are normally done by IKE (IV generation, prevent counter
> resets for same key, etc).
> 
> I changed monitoring to managing, because I don't understand how it would "monitor"
> the NSF or what it would monitor. I assume also if during monitoring it finds
> something that needs changing (eg vpn tunnel up or down) that it will do so,
> so I think managing is the better word.
> 
> For this diagram, I would say IPsec(SPD, SAD) would need to also list the IKE
> security function it needs to use in an IKE-less scenario, which above I had
> called "management of IPsec SAs"
> 
> Section 5.3:
> 
> As mentioned before, a better title would be: IKE vs IKEless
> 
> I would also start this section with:
> 
> Case 1 is more secure, because all security features of IKE are used to manage
> the IPsec SAs.
> 
> Section 5.3.1:
> 
>   For case 1, the rekeying process is carried out by IKEv2, following
>   the configuration defined in the SPD.
> 
> Technically this is the SPD and SAD, as the SPD would for example list the
> maximum number of packets allowed, but the SAD lists the number of packets
> that have happened.
> 
> I am a bit confused by the rekey description. I guess it assumes that the
> Security Controller sending of information is blocking? Eg when it tells
> the node to install an inbound IPsec SA, it is blocked from doing anything
> else. If this is not the case, then the 3 step program should be clarified
> that it needs to also receive confirmations. And if this is a non-blocking
> process, who is responsible for "retransmits" of these confirmations ?
> The same applies to the blocking or time-wait between step 2 and 3.
> 
> Also, I would say step 3 can be get rid of 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.
> 
> Section 5.3.2:
> 
> 	If one of the NSF restarts, it may lose part or all the IPsec state
> 
> This "may" suggests a very interesting case. I doubt the NSF would store on
> non-volatile memory the IPsec symmetric keys for encryption, and the counter
> or ICV's used. If it could store that, the document needs some text about
> this in the Security Considerations because it will allow for easier theft
> of session material.
> 
> Rather, I suspect this may really is not true, and all of these devices would
> lose state?
> 
> 	In both cases, the Security Controller MUST be aware of the affected NSF
> 
> I think this MUST is a little misleading. It is not desired behaviour but just
> an observation - it cannot not know its TCP session died ? Perhaps write:
> 
> 	In both cases, the Security Controller is aware of the affected NSF
> 
> A similar case for the next sentence MUST keyword:
> 
> 	Moreover, the Security Controller MUST have a register about all the
> 	NSFs that have IPsec SAs with the affected NSF.
> 
> so write:
> 
> 	Moreover, the Security Controller has a register about all the
> 	NSFs that have IPsec SAs with the affected NSF.
> 
> 
> 	(It is TBD in the model).
> 
> This remark seems to need to be resolved before publication ?
> 
>   In Case 2, if the Security Controller detects that a NSF has lost the
>   IPsec SAs (e.g. it reboots) it will follow similar steps to rekey:
>   the steps 1 and 2 remain equal but the step 3 will be slightly
>   different.  For example, if we assume that NSF B has lost its state,
>   the Security Controller MUST only delete the old IPsec SAs from A in
>   step 3.
> 
> I don't understand why this paragraph about NSF state loss seems to talk
> about "step 3" which can only be referring to the "rekeying process" of
> the previous section. It should only mention that it should configure new
> IPsec SAs between the failed node and all the nodes the failed node talked
> to and only after those are estlibhsed, it can send a delete for the old
> IPsec SAs on the non-failed nodes. This prevents the non-failed nodes from
> leaking plaintext.
> 
> Section 5.3.3:
> 
> I'm confused how the Security Controller can know what NAT ports to convey to
> the NSF agent. Without IKE traffic, there is no port NAT mapping? So unless
> the Security Controller _is_ the NAT gateway, it would never know this
> information? So I am skeptical if the NAT case can ever work without IKE.
> I also do not know how the Security Controller could decide between UDP,
> TCP or TLS without information from an IKE negotiation.
> 
> Section 6.1:
> 
> Note that the use of start and end addresses, means that this can never work
> with IKEv1, that can only negotiate CIDR networks. Perhaps this should be
> explicitely stated somewhere just in case some developer thinks they can
> use IKEv1 for this?
> 
> I dont understand the names in:
> 
> 	next-layer-protocol*   ipsec-next-layer-proto
> 
> It talks about ip-address and not ipsec-next-layer-ip-address? Simiarly it
> talks about local-ports. So why not just call this proto or protocol ?
> 
> Is selector-priority needed? Can the ts-number not be specified to be "lower
> numer is higher priority" ?
> 
> 
> Why do we have security-protocol? If you are considered about small footprints,
> clearly you would implement only ESP and use ESP-NULL if encryption is not
> needed. And simply not implement AH (which is already optional not mandatory
> to implement for IPsec). Also, this assumes IPCOMP, which has its own complexity,
> would also not be used. I think a case for AH would be very weak, while a case
> for IPCOMP could be made. I'd recommend only supporting ESP, at which point
> you would not need this parameter, or you could keep it for completeness
> sake even though it could only have one value "esp". This would also allow
> an updated ESP or some ESPlite to be used in the future if such a thing would
> be developed.
> 
> If we only go with AEAD algorithms, then the whole ah-algorithms part can be
> removed too. And esp-algorithms could be changed to only contain esp-encryption
> (or could be renamed aead-algorithm)
> 
> I see spd-mark but not spd-security-context  (see my Labeled IPsec draft)
> 
> I found the names in lifetimes confusing. "added" really means maxlife and
> "current" really means maxidle I think? So I think the names would be
> clearer as "time", "idle", "packets", "bytes". I think the time and idle
> units of uint64 could be narrowed down if the yang language allows some
> kind of timestamp type.
> 
> I am missing an entry for TFC (confidentiality, padding)
> I also see no entry for IPCOMP algorithm. But I am fine with not allowing
> IPCOMP at all :)
> 
> I am missing replay window parameters ?
> 
> I don't know what statefulfragCheck is ?
> 
> Section 6.2
> 
> See Section 6.1 comments.
> 
> Why are the encap entries in 6.2 and not 6.1 ?
> 
> What is the "state?" entry ?
> 
> Why is rpcs: only here?
> 
> I would not use minbits, maxbits. All modern ciphers, especially AEAD, are fixed key sizes. Make it a list of key sizes
> 
> I seem to miss the AEAD IV values ? Possibly the truncation values ?
> 
> Section 6.3:
> 
> I only see PSK and RSA algorithms? No ECDSA via RFC 7427 Digital Signatures?
> (no PAKE, we have two IKE PAKE's although it looks like we want an CFRG new one)
> 
> Should PSK be of type "string". Some people want to hex entry? Although those are often written
> as strings with a 0x prefix.
> 
> I see some CRL entries, but no OCSP entries. Is it expected those URI's are stored within the certificates,
> and the received OCSP responses will never be communicated through this interface?
> 
> I see no local and remote part, meaning that only symmetric authentication methods can be used. IKEv2 supports
> asymmetric authentication methods as well. Especially when using EAP and sometimes with AUTHHNULL, this method
> is very asymmetric. So I believe this entry needs to be split in a local/remote auth method.
> 
> Section 6.4:
> 
> type-autostartup issues...
> 
> typedef type-autostartup {
>      type enumeration {
>         enum ALWAYSON { description " ";}
>         enum INITIATE-ON-DEMAND {description " ";}
>         enum RESPOND-ONLY {description " ";}
>      }
>      description "Different types of how IKEv2 starts the IPsec SAs";
> 
> With libreswan we have similar keywords, and run into some interesting issues. We found what is really needed is to
> know the current state and the connection desired state. for example, when using ondemand and the connection is up,
> it is important to know it is up because of ondemand. So that when you receive a Delete request, you can go back
> to ondemand state. If you dont keep this state then you cannot tell the difference between alwayson/ondemand if
> the connection is "up". Similarly, we had issues where connections has a definition like above, but the administrator
> performs an action. It is unclear if such an action is considered a temporary override or basically a runtime update
> of the configuration. So lets say we are in respondonly, and the administrator brings the connection up. Now the other
> end sends a delete. Do you assume you are in alwayson state or in ondemand state? And if you believe alwayson, be
> prepared for a war with the remote endpoint that keeps sending deletes because it wants to be down.
> 
> 
> Why is there nat-traversal and espencap. Why not just have espencap allow to have a value of 'none' ?
> IKEv2 allows more then one oaddr  (eg multiple SOURCE NAT IPs) but the value is not a list?
> 
> The terms phase1-* should be called ike-* or ike-sa-* as phase1/phase2 is IKEv1 terminalogy not used for IKEv2.
> 
> Why is combined-enc-intr needed? These are stored with the encalg entries anyway.
> 
> I see:
> 
> 	+--rw local
>        |   +--rw (my-identifier-type)?
> 
> 	+--rw remote
>        |   +--rw (my-identifier-type)?
> 
> I think "my-identifier-type" should be "identifier-type". Talking about "my" for the remote peer is very confusing.
> 
> my-identifier-type does not support the value ID_NONE from RFC 7619
> 
> Why is there no secion for ike-lifetime-{soft|hard} ? We also have FIPS requirements that IKE SA's cannot do more
> then X bytes for Y algorithm. And also bringing down idle IKE SA's etc.
> 
> I think "added", "used", "bytes" "packets" should be called lifetime, idletime, maxbytes and maxpackets
> 
> The timestamp values "added" and "used" could have a better type than uint64, eg unix epoch of unixtime or something?
> 
> the term ike-stats was confusing me. I thought at first these were the global IKE statistics, like how many tunnels
> are up, how many errors received, but it turned out to be the state of a singe IKE SA. So perhaps ike-sa-state is a
> better term? IKE state I expect things like SPIi and SPIr as listed here.
> 
> I am not sure why there are three nat-* bools instead of one value representing the nat-state? Possibly because we
> use a number of bits to indicate which of the NAT properties are set, and you don't do this kind of bit mapping in
> yang?
> 
> I dont see an entry for "latest IKE SA". We have that in libreswan. You can have multiple IKE SA's when rekeying,
> and the older one is just lingering to die. Any operations such as informational requests, dpd/liveness probes
> should only be done with the "latest IKE SA".  However, this is kind of a state on the IKE node, so I am not
> sure if this belongs in the yang model or not.
> 
> I see a total SA's and halfopen SA's but I don't see anything related to anti-DDOS cookies. Eg the number of half
> open SA's before enabling cookies. Perhaps add half-open-cookies-treshhold ?
> 
> Section 7.1
> 
> Yes I do like this diagram style with less -+-+-+-+ better. Please use it everywhere in the document :)
> 
> 	In case 2, flow-based security policies defined by the administrator are also translated into IPsec SPD entries
> 
> I don't understand the use of "also" in this sentence. Since these are not translated into IKEv2 policies like case 1.
> (also reminder to use "IKE case" and "IKEless case" instead of "case 1" and "case 2")
> 
> 
> Question: since IPsec SA parameters installed in the kernel have lifetime values, what is installed here? Is it installed
> without any limits? If no limits what happens at max counter for AEAD? If there are limits, and the kernel generates
> ACQUIRES for userland, and no IKe is listening, what happens? How are things signaled? Does the ikeless node listen
> for netlink/pf_key messages to relay to the Security Controller?
> 
> Section 7.2:
> 
> Where does the terminology East/West come from? (and earlier South) ?
> 
>     3.  The Security Controller A realizes that protection is required
>       between the NSF1 and NSF2, but the NSF2 is under the control of
>       another Security Controller (Security Controller B), so it starts
>       negotiations with the other controller to agree on the IPsec SPD
>       policies and IKEv2 credentials for their respective NSFs.  NOTE:
>       This may require extensions in the East/West interface.
> 
> The negotiation protocol for this is not specified. I would think IKE can be used here? Is there a reason why this is
> not being defined here as using IKEv2? Is there another protocol ?  Especially since in the next case (with figure 6)
> it does state this protocol can be IKEv2 and states for both cases an IKE implementation was used.
> 
> 
> Section 8:
> 
> Is this section supposed to be an "Implementation Details" Section as per RFC 7942? If so, it is missing the required
> note to the RFC Editor to remove the entire section before publication as RFC.
> 
> section 9.1:
> 
> In case 1, add a note to use only strong PSKs, with a minimal length and strength.
> 
> Section 9.2:
> 
> 	when ESP is used
> 
> Hoping my advise is taken to only use ESP and not AH, and to use ESP-null in the case of encryption being unwanted, please
> remove this comment as ESP would always be used.
> 
> 	includes the keys for integrity and encryption
> 
> If we only allow AEAD's, maybe rewrite or leave tis out.
> 
> 
> Appendix A:
> 
> Why is it "eipsec" and not "ipsec" ?
> 
> Why is the organization not the IETF? Is this commonly done for yang models?
> 
> encryption-algorithm-t - I think this should only list the MUST/SOULD items, so AES_GCM (1 flavour), AES_CCM (1 flavour) CHACHAPOLY1305 and NULL
> 
> 	description "Encryption algorithms -&gt; RFC_5996";
> 
> RFC 5996 is an obsolete reference. It should either refer to RFC 7296, or it should simply refer to the IANA registry involved.
> 
> integrity-algorithm-t: If the above is agreed, this enumeration should only contain "none" and the entries from aes-cmac-96 and up. That is,
> not contain des,md5,sha1 and aes_xcbc.  Same obsolete reference is used here in the description.
> 
> type-autostartup: Maybe instead of RESPOND-ONLY, a better word is "STANDBY"? That way, the node could decide itself to bring a connection up
> without _requiring_ a remote signal, and could be triggered locally as as well as remotely to come up.
> 
>      description "Different types of how IKEv2 starts the IPsec SAs";
> 
> I would say "different policies of when to start an IKEv2 based IPsec SA". Does it need clarification this value is ignored or unused for the
> "case 2" scenario?
> 
> auth-protocol-type:  This should only contain IKEv2. The enum should remain in case we get IKEv2.1 or IKEv3 in the future.
> 
>      description "Peer authentication protocols";
> 
> I would say "IKE authentication protocol version", unless you want to keep the option open to have a non-IKE protocol. But I think that would
> run into many more issues of having parameters that are not defined for IKE?
> 
> ipsec-mode: It should only contain Tunnel and Transport Mode. It would be nice to see some text that forbids using Transport Mode when NAT is
> requested, just to avoid a lot of problems and disappointments. L2TP/IPsec still has major scars due to its use of Transport Mode with NAT.
> 
> esp-encap: as stated before, we are missing the port entry. I can also image that multiple encaps could be allowed, such as TCP-443 and TLS-443.
> Also note that the RFC for ESP over TCP recommends to continiously check if UDP encapsulation has become available, since it is so much prefered
> over TCP. But that only makes sense when the remote is a roaming client, and makes less sense if the network consists of data centers.
> 
> ipsec-protocol: I think this should only contain esp. See earlier comments. Notably, if supporting ipcomp here, the yang model must be clear
> that it could need an ipcomp + esp IPsec SA for a single 'connection'. Even if we only leave esp as a valid option, keep this enum in case
> we come up with a new esp version of some esp-lite version.
> 
> lifetime-action only has "terminate" and "rekey". For libreswan we use "clear", "hold" and "restart". The difference
> between clear and hold is what kernel policy to install for cleartext packets. For roadwarrionrs coming in, you
> want clear to leave no trace. for a site-to-site VPN you generally want "hold" to ensure you drop all packets until
> a new tunnel is established (possible ondemand, so this is different from restart)
> 
> ipsec-traffic-direction:
> 
> Traditionally, freeswan ony used inbound/outbound and not forward. Unfortunately, KAME and Linux/XFRM kept forward there. But I see no use
> case for it in these deployments. We have nothing specified in about ipsec-sa's for the forward (it's basically implied "completely open")
> But I understand it might be useful to just specify this because it is needed. But perhaps more language is then needed to clarify this?
> 
> ipsec-spd-operation:
> 
> The value BYPASS probably means "let go in the clear" but I don't find the name very obvious.
> The operation modes BYPASS and DISCARD cannot be configured with the current yang parameters
> for IKE? How would this be configured, eg to place a DISCARD for 192.1.2.0/24 ? In libreswan
> we can configure a connection with type=transport|tunnel|drop|reject but that meshes up the
> IPsec Mode with the SPD Operation. It is also a bit odd to configure "IKE" for a "DISCARD"
> since it really does not define anything in IKE - it bypasses IKE to put in an IPsec policy
> (SPD). But if the yang model used the ipsec parts to do this, it could be mixing IKE + manual
> IPsec policies, which could really confuse the IKE daemon. I am not sure what the best fix for
> this is. Note REJECT which is DISCARD+ICMP message is not defined here. Should it? I don't
> think Linux/XFRM/netlink supports this.
> 
> ipsec-next-layer-proto:
> 
> See earlier note. I think this is a really bad name.
> 
> ipsec-spd-nam:
> 
> This is the first time I have seen an SPD as having a name. normally these just have numbers. Or they
> have a number that is the link between SPD and SAD (with linux, that is called reqid=)
> So maybe add an enum id_number so implementors could map this onto the linux implementation ?
> 
> auth-method-type:
> 
> See earlier comment about asymmetric authentication.
> 
> This should only contain pre-shared, null, eap, rsa, rfc7427(digital signatures). That is I think dss-signature can be removed.
> I also wonder if this entry needs to have a better difference for the source of the private/public keypair, eg raw RSA versus
> RSA certificate based authentication. I guess this is now implied by not having a certificate entry?
> 
> Also rename description "Peer authentication method" to "IKE authentication method" (it should mention this is IKE, but also
> in case of asymmetric use, this is not neccessarilly for the peer but how we authenticate _to_ the peer)
> 
> sa-state:  I am not sure why this is needed? Unless someone would want to manually place larval/acquire states in the kernel
> without IKE, but then who is going to listen to these ACQUIRE messages? A non-IKE daemon? See earlier discussion on this topic.
> If you do want to get netlink/pf_key messages for max-counter, max-bytes etc without IKE, then this would be required. Otherwise
> this should be removed.
> 
> lifetime: See earlier comments.
> 
> grouping auth-method-grouping: See earlier comments about asymmetric authentication and raw vs cert based signatures. Note based
> on above entries, this is missing a dss-signature section. So if you agree with earlier comment to remove dss-signature, then
> it not appearing here is fine.
> 
> grouping identity-grouping:  See earlier comments on ID_NULL / AUTH_NULL support
> 
> grouping port-range: I don't see anywhere that protocol and ports together are "overloaded" for things like ICMP message types.
> Should language be added for that?
> 
> SAD and SPD grouping : See earlier note about Security Labels. It would need to be added here. Linux/XFRM already supports these.
> 
> I see container-ah and container-esp, but what about container-ipcomp? Again, if we remove IPCOMP as I'd like, then we are fine
> here. but if not, then something needs to be added here. If AH is removed as I'd recommended, it needs to be removed here.
> 
> 	container sad-lifetime-soft {
>         	description "SAD lifetime hard state data";
> 
> that "hard" should be "soft".
> 
> I am confused that "hard" and "soft" have an action that can be similar. soft could have an action to just notify userland via
> XFRM/PF_key, but "hard" can only have an option "terminate" ?
> 
> container encap: Now I see the sport / dport, but I'm still confused how this is populated from the higher level Security Controller
> communication (if not IKE). Note also the Security Controllers cannot know the port numbers without sending any traffic so the
> NAT gateway crates a mapping.
> 
> list traffic-selector-list: Can a traffic selector have a direction of fwd ? Not in the IKE protocol.
> 
>            leaf mode { type ipsec-mode; description "transport/tunnel"; }
> 
> This "description" does not match all the available options defined before.
> 
> container ah-algorithms: See AH comments before. Also see IPCOMP comments before.
> 
> 	description "Configure ESP encryption"
> 
> Maybe say "Configure ESP/AEAD encryption"
> 
> spd-mark:
> 
> Note that the IKE case cannot negotiate this spd-mark value. Is such a capability needed? It is similar to negotiating a Security Context.
> 
> 
> container spd-lifetime-*: See earlier comments on hard/soft and acceptable actions
> 
> grouping isakmp-proposal: I would call this ike-proposal as isakmp is kind of an IKEv1 term. See also phase1-* value suggestions earlier to
> make those ike-* or ike-sa-* values. This group is also missing pfs= which for IKEv2 only starts to matter when rekeying and whether or not
> to do a new DH.
> 
> grouping phase2-info: Call this ipsec-info: or ipsec-sa-info:
> 
> 	leaf-list pfs_group { type uint32;
> 
> This should really be an enum and not a uint32.
> 
> typedef sadb-msg-type: where/when are these used? That is, I understand the IKE <-> IPsec API uses this, but when does this ever need to
> go over the Security Controller's communication ?
> 
> typedef sadb-msg-satype: same here
> 
> container algorithm-supported: See earlier comments about all moderm algorithms not having a min/max but only having a limited set, for
> current ones this is only 128/192/256
> 
> Note the version of IKE within the ietf-ipsec namespace only has IKEv2. I agree with that now but earlier in the document it has IKEv1
> support/entries. So this needs to be made consistent here.
> 
> container ike-stats: does not contain encaps entry to show if it is using UDP, TCP or TLS and on which port.
> 
> list child-sas: does not contain some "state" that can be negotiated as part of an IKE session. For example, if narrowing was used, it could
> be that the current SPD is more narrow then the "connection" defined SPD. There seems to be no way to store this. Thus narrowing is not
> possible. If narrowing is not in scope here, it should probably be made more explicit. Otherwise, it needs to be better integrated.
> 
> container number-ike-sas: See earlier note about missing COOKIES parameter here.
> 
> 
> SPD related question: Is there support for multiple TSi/TSr generating a list of spd's in a single Child SA? If so, a Child SA can have
> multiple SPDs. Otherwise, it cannot. I dont think the model is very clear on this aspect. (I speak from experience, libreswan as an
> implementation is also not very clearly specified here, and we have one known interop issue with openbsd's iked because of this)
> 
> 
> // Those RPCs are needed by a Security Controller in case 2 */
> 
> Please think about XFRM/netlink and/or ACQUIRE messages that are needed if there is no IKE daemon. eg when max-counter is reached
> or when a hard limit action is performed by the kernel.
> 
> I am missing an "SPI already in use" error, in the case it is attempted to install an SPD that already exists and thus would conflict.
> 
> 
> 
> 
> 
> 
> NITS:
> 
> Introduction:
> 
> What does this mean:
> 
> 	"with a reduced intervention of the network administrator."
> 
> Section 1:
> 
> IPsec architecture -> The IPsec architecture
> 
> 
> Section 5.1:
> 
>   besides the IPsec support.
> 
> Change "besides" to "along with" ?
> 
>   With these entries, the IKEv2 implementation can operate to establish
>   the IPsec SAs.
> 
> Change to:
> 
>   With this information from the SDN Controller, the NSF can use its IKEv2
>   implementation to establish the IPsec SAs.
> 
> 
> Personally I don't like this Christmas like boxes:
> 
>                +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>                |IPsec Management/Orchestration Application | Client or
>                |          I2NSF Client                     | App Gateway
>                +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 
> and prefer:
> 
>                +-------------------------------------------+
>                |IPsec Management/Orchestration Application | Client or
>                |          I2NSF Client                     | App Gateway
>                +-------------------------------------------+
> 
> 
> Section 5.3:
> 
> 	Moreover hosts can install easily an IKE implementation.
> 	As downside, the NSF needs more resources to hold IKEv2.
> 	Moreover, the IKEv2 implementation needs to implement an
> 	interface so that the I2NSF Agent can interact with them.
> 
> Change to:
> 
> 	Usually, hosts can easily install an IKE implementation,
> 	although such an NSF needs more resources to run and it
> 	would need an interface for the communication between IKEv2
> 	and the I2NSF Agent.
> 
> Section 5.3.2:
> 
> 	It has also to send new
> 
> Change to:
> 
> 	It also has to send new
> 
> 
> 	Nevertheless other more optimized options can be considered (e.g.
> 	making iKEv2 configuration permanent between reboots).
> 
> Change to:
> 
> 	A more consistent and secure deployment would store the IKEv2 configuration
>        on non-volatile storage, so all information is available after a crash or
> 	reboot. IPsec SA information MUST NOT be stored on non-volatile storage to
> 	prevent incorrect and insecure re-use of symmetric key material.
> 
> Section 7.1:
> 
> 	Besides, fresh SAD entries will be also generated by the Security Controller
> 
> Change to:
> 
> 	Fresh SAD entries will also be generated by the Security Controller
> 
> What are "IaaS services" ?
> 
> section 9.2:
> 
> 	SHOULD NEVER stores the keys
> 
> change to:
> 
> 	SHOULD NEVER store the keys
> 
> 
> 	it may access to these values.
> 
> change to:
> 
> 	it may have access to these values.
> 
> 
> 	observe the traffic
> 
> change to:
> 
> 	observe traffic
> 
> 
> 	In any case, some escenarios
> 
> change to:
> 
> 	Some scenarios
> 
> 
> 	Moreover, some scenarios
> 
> change to:
> 
> 	Scenarios
> 
> Appendix:
> 
> I see a few "-&gt;" manglings.
> 
> _______________________________________________
> I2nsf mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/i2nsf