Hi all, Please see inline for additional comment to those already provided by Valery.
Cheers, Med > -----Message d'origine----- > De : Valery Smyslov <[email protected]> > Envoyé : mardi 31 janvier 2023 09:20 > À : 'Tero Kivinen' <[email protected]>; draft-ietf-ipsecme-add- > [email protected] > Cc : [email protected] > 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 > ~ > > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+- > +-+-+ > > > > 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... > > > 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? > [Med] That's normal for the request. No problem from my side to split the figure into two -for better clarity. > > -- > > > > In section 4 there is text: > > > > If the CFG_REPLY includes an ENCDNS_DIGEST_INFO attribute, > the DNS > > client has to create a digest of the DNS resolver certificate > > received in the TLS handshake using the negotiated hash > algorithm in > > the ENCDNS_DIGEST_INFO attribute. > > > > But this does not specify how the digest of the DNS resolver > > certificate is calculated. There are multiple ways of doing this > (only > > Subject Public Key Info element like RFC7296 or SPKI(1) selector > field > > of RFC7671, or full certificate like Cert(0) selector field of > > RFC7671). > > > > 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? > > In my reading "digest of the certificate" means the digest over > whole certificate (Cert(0)), but I'd rather hear from my co- > authors. > > > -- > > > > 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. [Med] Agree. It is normative because this is required to implement. > > Or did you mean [I-D.ietf-dnsop-svcb-https]? > > > -- > > > > IANA-IKE is bit misleading reference name when you are actually > > refering to the IKEv2 Configuration Payload Attribute Types. > Perhaps > > using IANA-IKE-HASH for the [Hash] above, and IANA-IKE-CFG for > this. > > OK. > > > -- > > > > It would actually be useful to have example configuration for > some of > > the deployment scenarios in Appendix A, similar than in 3.15.2 > of > > RFC7296. > > > > i.e., > > > > CP(CFG_REQUEST) = > > INTERNAL_IP6_ADDRESS() > > INTERNAL_IP6_DNS() > > ENCDNS_IP6() > > ENCDNS_DIGEST_INFO(0, "", (SHA2-256, SHA2-384, SHA2-512)) > > > > and getting reply > > > > CP(CFG_REPLY) = > > INTERNAL_IP6_ADDRESS(2001:DB8:0:1:2:3:4:5/64) > > ENCDNS_IP6(23, 1, 16, > > (2001:DB8:99:88:77:66:55:44), > > "doh1.example.com", > > "???") > > ENCDNS_DIGEST_INFO(0, "", SHA2-256, > > 00112233445566778899aabbccddeeff) > > > > where the data in ECNDNS_IP6 is in the same order they are in > the > > actual packet, i.e., 23 is the Service Priority, 1 is num > address, 16 > > is ADN Length and then is list of IPv6 addresses and so on. > > > > I for example have no idea what could be in the Service > Parameters > > field for some specific service (for example DoH), and how it > would be > > different for DoT... > > Adding an example makes sense, IMHO. > > Regards, > Valery. > > > -- > > [email protected] > > > > _______________________________________________ > > IPsec mailing list > > [email protected] > > https://www.ietf.org/mailman/listinfo/ipsec _________________________________________________________________________________________________________________________ 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 [email protected] https://www.ietf.org/mailman/listinfo/ipsec
