android: Explicitly apply DNS servers to the TUN device
authorTobias Brunner <tobias@strongswan.org>
Tue, 24 Nov 2020 15:50:55 +0000 (16:50 +0100)
committerTobias Brunner <tobias@strongswan.org>
Thu, 4 Feb 2021 15:52:15 +0000 (16:52 +0100)
If the peer deletes the CHILD_SA, we recreate it due to the close
action.  However, if we create a new TUN device, we do so with a new
VpnService.Builder object and on that the DNS servers were never applied.
The latter happened only on the fly in the attribute handler when an
IKE_SA was established.  Now we do this explicitly when creating the TUN
device, like the virtual IPs and routes.  While we could avoid the
recreation of the TUN device if the CHILD_SA is recreated, there is the
theoretical possibility that the remote traffic selectors change.  This
way we also avoid adding stuff to the builder in different places.

Fixes #3637.

src/frontends/android/app/src/main/jni/libandroidbridge/backend/android_attr.c
src/frontends/android/app/src/main/jni/libandroidbridge/backend/android_service.c

index be622d6..ffaf86a 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2012-2013 Tobias Brunner
+ * Copyright (C) 2012-2020 Tobias Brunner
  * Copyright (C) 2012 Giuliano Grassi
  * Copyright (C) 2012 Ralf Sager
  * HSR Hochschule fuer Technik Rapperswil
@@ -16,7 +16,6 @@
  */
 
 #include "android_attr.h"
-#include "../charonservice.h"
 
 #include <utils/debug.h>
 #include <library.h>
@@ -38,8 +37,8 @@ METHOD(attribute_handler_t, handle, bool,
        private_android_attr_t *this, ike_sa_t *ike_sa,
        configuration_attribute_type_t type, chunk_t data)
 {
-       vpnservice_builder_t *builder;
-       host_t *dns;
+       host_t *dns = NULL;
+       bool handled = FALSE;
 
        switch (type)
        {
@@ -50,19 +49,17 @@ METHOD(attribute_handler_t, handle, bool,
                        dns = host_create_from_chunk(AF_INET6, data, 0);
                        break;
                default:
-                       return FALSE;
+                       break;
        }
-
-       if (!dns || dns->is_anyaddr(dns))
+       if (dns && !dns->is_anyaddr(dns))
        {
-               DESTROY_IF(dns);
-               return FALSE;
+               DBG1(DBG_IKE, "installing DNS server %H", dns);
+               /* we don't actually handle them here, they are added to the TUN device
+                * explicitly when necessary, we still mark them as handled */
+               handled = TRUE;
        }
-       DBG1(DBG_IKE, "installing DNS server %H", dns);
-       builder = charonservice->get_vpnservice_builder(charonservice);
-       builder->add_dns(builder, dns);
-       dns->destroy(dns);
-       return TRUE;
+       DESTROY_IF(dns);
+       return handled;
 }
 
 METHOD(attribute_handler_t, release, void,
index c1c1e3a..9675cbb 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2010-2018 Tobias Brunner
+ * Copyright (C) 2010-2020 Tobias Brunner
  * Copyright (C) 2012 Giuliano Grassi
  * Copyright (C) 2012 Ralf Sager
  * HSR Hochschule fuer Technik Rapperswil
@@ -258,6 +258,41 @@ static bool add_routes(vpnservice_builder_t *builder, child_sa_t *child_sa)
 }
 
 /**
+ * Add DNS servers to the builder
+ */
+static bool add_dns_servers(vpnservice_builder_t *builder, ike_sa_t *ike_sa)
+{
+       enumerator_t *enumerator;
+       configuration_attribute_type_t type;
+       chunk_t data;
+       bool handled;
+       host_t *dns;
+
+       enumerator = ike_sa->create_attribute_enumerator(ike_sa);
+       while (enumerator->enumerate(enumerator, &type, &data, &handled))
+       {
+               switch (type)
+               {
+                       case INTERNAL_IP4_DNS:
+                               dns = host_create_from_chunk(AF_INET, data, 0);
+                               break;
+                       case INTERNAL_IP6_DNS:
+                               dns = host_create_from_chunk(AF_INET6, data, 0);
+                               break;
+                       default:
+                               continue;
+               }
+               if (dns && !dns->is_anyaddr(dns))
+               {
+                       builder->add_dns(builder, dns);
+               }
+               DESTROY_IF(dns);
+       }
+       enumerator->destroy(enumerator);
+       return TRUE;
+}
+
+/**
  * Setup a new TUN device for the supplied SAs, also queues a job that
  * reads packets from this device.
  * Additional information such as DNS servers are gathered in appropriate
@@ -297,7 +332,8 @@ static bool setup_tun_device(private_android_service_t *this,
                DBG1(DBG_DMN, "setting up TUN device failed, no virtual IP found");
                return FALSE;
        }
-       if (!add_routes(builder, child_sa) ||
+       if (!add_dns_servers(builder, ike_sa) ||
+               !add_routes(builder, child_sa) ||
                !builder->set_mtu(builder, this->mtu))
        {
                return FALSE;