[IPsec] Paul Wouters' Discuss on draft-ietf-ipsecme-add-ike-11: (with DISCUSS and COMMENT)

[Paul Wouters via Datatracker <[e-mail]>]

Paul Wouters has entered the following ballot position for
draft-ietf-ipsecme-add-ike-11: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)


Please refer to https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/ 
for more information about how to handle DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-ipsecme-add-ike/



----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

I have a few important items I believe needs fixing, but I believe those are
still fairly easy to address.

#1 payload format of ENCDNS_DIGEST_INFO

I believe the proposed syntax for ENCDNS_DIGEST_INFO in this document
should not be specified this way. Depending on the use of this payload,
it has a different field construction. That is, we have two different
kinds of ENCDNS_DIGEST_INFO, which would make defining this field (eg in
C headers or in a class object) impossible without splitting it into two
different names and definitions. Either all the fields must be identical,
with optional 0 lengths field omitted, or the draft should define
ENCDNS_DIGEST_INFO_REQUEST and ENCDNS_DIGEST_INFO_RESPONSE with their different
field types. This can be further seen by the difficulty to read the examples
in the appendici with the ENCDNS_DIGEST_INFO() syntax.

If one ENCDNS_DIGEST_INFO type is used, I think the syntax for both request
and response should be:

 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             |
+-+-----------------------------+---------------+---------------+
| Num Hash Algs |  ADN Length   |                               |
+---------------+---------------+                               +
~                Authentication Domain Name                     ~
+-------------------------------+-------------------------------+
| Digest Hash Alg Identifier    |                               ~
+-------------------------------+                               +
~                     Certificate Digest                        ~
+-------------------------------+-------------------------------+

(eg as the current "response" version)

And Num Hash Algs, ADN Length and Digest Hash Alg Identifier
are mandatory fields in both the request and the response. I would
also always list these 3 fields in the presentation format of
ENCDNS_DIGEST_INFO() as used in the appendici examples.

I would rename "Hash Alg Identifier" to "Digest Hash Alg Identifier"
to make it more obvious that is what the hash algorithm is for.

#2 Updates RFC 8598

        Note: [RFC8598] requires INTERNAL_IP6_DNS (or INTERNAL_IP4_DNS)
        attribute to be mandatory present when INTERNAL_DNS_DOMAIN is
        included. This specification relaxes that constraint

This clearly updates RFC8598, but the document is lacking an Update: clause.
Please add the Update clause and mention the update in the
abstract/introduction.

#3 Security Considerations

        The initiator may trust the encrypted DNS resolvers supplied by
        means of IKEv2 from a trusted responder more than the locally
        provided DNS resolvers, especially in the case of connecting
        to unknown or untrusted networks (e.g., coffee shops or hotel
        networks).

This does not seem to be a "Security Consideration".
Also, before this draft, receiving an (unencrypted) DNS server supplied
by IKEv2 would also be more trusted. In general, VPN clients trust the
"VPN provided nameserver" more than the local network one, irrespective
of transport encryption. Perhaps this sentence can just be deleted?

#4 Appendix A.2 and A.3

        Legacy VPN service providers usually preserve end-users' data
        confidentiality by sending all communication traffic through an
        encrypted tunnel.

What is "legacy" about this? I do not understand the point that A.2 is
trying to make?

Similarly, I don't understand Appendix A.3. The VPN service is not involved
in "allowing" an application to send traffic through the tunnel. It is the
VPN client that decided whether or not to send its traffic through the tunnel
or not. Also VPNs typically are configured to be either split-tunnel or not.
This can be, be hardly ever is, dynamic. I don't understand what A.3 is trying
to convey as example use related to the encrypted dns capability of the
document.


----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Appendix A and B are marked as "example". This is confusing. I would rename
Appendix B to "Configuration Payload examples".

The Figure 5 text should add the line:

        * Its Service Priority is 1

Which explains one of the number "1"s in the Figure which is otherwise
unreferenced.

The placement of "AliasMode" confuses me. It appears as part of the
Service Priority as a field name, but it is not a mode or value of
that. Perhaps the text should be moved somewhat down, after the
listing od fields? (It is also somewhat confusing in the reference
document I-D.ietf-dnsop-svcb-https.


        Note that, for many years, typical designs have often considered
        that the DNS resolver was usually located inside the protected
        domain, but could be located outside of it. With encrypted DNS,
        the latter option becomes plausible.

Note that, VPN clients might have code that specifically prohibits the
use of DNS servers on IP addresses that are not covered by the VPN tunnel.

I also have some concern with the word plausible, as that seems to only take
encryption into account and not redirection or privacy concerns towards
the encrypted DNS server, or what could be monitored from an adversary
seeing the encrypted DNS stream not protected by the VPN to a DNS server.
(eg an adversay sees an encrypted DNS packet to 192.1.2.1, and then sees
a plaintext query to the root server for ohnoos.org from IP 192.1.2.1).
Note that based on this reasoning, perhaps a consideration for this should
be added to the Privacy Considerations section.

I would remove this note or rewrite it as a caution note instead, eg:

        Note that existing VPN client implementations might not expect
        the new use case of an obtained DNS server IP being outside of
        the covered IP ranges of the VPN tunnel.



This example does not make it clear if the encrypted DNS resolver can be
used for all DNS or not. It appears to say there is a limitation to only
use it for internal-only domain names. I do not think such a protocol
limitation should be implied by this example?

        Enterprise networks are susceptible to internal and external
        attacks. To minimize that risk all enterprise traffic is encrypted
        (Section 2.1 of [I-D.arkko-farrell-arch-model-t]).

I'm not sure if this sentence is relevant to the document in question? Or
actually, if all enterprise network is encrypted anyway, why cant one just
send "unencrypted DNS", encrypted over the VPN to the VPN server? The VPN
server would then encrypt that traffic to the target internal DNS server
using its regular "all enterprise traffic is encrypted" model. I suggest
to just remove this sentence.


        Hosting encrypted DNS resolvers even in case of split-VPN
        configuration minimizes the attack vector (e.g., a compromised
        network device cannot monitor/modify DNS traffic).

I think "minimizes" should be changed to "can minimize". For example, if the
encrypted DNS is hosted on the VPN server itself, the above claim is not true.


        In this example, the initiator uses the ENCDNS_DIGEST_INFO
        attribute to indicate that the encrypted DNS client supports
        SHA2-256 (2), SHA2-384 (3), and SHA2-512 (4) hash algorithms.

Can we add "for certificate digests" to this sentence. I was a bit confused
when I read this and saw support for hash algorithms and wondered what
the list of hash algorithms was for again.


        In this example, no ADN is included ...

Because this sentence is between two examples, it is really confusing as to which
example it belongs to. how about:

        In the following example, no ADN is included ...