Re: [IPsec] Shepherd review of the draft-ietf-ipsecme-add-ike

2023-01-31 Thread mohamed.boucadair
Re-,

Please see inline. 

Cheers,
Med

> -Message d'origine-
> De : Tero Kivinen 
> Envoyé : mardi 31 janvier 2023 15:33
> À : BOUCADAIR Mohamed INNOV/NET 
> Cc : Valery Smyslov ; draft-ietf-ipsecme-
> add-...@ietf.org; ipsec@ietf.org
> Objet : RE: [IPsec] Shepherd review of the draft-ietf-ipsecme-add-
> ike
> 
> mohamed.boucad...@orange.com writes:
> > [Med] Yes, the initiator may include a suggested ALPN (protocol)
> for
> > example to specifically indicate it is looking for DoT (or
> another
> > protocol). The initiator may omit the ADN, but only include
> service
> > parameters (typically, ALPN) to indicate a preferred transport
> > protocol.
> 
> I was assuming there is some way of indicating that, but I could
> not quickly find any examples of that, that is why I wanted to
> have more examples in this draft, so I could see what values the
> alpn can have
> :-)
> 

[Med] I hope this is now better with the new appendix in -07 ...unless you want 
to see more examples :-)


_

Ce message et ses pieces jointes peuvent contenir des informations 
confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce 
message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages 
electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou 
falsifie. Merci.

This message and its attachments may contain confidential or privileged 
information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete 
this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been 
modified, changed or falsified.
Thank you.

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


Re: [IPsec] Shepherd review of the draft-ietf-ipsecme-add-ike

2023-01-31 Thread Tero Kivinen
mohamed.boucad...@orange.com writes:
> [Med] Yes, the initiator may include a suggested ALPN (protocol) for
> example to specifically indicate it is looking for DoT (or another
> protocol). The initiator may omit the ADN, but only include service
> parameters (typically, ALPN) to indicate a preferred transport
> protocol.

I was assuming there is some way of indicating that, but I could not
quickly find any examples of that, that is why I wanted to have more
examples in this draft, so I could see what values the alpn can have
:-) 

> 
> > 
> >   CP(CFG_REQUEST) =
> >  ENCDNS_IP6(23, 1, 16,
> > (2001:DB8:99:88:77:66:55:44),
> > "doh1.example.com",
> > "???")
> > 
> > initiator requesting the responder for specific information, most
> > likely something that it got last time, and initiator proposes it
> > to the responder, in case it is still valid, and responder can
> > send that same information back if it is valid, or return
> > something else.
> 
> [Med] Yeah, but still this is just a suggested value and it is up to
> the responder to decide to honor it or not. If a preference does not
> make sense, the response will simply ignore it.

Yep, this is standard IKEv2 behavior. 

> > Btw, for the second use case where the initiator fills in some of
> > the information about the server it wants to talk to, it would be
> > usefull to allow Service Priority to be 0, and explictly mention
> > that this is not AliasMode, this is meaning that initiator does
> > not care about the exact priority or does not know the priority,
> > so it used 0 as placeholder.
> 
> [Med] The initiator can use any non-null value for the priority in
> such case. No need to relax the constraint imposed on svcpriority.

My guess is that people are going to use zero still, as that is the
obvious number to use, which is why I think it would be better to
allow that... As long as responders do not start checking that
CFG_REQUEST priority must be non zero everything is fine... 
-- 
kivi...@iki.fi

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


Re: [IPsec] Shepherd review of the draft-ietf-ipsecme-add-ike

2023-01-31 Thread mohamed.boucadair
Re-,

Please see inline. 

Cheers,
Med

> -Message d'origine-
> De : Tero Kivinen 
> Envoyé : mardi 31 janvier 2023 15:20
> À : BOUCADAIR Mohamed INNOV/NET 
> Cc : Valery Smyslov ; draft-ietf-ipsecme-
> add-...@ietf.org; ipsec@ietf.org
> Objet : RE: [IPsec] Shepherd review of the draft-ietf-ipsecme-add-
> ike
> 
> mohamed.boucad...@orange.com writes:
> > > of the cases the information in IANA registries are already in
> the
> > > normative reference RFCs
> >
> > RFCs may include stale/inaccurate values (e.g., new/deprecated
> > values). The IANA registry is authoritative.
> 
> Yes, but you only need one value to actually implement standard.

[Med] ... but this is broken if we want interop between distinct implements. 

> You do not need to know all currently supported values. I would
> assume that implementators will go to the IANA regardless whether
> ther reference is normative or informative.
> 
> > I still think maintaining the refs as they are is aligned with
> > https://www.ietf.org/about/groups/iesg/statements/normative-
> informative-references/.
> 
> Yes, most likely, but ID nits still complains about it.
> --

[Med] IMO, that's a false positive. We can report that as such in the writeup. 
Thanks.

Cheers,
Med


> kivi...@iki.fi

_

Ce message et ses pieces jointes peuvent contenir des informations 
confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce 
message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages 
electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou 
falsifie. Merci.

This message and its attachments may contain confidential or privileged 
information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete 
this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been 
modified, changed or falsified.
Thank you.

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


Re: [IPsec] Shepherd review of the draft-ietf-ipsecme-add-ike

2023-01-31 Thread Valery Smyslov
> > > Actually is there any point of having ADN Length and Authenticated
> > > Domain Name in CFG_REQUESTS ever? Why would someone calculate hashes
> > > with certain domain names with different hash algorithms? Perhaps we
> > > should define the format for CFG_REQUEST as follows:
> > >
> > >
> > > 1   2   3
> > > 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> > >+-+-+---+
> > >|R| Attribute Type  |Length |
> > >+---+---+
> > >~  List of Hash Algorithm Identifiers   ~
> > >+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> > >
> > >  Figure 3: ENCDNS_DIGEST_INFO Attribute Format for CFG_REQUEST
> >
> > I'm confused, since CFG_REQUEST doesn't include Digest.
> > Am I missing your point?
> 
> Thats the point. As the CFG_REQUEST does not include Certificate
> Digest, it only includes list of hash algorithms, there is no point of
> having Authentication Domain Name there either. So the ADN Length of
> CFG_REQUEST will always be zero, and Authentication Domain Name is
> omitted, but then we could simply omit the RESERVED and ADN Length
> fields also for more optimal encoding. I.e., if as we already have
> different format for CFG_REQUEST and CFG_REPLY/CFG_SET then making
> them even more different does not matter.
>
> If we want to keep the format same for all CFG_* then we need to
> format this payload as follows:
> 
> 
> 1   2   3
> 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>+-+-+---+
>|R| Attribute Type  |Length |
>+-+-+---+---+
>|RESERVED   | Num Hash Algs |  ADN Length   |
>+---+---+
>~  Authentication Domain Name   ~
>+---+
>~  Hash Algorithm Identifiers   ~
>+---+
>~ Certificate Digest~
>+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 
> 
> Where for CFG_REQUEST the Num Hash Algs tells how many hash algorithms
> there are in the Hash Algorithm Identifiers list, and ADN Length will
> be zero, and Authentication Domain Name and Certificate Digest are
> omitted.
> 
> For CFG_REPLY/CFG_SET the Num Hash Algs MUST be one, and Hash
> Algoritms Identifiers includes exactly one hash algorithm identifier
> which is used to calculate the Certificate Digest.
> 
> With this payload format the decoder for this configuration payload
> attribute does not need to know the type of the Configuration Payload
> CFG Type, it can decode and encode the payload without knowing that
> information, and the actual code that fills or uses them can then do
> checking whether they have suitable fields set to suitable values.
> 
> But either one of those ways is fine, I would assume you as an
> implementor has more preference which one is nicer than we others who
> most likely will not do implementation of this.

I think that the latter approach is better (and it is more consistent
with ENCDNS_IP* encoding). I also think it's a bit easier to implement.

> > > I would prefer the SPKI (Subject Public Key Info) selector field way
> > > of RFC7671, as then it does not matter if the certificate is renewed
> > > etc.
> >
> > Does certificate renewal matter in this case?
> 
> Not really, but for TLSA certificates I have been mostly using SPKI
> and when I need to renew certificate because it expired, I will simply
> reuse the private/public key and generate new certificate, and as I am
> using SPKI I do not need to update my dns zone when I do that. I would
> assume I am not alone in this situation, but I have no idea what will
> be the operational practices for these certificates.
> 
> Both full certificate and SPKI digest allow checking that key is
> correct, but full certificate digest might get broken in case there is
> different certificates in the VPN gateway and the DNS server (i.e.,
> DNS server hash renewed its certificate with same public key, but this
> has not yet been migrated to the configuration of the VPN gateway).

OK, as Tiru clarified, it is SPKI what is used.

> > > I do not think the [Hash] is normative reference. I did not need to
> > > read and understand that to somewhat understand this document :-)
> >
> > Well, it's only 58 words including title, you may read them in few
> > seconds :-) Kidding aside, how this can be informative? The document
> > 

Re: [IPsec] Shepherd review of the draft-ietf-ipsecme-add-ike

2023-01-31 Thread Tero Kivinen
mohamed.boucad...@orange.com writes:
> > of the cases the information in IANA registries are already in the
> > normative reference RFCs
> 
> RFCs may include stale/inaccurate values (e.g., new/deprecated
> values). The IANA registry is authoritative.

Yes, but you only need one value to actually implement standard. You
do not need to know all currently supported values. I would assume
that implementators will go to the IANA regardless whether ther
reference is normative or informative.

> I still think maintaining the refs as they are is aligned with
> https://www.ietf.org/about/groups/iesg/statements/normative-informative-references/.

Yes, most likely, but ID nits still complains about it.
-- 
kivi...@iki.fi

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


Re: [IPsec] Shepherd review of the draft-ietf-ipsecme-add-ike

2023-01-31 Thread mohamed.boucadair
Re-,

Please see inline. 

Cheers,
Med

> -Message d'origine-
> De : Tero Kivinen 
> Envoyé : mardi 31 janvier 2023 14:49
> À : BOUCADAIR Mohamed INNOV/NET 
> Cc : Valery Smyslov ; draft-ietf-ipsecme-
> add-...@ietf.org; ipsec@ietf.org
> Objet : RE: [IPsec] Shepherd review of the draft-ietf-ipsecme-add-
> ike
> 
> mohamed.boucad...@orange.com writes:
> > > > Also the text in Num Addresses indicate that it would be
> valid
> > > to send
> > > > CFG_REQUEST with proposed Service Priority, but having Num
> > > Addresses
> > > > set to zero?
> > > >
> > > > Is this intended? I.e., is the client allowed to request
> data,
> > > but not
> > > > propose IP address? If so, perhaps add sentence to Num
> Addresses
> > > > explaining that it can be 0 for CFG_REQUEST when no specific
> > > address
> > > > is request, but other parameters are requested.
> > >
> > > Hm... I think my co-authors can comment on this.
> > >
> >
> > [Med] This is intended. The requestor can suggest any of the
> > parameters in a request. That is already covered by the
> following:
> >
> > * " 0 if the Configuration payload has types CFG_REQUEST (if no
> > * specific DNS resolver is requested) ..."
> > * "If the initiator does not want to request specific DNS
> resolvers,
> > * it sets the Length field to 0 ..."
> > * "For a given attribute type, the initiator MAY send either an
> > * empty attribute or a list of distinct suggested resolvers."
> 
> This is different case.
> 
> I.e., there are (possibly) 3 different formats of CFG_REQUEST:
> 
>   CP(CFG_REQUEST) =
>  ENCDNS_IP6()
> 
> i.e., length = 0, initiator just request responder to send us
> informationfor ENCDNS_IP6.

[Med] Yes.

> 
>   CP(CFG_REQUEST) =
>  ENCDNS_IP6(Service Priority = 0, Num Addresses = 0,
>   ADN Length = 16, IP addresses = (),
>   Authentication Domain Name = "doh1.example.com",
>   Service Paramters = "???")
> 
> i.e., initiator requesting the responder to return information for
> Authentication Domain Name of doh1.example.com, and not providing
> priority or address of it, but perhaps including some service
> parameters it want the that server to have.

[Med] Yes, the initiator may include a suggested ALPN (protocol) for example to 
specifically indicate it is looking for DoT (or another protocol). The 
initiator may omit the ADN, but only include service parameters (typically, 
ALPN) to indicate a preferred transport protocol. 

> 
>   CP(CFG_REQUEST) =
>  ENCDNS_IP6(23, 1, 16,
> (2001:DB8:99:88:77:66:55:44),
>   "doh1.example.com",
>   "???")
> 
> initiator requesting the responder for specific information, most
> likely something that it got last time, and initiator proposes it
> to the responder, in case it is still valid, and responder can
> send that same information back if it is valid, or return
> something else.

[Med] Yeah, but still this is just a suggested value and it is up to the 
responder to decide to honor it or not. If a preference does not make sense, 
the response will simply ignore it. 

> 
> Btw, for the second use case where the initiator fills in some of
> the information about the server it wants to talk to, it would be
> usefull to allow Service Priority to be 0, and explictly mention
> that this is not AliasMode, this is meaning that initiator does
> not care about the exact priority or does not know the priority,
> so it used 0 as placeholder.

[Med] The initiator can use any non-null value for the priority in such case. 
No need to relax the constraint imposed on svcpriority.  

> 
> > > > In IP Address(es) explictly mention that it is field contain
> 4
> > > octet
> > > > IPv4 addresses, or 16 octet IPv6 address without any
> delimeters
> > > etc.
> > > > This can be derived from the calculation of the length
> field,
> > > but I
> > > > think it should be mentioned here, even when it is obvious.
> > >
> > > OK.
> >
> > [Med] Actually, no. We don't have a mix. The AF is determined by
> the
> > attribute type. This is why we have the following: "One or more
> IPv4
> > (for ENCDNS_IP4) or IPv6 (for ENCDNS_IP6)."
> 
> Yes, I know, but it does not say how those IP-addresses are
> encoded in that field. They could be sent out as 16-octet values
> for both IPv4 and IPv6 addresses, where the IPv4 address would
> only use last 4 octets

Re: [IPsec] Shepherd review of the draft-ietf-ipsecme-add-ike

2023-01-31 Thread mohamed.boucadair
Re-,

Not sure to follow this:

> of the cases the information in IANA registries are already in the
> normative reference RFCs

RFCs may include stale/inaccurate values (e.g., new/deprecated values). The 
IANA registry is authoritative.

I still think maintaining the refs as they are is aligned with 
https://www.ietf.org/about/groups/iesg/statements/normative-informative-references/.

Thanks. 

Cheers,
Med

> -Message d'origine-
> De : Tero Kivinen 
> Envoyé : mardi 31 janvier 2023 14:31
> À : Valery Smyslov 
> Cc : draft-ietf-ipsecme-add-...@ietf.org; ipsec@ietf.org
> Objet : RE: [IPsec] Shepherd review of the draft-ietf-ipsecme-add-
> ike
> 
...
> > > I do not think the [Hash] is normative reference. I did not
> need to
> > > read and understand that to somewhat understand this document
> :-)
> >
> > Well, it's only 58 words including title, you may read them in
> few
> > seconds :-) Kidding aside, how this can be informative? The
> document
> > uses these codepoints.
> 
> I usually have been considered things to normative if you need to
> read and understand to be able to implement the protocol. In most
> of the cases the information in IANA registries are already in the
> normative reference RFCs, thus having normative reference to one
> iana registry is not needed. The reason I pointed this out is
> because the ID nits complained about possible downref for having
> normative reference to non-rfc document...

_

Ce message et ses pieces jointes peuvent contenir des informations 
confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce 
message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages 
electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou 
falsifie. Merci.

This message and its attachments may contain confidential or privileged 
information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete 
this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been 
modified, changed or falsified.
Thank you.

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


Re: [IPsec] Shepherd review of the draft-ietf-ipsecme-add-ike

2023-01-31 Thread Tero Kivinen
mohamed.boucad...@orange.com writes:
> > > Also the text in Num Addresses indicate that it would be valid
> > to send
> > > CFG_REQUEST with proposed Service Priority, but having Num
> > Addresses
> > > set to zero?
> > >
> > > Is this intended? I.e., is the client allowed to request data,
> > but not
> > > propose IP address? If so, perhaps add sentence to Num Addresses
> > > explaining that it can be 0 for CFG_REQUEST when no specific
> > address
> > > is request, but other parameters are requested.
> > 
> > Hm... I think my co-authors can comment on this.
> > 
> 
> [Med] This is intended. The requestor can suggest any of the
> parameters in a request. That is already covered by the following:
> 
> * " 0 if the Configuration payload has types CFG_REQUEST (if no
> * specific DNS resolver is requested) ..." 
> * "If the initiator does not want to request specific DNS resolvers,
> * it sets the Length field to 0 ..." 
> * "For a given attribute type, the initiator MAY send either an
> * empty attribute or a list of distinct suggested resolvers." 

This is different case.

I.e., there are (possibly) 3 different formats of CFG_REQUEST:

  CP(CFG_REQUEST) =
 ENCDNS_IP6()

i.e., length = 0, initiator just request responder to send us
informationfor ENCDNS_IP6.

  CP(CFG_REQUEST) =
 ENCDNS_IP6(Service Priority = 0, Num Addresses = 0,
ADN Length = 16, IP addresses = (),
Authentication Domain Name = "doh1.example.com",
Service Paramters = "???")

i.e., initiator requesting the responder to return information for
Authentication Domain Name of doh1.example.com, and not providing
priority or address of it, but perhaps including some service
parameters it want the that server to have.

  CP(CFG_REQUEST) =
 ENCDNS_IP6(23, 1, 16,
(2001:DB8:99:88:77:66:55:44),
"doh1.example.com",
"???")

initiator requesting the responder for specific information, most
likely something that it got last time, and initiator proposes it to
the responder, in case it is still valid, and responder can send that
same information back if it is valid, or return something else.

Btw, for the second use case where the initiator fills in some of the
information about the server it wants to talk to, it would be usefull
to allow Service Priority to be 0, and explictly mention that this is
not AliasMode, this is meaning that initiator does not care about the
exact priority or does not know the priority, so it used 0 as
placeholder. 

> > > In IP Address(es) explictly mention that it is field contain 4
> > octet
> > > IPv4 addresses, or 16 octet IPv6 address without any delimeters
> > etc.
> > > This can be derived from the calculation of the length field,
> > but I
> > > think it should be mentioned here, even when it is obvious.
> > 
> > OK.
> 
> [Med] Actually, no. We don't have a mix. The AF is determined by the
> attribute type. This is why we have the following: "One or more IPv4
> (for ENCDNS_IP4) or IPv6 (for ENCDNS_IP6)."

Yes, I know, but it does not say how those IP-addresses are encoded in
that field. They could be sent out as 16-octet values for both IPv4
and IPv6 addresses, where the IPv4 address would only use last 4
octets :-) Only way to know that this is not true is to check the
formula in Length calculation...

It is obvious that they are encoded as 4 octets for each IPv4 address
and there is nothing between them, and same for IPv6 addresses, except
each address takes 16 octets, but I think it would be good to explain
it here.

Something like this:

   For ENCDNS_IP4 this field contains one or more 4 octet IPv4
   address, and for ENCDNS_IP6 this field contains one or more 16
   octet IPv6 address. Address in this field can be used to reach ...

-- 
kivi...@iki.fi

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


Re: [IPsec] Shepherd review of the draft-ietf-ipsecme-add-ike

2023-01-31 Thread Tero Kivinen
Valery Smyslov writes:
> > In section 3.2 it is not clear what the length of the Hash Algorithm
> > Identifiers fields is. It contains list of hash algorithms or one hash
> > algorithm if this is response, but it is not clear what is response.
> 
> What was meant is that a list of hashes is sent by a client (in
> CFG_REQUEST) and a single hash is sent by a server (in CFG_REPLY).

Yes, and I think it is easier to understand if we have two figures,
one for the CFG_REQUEST and another for CFG_REPLY/CFG_SET. 

> > We have CFG_REQUEST, CFG_REPLY, CFG_SET, and CFG_ACK. Most likely
> > CFG_REPLY and CFG_ACK are sent in IKE exchange response. On the other
> > hand CFG_SET is usually used to set the parameters, thus the
> > Certificate Digest would be required there.
> 
> True, but IKEv2 doesn't currently use CFG_SET/CFG_ACK and
> it explicitly allows implementations to ignore them.

True, but you do include some text in some places about the CFG_ACK
for example, so for completeness it would be good to define them in
same way than RFC7296 does.

I.e., CFG_SET is same as CFG_REPLY, and CFG_ACK is always empty (i.e.,
length = 0, and no other fields after that).

> > 1   2   3
> > 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> >+-+-+---+
> >|R| Attribute Type  |Length |
> >+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> > 
> >  Figure 4: ENCDNS_DIGEST_INFO Attribute Format for CFG_ACK.
> > 
> > and then explain the Hash Algorithm Identifier and List of Hash
> > Algorithm Identifiers separately.
> 
> We may do this for completeness, but as I've already mentioned
> CFG_SET/CFG_ACK are not currently used in IKEv2. So I'm in not sure
> if this is really needed and won't further confuse implementers...

I think first two ones are needed, the last one can be left out and
simply say in length definition that it is zero for CFG_ACK, just like
you do for ENCDNS_IP*.

> > Actually is there any point of having ADN Length and Authenticated
> > Domain Name in CFG_REQUESTS ever? Why would someone calculate hashes
> > with certain domain names with different hash algorithms? Perhaps we
> > should define the format for CFG_REQUEST as follows:
> > 
> > 
> > 1   2   3
> > 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> >+-+-+---+
> >|R| Attribute Type  |Length |
> >+---+---+
> >~  List of Hash Algorithm Identifiers   ~
> >+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> > 
> >  Figure 3: ENCDNS_DIGEST_INFO Attribute Format for CFG_REQUEST
> 
> I'm confused, since CFG_REQUEST doesn't include Digest.
> Am I missing your point?

Thats the point. As the CFG_REQUEST does not include Certificate
Digest, it only includes list of hash algorithms, there is no point of
having Authentication Domain Name there either. So the ADN Length of
CFG_REQUEST will always be zero, and Authentication Domain Name is
omitted, but then we could simply omit the RESERVED and ADN Length
fields also for more optimal encoding. I.e., if as we already have
different format for CFG_REQUEST and CFG_REPLY/CFG_SET then making
them even more different does not matter.

If we want to keep the format same for all CFG_* then we need to
format this payload as follows:


1   2   3
0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
   +-+-+---+
   |R| Attribute Type  |Length |
   +-+-+---+---+
   |RESERVED   | Num Hash Algs |  ADN Length   |
   +---+---+
   ~  Authentication Domain Name   ~
   +---+
   ~  Hash Algorithm Identifiers   ~
   +---+
   ~ Certificate Digest~
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+


Where for CFG_REQUEST the Num Hash Algs tells how many hash algorithms
there are in the Hash Algorithm Identifiers list, and ADN Length will
be zero, and Authentication Domain Name and Certificate Digest are
omitted.

For CFG_REPLY/CFG_SET the Num Hash Algs MUST be one, and Hash
Algoritms Identifiers includes exactly one hash algorithm identifier
which is used to calculate the Certificate Digest.

With this payload format the decoder for this 

Re: [IPsec] Shepherd review of the draft-ietf-ipsecme-add-ike

2023-01-31 Thread mohamed.boucadair
Hi all, 

Please see inline for additional comment to those already provided by Valery. 

Cheers,
Med

> -Message d'origine-
> De : Valery Smyslov 
> Envoyé : mardi 31 janvier 2023 09:20
> À : 'Tero Kivinen' ; draft-ietf-ipsecme-add-
> i...@ietf.org
> Cc : ipsec@ietf.org
> Objet : RE: [IPsec] Shepherd review of the draft-ietf-ipsecme-add-
> ike
> 
> Hi Tero,
> 
> thank you for the review. Please see inline.
> 
> > Here are some my review comments while reading
> > draft-ietf-ipsecme-add-ike:
> >
> > 
> --
> > The text in section 3.1 should say that if length is 0, then no
> > Service Priority, Num Addresses etc fields are present.
> >
> > I.e., add text in first bullet under Length saying that if
> length is
> > zero, then later fields are not present.
> 
> Makes sense.
> 
> > --
> >
> > Also the text in Num Addresses indicate that it would be valid
> to send
> > CFG_REQUEST with proposed Service Priority, but having Num
> Addresses
> > set to zero?
> >
> > Is this intended? I.e., is the client allowed to request data,
> but not
> > propose IP address? If so, perhaps add sentence to Num Addresses
> > explaining that it can be 0 for CFG_REQUEST when no specific
> address
> > is request, but other parameters are requested.
> 
> Hm... I think my co-authors can comment on this.
> 

[Med] This is intended. The requestor can suggest any of the parameters in a 
request. That is already covered by the following:

* " 0 if the Configuration payload has types CFG_REQUEST (if no specific DNS 
resolver is requested) ..."
* "If the initiator does not want to request specific DNS resolvers, it sets 
the Length field to 0 ..."
* "For a given attribute type, the initiator MAY send either an empty attribute 
or a list of distinct suggested resolvers."

> > --
> >
> > In IP Address(es) explictly mention that it is field contain 4
> octet
> > IPv4 addresses, or 16 octet IPv6 address without any delimeters
> etc.
> > This can be derived from the calculation of the length field,
> but I
> > think it should be mentioned here, even when it is obvious.
> 
> OK.

[Med] Actually, no. We don't have a mix. The AF is determined by the attribute 
type. This is why we have the following: "One or more IPv4 (for ENCDNS_IP4) or 
IPv6 (for ENCDNS_IP6)."

> 
> > --
> >
> > In section 3.2 it is not clear what the length of the Hash
> Algorithm
> > Identifiers fields is. It contains list of hash algorithms or
> one hash
> > algorithm if this is response, but it is not clear what is
> response.
> 
> What was meant is that a list of hashes is sent by a client (in
> CFG_REQUEST) and a single hash is sent by a server (in CFG_REPLY).
> 
> > We have CFG_REQUEST, CFG_REPLY, CFG_SET, and CFG_ACK. Most
> likely
> > CFG_REPLY and CFG_ACK are sent in IKE exchange response. On the
> other
> > hand CFG_SET is usually used to set the parameters, thus the
> > Certificate Digest would be required there.
> 
> True, but IKEv2 doesn't currently use CFG_SET/CFG_ACK and it
> explicitly allows implementations to ignore them.
> 
> > I would assume that there is only one Hash Algorithm Identifier
> for
> > CFG_REPLY and CFG_SET, and then the Certificate Digest field is
> > present. For CFG_REQUEST the Hash Algorithm Identifier is a list
> of
> > two octet hash algorithm identifiers and the Certificate field
> is
> > omitted. For the CFG_ACK only first 4 octets are included and
> Length
> > is set to zero.
> >
> > I think it would be better to split the Figure 2 to three
> different
> > figures:
> >
> >
> > 1   2
> 3
> > 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9
> 0 1
> >+-+-+
> ---+
> >|R| Attribute Type  |Length
> |
> >+-+-+---+
> ---+
> >|RESERVED   |  ADN Length
> |
> >+---+
> ---+
> >~  Authentication Domain Name
> ~
> >+
> ---+
> >~  Hash Algorithm Identifier (2 octets)
> ~
> >+
> ---+
> >~ Certificate Digest
> ~
> >+-+-+-+-+

Re: [IPsec] Shepherd review of the draft-ietf-ipsecme-add-ike

2023-01-31 Thread tirumal reddy
On Tue, 31 Jan 2023 at 13:49, Valery Smyslov  wrote:

> Hi Tero,
>
> thank you for the review. Please see inline.
>
> > Here are some my review comments while reading
> > draft-ietf-ipsecme-add-ike:
> >
> > --
> > The text in section 3.1 should say that if length is 0, then no
> > Service Priority, Num Addresses etc fields are present.
> >
> > I.e., add text in first bullet under Length saying that if length is
> > zero, then later fields are not present.
>
> Makes sense.
>
> > --
> >
> > Also the text in Num Addresses indicate that it would be valid to send
> > CFG_REQUEST with proposed Service Priority, but having Num Addresses
> > set to zero?
> >
> > Is this intended? I.e., is the client allowed to request data, but not
> > propose IP address? If so, perhaps add sentence to Num Addresses
> > explaining that it can be 0 for CFG_REQUEST when no specific address
> > is request, but other parameters are requested.
>
> Hm... I think my co-authors can comment on this.
>
> > --
> >
> > In IP Address(es) explictly mention that it is field contain 4 octet
> > IPv4 addresses, or 16 octet IPv6 address without any delimeters etc.
> > This can be derived from the calculation of the length field, but I
> > think it should be mentioned here, even when it is obvious.
>
> OK.
>
> > --
> >
> > In section 3.2 it is not clear what the length of the Hash Algorithm
> > Identifiers fields is. It contains list of hash algorithms or one hash
> > algorithm if this is response, but it is not clear what is response.
>
> What was meant is that a list of hashes is sent by a client (in
> CFG_REQUEST) and
> a single hash is sent by a server (in CFG_REPLY).
>
> > We have CFG_REQUEST, CFG_REPLY, CFG_SET, and CFG_ACK. Most likely
> > CFG_REPLY and CFG_ACK are sent in IKE exchange response. On the other
> > hand CFG_SET is usually used to set the parameters, thus the
> > Certificate Digest would be required there.
>
> True, but IKEv2 doesn't currently use CFG_SET/CFG_ACK and
> it explicitly allows implementations to ignore them.
>
> > I would assume that there is only one Hash Algorithm Identifier for
> > CFG_REPLY and CFG_SET, and then the Certificate Digest field is
> > present. For CFG_REQUEST the Hash Algorithm Identifier is a list of
> > two octet hash algorithm identifiers and the Certificate field is
> > omitted. For the CFG_ACK only first 4 octets are included and Length
> > is set to zero.
> >
> > I think it would be better to split the Figure 2 to three different
> > figures:
> >
> >
> > 1   2   3
> > 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> >+-+-+---+
> >|R| Attribute Type  |Length |
> >+-+-+---+---+
> >|RESERVED   |  ADN Length   |
> >+---+---+
> >~  Authentication Domain Name   ~
> >+---+
> >~  Hash Algorithm Identifier (2 octets) ~
> >+---+
> >~ Certificate Digest~
> >+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> >
> >   Figure 2: ENCDNS_DIGEST_INFO Attribute Format for CFG_REPLY and CFG_SET
> >
> >
> >
> > 1   2   3
> > 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> >+-+-+---+
> >|R| Attribute Type  |Length |
> >+-+-+---+---+
> >|RESERVED   |  ADN Length   |
> >+---+---+
> >~  Authentication Domain Name   ~
> >+---+
> >~  List of Hash Algorithm Identifiers   ~
> >+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> >
> >  Figure 3: ENCDNS_DIGEST_INFO Attribute Format for CFG_REQUEST
> >
> > 1   2   3
> > 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> >+-+-+---+
> >|R| Attribute Type  |Length |
> >+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> >
> >  Figure 4: ENCDNS_DIGEST_INFO Attribute Format for CFG_ACK.
> >
> > and then explain the Hash Algorithm Identifier and List of Hash
> 

Re: [IPsec] Shepherd review of the draft-ietf-ipsecme-add-ike

2023-01-31 Thread Valery Smyslov
Hi Tero,

thank you for the review. Please see inline.

> Here are some my review comments while reading
> draft-ietf-ipsecme-add-ike:
> 
> --
> The text in section 3.1 should say that if length is 0, then no
> Service Priority, Num Addresses etc fields are present.
> 
> I.e., add text in first bullet under Length saying that if length is
> zero, then later fields are not present.

Makes sense.

> --
> 
> Also the text in Num Addresses indicate that it would be valid to send
> CFG_REQUEST with proposed Service Priority, but having Num Addresses
> set to zero?
> 
> Is this intended? I.e., is the client allowed to request data, but not
> propose IP address? If so, perhaps add sentence to Num Addresses
> explaining that it can be 0 for CFG_REQUEST when no specific address
> is request, but other parameters are requested.

Hm... I think my co-authors can comment on this.

> --
> 
> In IP Address(es) explictly mention that it is field contain 4 octet
> IPv4 addresses, or 16 octet IPv6 address without any delimeters etc.
> This can be derived from the calculation of the length field, but I
> think it should be mentioned here, even when it is obvious.

OK.

> --
> 
> In section 3.2 it is not clear what the length of the Hash Algorithm
> Identifiers fields is. It contains list of hash algorithms or one hash
> algorithm if this is response, but it is not clear what is response.

What was meant is that a list of hashes is sent by a client (in CFG_REQUEST) and
a single hash is sent by a server (in CFG_REPLY).

> We have CFG_REQUEST, CFG_REPLY, CFG_SET, and CFG_ACK. Most likely
> CFG_REPLY and CFG_ACK are sent in IKE exchange response. On the other
> hand CFG_SET is usually used to set the parameters, thus the
> Certificate Digest would be required there.

True, but IKEv2 doesn't currently use CFG_SET/CFG_ACK and
it explicitly allows implementations to ignore them.

> I would assume that there is only one Hash Algorithm Identifier for
> CFG_REPLY and CFG_SET, and then the Certificate Digest field is
> present. For CFG_REQUEST the Hash Algorithm Identifier is a list of
> two octet hash algorithm identifiers and the Certificate field is
> omitted. For the CFG_ACK only first 4 octets are included and Length
> is set to zero.
> 
> I think it would be better to split the Figure 2 to three different
> figures:
> 
> 
> 1   2   3
> 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>+-+-+---+
>|R| Attribute Type  |Length |
>+-+-+---+---+
>|RESERVED   |  ADN Length   |
>+---+---+
>~  Authentication Domain Name   ~
>+---+
>~  Hash Algorithm Identifier (2 octets) ~
>+---+
>~ Certificate Digest~
>+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 
>   Figure 2: ENCDNS_DIGEST_INFO Attribute Format for CFG_REPLY and CFG_SET
> 
> 
> 
> 1   2   3
> 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>+-+-+---+
>|R| Attribute Type  |Length |
>+-+-+---+---+
>|RESERVED   |  ADN Length   |
>+---+---+
>~  Authentication Domain Name   ~
>+---+
>~  List of Hash Algorithm Identifiers   ~
>+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 
>  Figure 3: ENCDNS_DIGEST_INFO Attribute Format for CFG_REQUEST
> 
> 1   2   3
> 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>+-+-+---+
>|R| Attribute Type  |Length |
>+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 
>  Figure 4: ENCDNS_DIGEST_INFO Attribute Format for CFG_ACK.
> 
> and then explain the Hash Algorithm Identifier and List of Hash
> Algorithm Identifiers separately.

We may do this for completeness, but as I've already mentioned
CFG_SET/CFG_ACK are not currently used in IKEv2.
So I'm in not sure if this is really needed and won't further confuse 
implementers...

> 

[IPsec] Shepherd review of the draft-ietf-ipsecme-add-ike

2023-01-30 Thread Tero Kivinen
Here are some my review comments while reading
draft-ietf-ipsecme-add-ike:

--
The text in section 3.1 should say that if length is 0, then no
Service Priority, Num Addresses etc fields are present.

I.e., add text in first bullet under Length saying that if length is
zero, then later fields are not present.

--

Also the text in Num Addresses indicate that it would be valid to send
CFG_REQUEST with proposed Service Priority, but having Num Addresses
set to zero?

Is this intended? I.e., is the client allowed to request data, but not
propose IP address? If so, perhaps add sentence to Num Addresses
explaining that it can be 0 for CFG_REQUEST when no specific address
is request, but other parameters are requested.

--

In IP Address(es) explictly mention that it is field contain 4 octet
IPv4 addresses, or 16 octet IPv6 address without any delimeters etc.
This can be derived from the calculation of the length field, but I
think it should be mentioned here, even when it is obvious.

--

In section 3.2 it is not clear what the length of the Hash Algorithm
Identifiers fields is. It contains list of hash algorithms or one hash
algorithm if this is response, but it is not clear what is response.

We have CFG_REQUEST, CFG_REPLY, CFG_SET, and CFG_ACK. Most likely
CFG_REPLY and CFG_ACK are sent in IKE exchange response. On the other
hand CFG_SET is usually used to set the parameters, thus the
Certificate Digest would be required there.

I would assume that there is only one Hash Algorithm Identifier for
CFG_REPLY and CFG_SET, and then the Certificate Digest field is
present. For CFG_REQUEST the Hash Algorithm Identifier is a list of
two octet hash algorithm identifiers and the Certificate field is
omitted. For the CFG_ACK only first 4 octets are included and Length
is set to zero.

I think it would be better to split the Figure 2 to three different
figures:


1   2   3
0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
   +-+-+---+
   |R| Attribute Type  |Length |
   +-+-+---+---+
   |RESERVED   |  ADN Length   |
   +---+---+
   ~  Authentication Domain Name   ~
   +---+
   ~  Hash Algorithm Identifier (2 octets) ~
   +---+
   ~ Certificate Digest~
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

  Figure 2: ENCDNS_DIGEST_INFO Attribute Format for CFG_REPLY and CFG_SET



1   2   3
0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
   +-+-+---+
   |R| Attribute Type  |Length |
   +-+-+---+---+
   |RESERVED   |  ADN Length   |
   +---+---+
   ~  Authentication Domain Name   ~
   +---+
   ~  List of Hash Algorithm Identifiers   ~
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

 Figure 3: ENCDNS_DIGEST_INFO Attribute Format for CFG_REQUEST

1   2   3
0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
   +-+-+---+
   |R| Attribute Type  |Length |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

 Figure 4: ENCDNS_DIGEST_INFO Attribute Format for CFG_ACK.

and then explain the Hash Algorithm Identifier and List of Hash
Algorithm Identifiers separately.

Actually is there any point of having ADN Length and Authenticated
Domain Name in CFG_REQUESTS ever? Why would someone calculate hashes
with certain domain names with different hash algorithms? Perhaps we
should define the format for CFG_REQUEST as follows:


1   2   3
0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
   +-+-+---+
   |R| Attribute Type  |Length |
   +---+---+
   ~  List of Hash Algorithm Identifiers   ~
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+