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

[tirumal reddy <[e-mail]>]

On Tue, 31 Jan 2023 at 13:49, Valery Smyslov <[e-mail]> 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
> > 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?
>
> > --
> >
> > 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.
>

Yes, it means "digest of the certificate" but we go with SPKI hash. In both
cases, PKIX validation is not required.

-Tiru


>
> > --
> >
> > 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.
>
> 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.
>
> > --
> > [e-mail]
> >
> > _______________________________________________
> > IPsec mailing list
> > [e-mail]
> > https://www.ietf.org/mailman/listinfo/ipsec
>
>