Hi Paul:

Please see some additional comments inline:

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

[Authors] Then it is safer to remove this possibility. Done.

> 
>>        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.
[Authors] We understand. In the IKE-less case we do not have this kind of 
"configured but not up” state. We assume that once the SC configures the NSF 
with the IPsec SA, the NSF will apply the configuration right way without 
waiting anything else.

In IKE case it is possible to define this with the type-autostartup (on-demand) 
by adding your text to the description. However, I was thinking that, actually, 
this is a general security policy that should be applied in both iKE-case and 
IKE-less and, even, when the NSF starts in the network and there is no contact 
with the NSF.

Perhaps the best place to mention this is in the Security Considerations 
section. Then, we can add in the model the text you mention. We could include 
this paragraph in Security Considerations (the general part not specific to any 
case):

“For security reasons, a NSF MUST NOT allow any traffic unless the Security 
Controller mandates so. In other words, since the NSF needs IPsec information 
coming from the SC, if that information is not in place yet the safest option 
is DISCARD (drop) packets."



> 
>>      +--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 ipse-protocol-parameters ?

[Authors] Ok.

> 
>>        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] Ok
> 
>> [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.

[Authors] Ok.
> 
>> "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 ?

[Authors] Our approach so far will be similar to the one we found in RFC 8519. 
There they use a type uint8 and give a description and preference. More 
specifically:

leaf protocol {
      type uint8;
      description
        "Internet Protocol number.  Refers to the protocol of the
         payload.  In IPv6, this field is known as 'next-header',
         and if extension headers are present, the protocol is
         present in the 'upper-layer' header.";
      reference
        "
RFC 791
: Internet Protocol
         
RFC 8200
: Internet Protocol, Version 6 (IPv6) Specification.";
    }

Therefore, we would like to add something a uint8 and referring to IANA 
registry. Is this approach acceptable for you?

We will take also the same approach for crypto-algs so far in order to see if 
this is valid for yang-doctors' review


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

[Authors] Great. Here again, it is worth noting that we will specify the uint16 
and link it to the information you provide to that IANA registry.

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

[Authors] We have included this in the description:

"The flag indicating whether overflow of the sequence number counter should 
prevent
transmission of additional packets on the IPsec SA (FALSE), or whether rollover 
is permitted (TRUE). If Authenticated Encryption with Associated Data (AEAD) is 
used this flag MUST BE false.”;

> 
> 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] OK
> 
>> [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] No problem. We can leave it.
> 
>> [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] Correct. We think this can be handled by the implementation as well.

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

Indeed.

Thank you very much again!.
> 
> Thanks!
> 
> Paul
> 
> _______________________________________________
> IPsec mailing list
> [email protected]
> https://www.ietf.org/mailman/listinfo/ipsec

_______________________________________________
I2nsf mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/i2nsf

Reply via email to