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

[Valery Smyslov <[e-mail]>]

Hi Paul,

thank you for your comments, please see inline.

> Paul Wouters has entered the following ballot position for
> draft-ietf-ipsecme-add-ike-11: Discuss
> 
> ----------------------------------------------------------------------
> 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)

Actually, the format is the same for both request and response,
but depending on Num Hash Algs and AND Length and also on Length,
some fields may be omitted.

The most generic format is:

 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 Identifiers        ~
+---------------------------------------------------------------+
~                     Certificate Digest                        ~
+-------------------------------+-------------------------------+

Figures 2 and 3 just show how this attribute
looks when Num Hash Algs, AND Length and Length 
have specific values, which make sense for the request and response.

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

It was discussed in the ipsecme WG. The conclusion is that the "Update"
clause is only needed if a new specification changes the behavior of 
legacy implementations. In other words, if you need to fix old implementations
even if they don't support new specification - then you should mark
this new specification as "Update" for an old. This is not the case,
the specified behavior is only relevant to implementations supporting
encrypted DNS. If they don't - they follow RFC 8598.

This is a long time approach in ipsecme WG, otherwise every new
extension to IKEv2 would update RFC 7296  (which is not the case). 

> #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?

Perhaps, but I would like to hear from my co-authors.

> #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?

As far as I remember, the point is that modern browsers rely on encrypted
DNS resolvers, and currently there is no way to configure such rresolvers
in case DNS servers are supplied by VPN provider (via IKEv2).

My co-authors, who wrote this text, perhaps will further clarify its meaning.

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

I think my co-authors will clarify this.

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

Your comments look reasonable, but I'd like to hear from my co-authors.

Regards,
Valery.