Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

WIP: Implement Dns record filtering #1

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

WIP: Implement Dns record filtering #1

wants to merge 4 commits into from

Conversation

LaakkonenJussi
Copy link
Contributor

@LaakkonenJussi LaakkonenJussi commented Jun 18, 2021 •
edited
Loading

A work-in-progress version of DNS record filtering. ipconfig notifications are sometimes lacking either of IP families so this does need some work still.

This addresses the issue of leaking data to IPv6 network when IPv4 VPN is connected using transport that supports both IP networks. This can happen because the DNS can return IPv6 (AAAA) records as well, and AAAA records are also requested if IPv6 is enabled on an online service. Since IPv6 is preferred over IPv4 the AAAA record is used and connection is made over IPv6 network, bypassing the default service in such case. By filtering out the DNS records based on the networks that the default service has in connected state this issue is prevented from happening.

An option is added to configuration to toggle whether the feature implemented here is enabled or not. By default the option FilterDNSRecords is on (true). TODO: this could be the other way around now as there are other means to prevent using IPv6 with VPNs.

This is a moved PR from https://git.sailfishos.org/mer-core/connman/merge_requests/307

@LaakkonenJussi
config: Add FilterDNSRecords option for DNS leak prevention
e4f666b 
FilterDNSRecords option determines whether to use DNS request/reply
filtering in dnsproxy.c. By default the value is true and filtering is
on to avoid DNS leaks when using IPv4 VPN over transport that also
supports IPv6. In systems where multiple networks are simultaneously
utilized this option should be explicitly set as false to disable the
filtering.
@LaakkonenJussi
resolver: Support options in resolvfile and prepending entries
0f214fa 
[resolver] Support options in resolvfile and prepending entries. JB#48769

Add support for enabling options in resolvfile with a simple toggle
function. This currently enables 'single-request' option, which is
intended to be used when only either of IP families is in use. Reason is
that with this option the requests are sequential instead of parallel
and will eliminate the default delay. However, the queries take twice
the time to complete when this option is set so it is not to be set when
both IP families are supported. Many glibc applications do both A and
AAAA queries in parallel.

Added also support for prepending a nameserver entry to the top of the
list. This allows to put IPv6 nameserver on top of IPv4.
@LaakkonenJussi
dnsproxy: Filter DNS records based on IP families enabled
385f2e3 
[dnsproxy] Filter DNS records based on IP families enabled. JB#48769

If a VPN is connected over a transport that supports both IPv4 and IPv6
and the VPN is IPv4 only, also AAAA DNS queries are sent, thus responses
may contain AAAA records if the DNS server supports them. In such case,
connections can be made over transport's IPv6 network bypassing VPN
connection.

The issue may get even worse if the SingleConnectedTech is not used and
there are multiple networks online at the same time. If network A and B
are both online and A supports IPv4 and IPv6, whereas B being the
default service supports only IPv4 and is used transport for a VPN, then
traffic is routed to A's IPv6 network if there is a AAAA record for
requested domain name.

Therefore, it is imperative to filter out the DNS requests and replies
that have no network available on the default service. This change makes
this behavior configurable, as in some setups multiple network types
and/or services are to be used in conjunction.

When the request type is filtered out the reponse is sent as
ns_r_noerror message containing the authoritative bit. No query is done
in this case.

If the DNS record is other than A or AAAA (e.g., CNAME), it will be
processed. To be able to detect the type of the DNS request/reply
cache_check() is changed to always set the type before returning in case
the type is something else than A or AAAA. This allows to filter out the
A/AAAA records when the necessary IP family is not connected on the
default service, and also allows to return a reply when the type is not
A/AAAA, e.g., CNAME.

Default service and ipconfig notifications are utilized to set correct
IP family types enabled on dnsproxy. When the IP family is disabled it
is also removed from resolvfile, and when only either of the IPv4 and
IPv6 is used the additional options are enabled as well to the
resolvfile. This is to minimize the delays caused by applications
sending both A and AAAA requests as the currently used 'single-request'
forces the requests to be sequential.

Also, replace all static 1/5/28 use for DNS record types with correct
types from arpa/nameser.h.
@LaakkonenJussi
dnsproxy: no toggle if filtering disabled.
e005625
@LaakkonenJussi LaakkonenJussi self-assigned this Jun 18, 2021
@LaakkonenJussi LaakkonenJussi added enhancement New feature or request wip Work in progress labels Jun 18, 2021
@LaakkonenJussi
Copy link
Contributor Author

Kind of forgot what is to be done with this one. It simply may be that in every situation this does not work and it heavily enforces the current default service's enabled ipconfig types on the filtering. But eventually, that is the effect of this feature..

Need to revisit this later on, and to have it as optional at least.

LaakkonenJussi referenced this pull request in LaakkonenJussi/connman Jun 10, 2024
@monich @LaakkonenJussi
device: Empty networks table before freeing device ident
1fd5e07 
Networks still need it when they are being deallocated:

  #0  connman_device_get_ident (src/device.c)
  #1  __connman_network_get_ident (src/network.c)
  sailfishos#2  connman_service_lookup_from_network (src/service.c)
  sailfishos#3  __connman_service_remove_from_network (src/service.c)
  sailfishos#4  network_remove (src/network.c)
  sailfishos#5  __connman_network_set_device (src/network.c)
  sailfishos#6  free_network (src/device.c)
  sailfishos#7  g_hash_table_remove_all_nodes (ghash.c)
  sailfishos#9  g_hash_table_remove_all (ghash.c)
  sailfishos#10 g_hash_table_destroy (ghash.c)
  sailfishos#11 device_destruct (src/device.c)

which results in use after free.

(cherry picked from commit 5e1daf5)
LaakkonenJussi referenced this pull request in LaakkonenJussi/connman Jul 4, 2024
@m-gardet @LaakkonenJussi
tethering: Fix use after free
26ad44c 
Methods of reproducing:

in loop
1) connmanctl tether wifi on my_ssid my_pasword
2) conencting client
3) connmanctl tether wifi off

con[14819.539062] tether: port 1(wlan0) entered disabled state
nmand2[3831]: ../git/src/technology.c:set_property() property Tethering
connmand2[3831]: ../git/plugins/wifi.c:tech_set_tethering()
connmand2[3831]: ../git/src/technology.c:connman_technology_tethering_notify() technology 0xb57006e0 enabled 0
connmand2[3831]: ../git/src/tethering.c:__connman_tethering_set_disabled() enabled 0
=================================================================
==3831==ERROR: AddressSanitizer: heap-use-after-free on address 0xb490c370 at pc 0x41c2e9c0 bp 0xbedf7494 sp 0xbedf7060
READ of size 2 at 0xb490c370 thread T0
    #0 0x41c2e9bf  (/usr/lib/libasan.so.5+0x41c2e9bf)

0xb490c370 is located 0 bytes inside of 18-byte region [0xb490c370,0xb490c382)
freed by thread T0 here:
    #0 0x41c73ee7 in free (/usr/lib/libasan.so.5+0x41c73ee7)
    #1 0x42877473  (/usr/lib/libglib-2.0.so.0+0x42877473)

previously allocated by thread T0 here:
    #0 0x41c7421b in malloc (/usr/lib/libasan.so.5+0x41c7421b)
    #1 0x42890b8b in g_malloc (/usr/lib/libglib-2.0.so.0+0x42890b8b)
    sailfishos#2 0x9d3a7 in sta_authorized ../git/plugins/wifi.c:3004
    sailfishos#3 0xa79eb in callback_sta_authorized ../git/gsupplicant/supplicant.c:626
    sailfishos#4 0xc3dd7 in signal_sta_authorized ../git/gsupplicant/supplicant.c:2779
    sailfishos#5 0xceb2f in g_supplicant_filter ../git/gsupplicant/supplicant.c:3620
    sailfishos#6 0x419fb123 in dbus_connection_dispatch (/usr/lib/libdbus-1.so.3+0x419fb123)
    sailfishos#7 0xb2501d17  (<unknown module>)

SUMMARY: AddressSanitizer: heap-use-after-free (/usr/lib/libasan.so.5+0x41c2e9bf)
Shadow bytes around the buggy address:
  0x36921810: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x36921820: fa fa fa fa fa fa fa fa fa fa fa fa fd fd fd fd
  0x36921830: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x36921840: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x36921850: fa fa fa fa fa fa 00 00 00 fa fa fa fa fa fa fa
=>0x36921860: fa fa 00 00 00 00 fa fa fa fa fa fa fa fa[fd]fd
  0x36921870: fd fa fa fa 00 00 00 00 fa fa fa fa fa fa fa fa
  0x36921880: fa fa fa fa fa fa 00 00 00 04 fa fa fa fa fa fa
  0x36921890: fa fa fa fa fa fa fa fa fd fd fd fd fa fa fa fa
  0x369218a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x369218b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==3831==ABORTING

(cherry picked from commit 799334d)
LaakkonenJussi referenced this pull request in LaakkonenJussi/connman Jul 4, 2024
@igaw @LaakkonenJussi
wireguard: Initialize data structure properly
839a6ec 
The info data structure should be properly initialized.

  #0  0x00007ffff7bd98f7 in __memmove_avx_unaligned () from /lib64/libc.so.6
  #1  0x00007ffff7fa4925 in mnl_attr_put () from /usr/lib64/libmnl.so.0
  sailfishos#2  0x00007ffff7fa496f in mnl_attr_put_check () from /usr/lib64/libmnl.so.0
  sailfishos#3  0x000000000040dd95 in wg_set_device (dev=0x480c70) at vpn/plugins/libwireguard.c:341
  sailfishos#4  0x0000000000410dd7 in wg_connect (provider=0x47e100, task=0x0, if_name=0x0, cb=0x0, dbus_sender=0x0,
      user_data=0x0) at vpn/plugins/wireguard.c:336
  sailfishos#5  0x0000000000413092 in vpn_connect (provider=0x47e100, cb=0x41e946 <connect_cb>,
      dbus_sender=0x481dd4 ":1.46", user_data=0x475a10) at vpn/plugins/vpn.c:632
  sailfishos#6  0x000000000041ec52 in __vpn_provider_connect (provider=0x47e100, msg=0x475a10) at vpn/vpn-provider.c:1206
  sailfishos#7  0x000000000041d488 in do_connect (conn=0x473ce0, msg=0x475a10, data=0x47e100) at vpn/vpn-provider.c:505
  sailfishos#8  0x000000000041d4da in do_connect2 (conn=0x473ce0, msg=0x475a10, data=0x47e100) at vpn/vpn-provider.c:515
  sailfishos#9  0x0000000000435691 in process_message (connection=0x473ce0, message=0x475a10,
      method=0x43bfe0 <connection_methods+160>, iface_user_data=0x47e100) at gdbus/object.c:259
  sailfishos#10 0x00000000004371a5 in generic_message (connection=0x473ce0, message=0x475a10, user_data=0x47f340)
      at gdbus/object.c:1071
  sailfishos#11 0x00007ffff7e4fc4d in ?? () from /usr/lib64/libdbus-1.so.3
  sailfishos#12 0x00007ffff7e407a4 in dbus_connection_dispatch () from /usr/lib64/libdbus-1.so.3
  sailfishos#13 0x000000000043306b in message_dispatch (data=0x473ce0) at gdbus/mainloop.c:72
  sailfishos#14 0x00007ffff7eca9f7 in ?? () from /usr/lib64/libglib-2.0.so.0
  sailfishos#15 0x00007ffff7ecdf88 in g_main_context_dispatch () from /usr/lib64/libglib-2.0.so.0
  sailfishos#16 0x00007ffff7ece310 in ?? () from /usr/lib64/libglib-2.0.so.0
  sailfishos#17 0x00007ffff7ece5e3 in g_main_loop_run () from /usr/lib64/libglib-2.0.so.0
  sailfishos#18 0x0000000000419f6f in main (argc=1, argv=0x7fffffffec28) at vpn/main.c:275

(cherry picked from commit c8ed849)
LaakkonenJussi pushed a commit that referenced this pull request Jul 18, 2024
@m-gardet @LaakkonenJussi
tethering: Fix use after free
72f5485 
Methods of reproducing:

in loop
1) connmanctl tether wifi on my_ssid my_pasword
2) conencting client
3) connmanctl tether wifi off

con[14819.539062] tether: port 1(wlan0) entered disabled state
nmand2[3831]: ../git/src/technology.c:set_property() property Tethering
connmand2[3831]: ../git/plugins/wifi.c:tech_set_tethering()
connmand2[3831]: ../git/src/technology.c:connman_technology_tethering_notify() technology 0xb57006e0 enabled 0
connmand2[3831]: ../git/src/tethering.c:__connman_tethering_set_disabled() enabled 0
=================================================================
==3831==ERROR: AddressSanitizer: heap-use-after-free on address 0xb490c370 at pc 0x41c2e9c0 bp 0xbedf7494 sp 0xbedf7060
READ of size 2 at 0xb490c370 thread T0
    #0 0x41c2e9bf  (/usr/lib/libasan.so.5+0x41c2e9bf)

0xb490c370 is located 0 bytes inside of 18-byte region [0xb490c370,0xb490c382)
freed by thread T0 here:
    #0 0x41c73ee7 in free (/usr/lib/libasan.so.5+0x41c73ee7)
    #1 0x42877473  (/usr/lib/libglib-2.0.so.0+0x42877473)

previously allocated by thread T0 here:
    #0 0x41c7421b in malloc (/usr/lib/libasan.so.5+0x41c7421b)
    #1 0x42890b8b in g_malloc (/usr/lib/libglib-2.0.so.0+0x42890b8b)
    #2 0x9d3a7 in sta_authorized ../git/plugins/wifi.c:3004
    #3 0xa79eb in callback_sta_authorized ../git/gsupplicant/supplicant.c:626
    #4 0xc3dd7 in signal_sta_authorized ../git/gsupplicant/supplicant.c:2779
    #5 0xceb2f in g_supplicant_filter ../git/gsupplicant/supplicant.c:3620
    #6 0x419fb123 in dbus_connection_dispatch (/usr/lib/libdbus-1.so.3+0x419fb123)
    #7 0xb2501d17  (<unknown module>)

SUMMARY: AddressSanitizer: heap-use-after-free (/usr/lib/libasan.so.5+0x41c2e9bf)
Shadow bytes around the buggy address:
  0x36921810: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x36921820: fa fa fa fa fa fa fa fa fa fa fa fa fd fd fd fd
  0x36921830: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x36921840: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x36921850: fa fa fa fa fa fa 00 00 00 fa fa fa fa fa fa fa
=>0x36921860: fa fa 00 00 00 00 fa fa fa fa fa fa fa fa[fd]fd
  0x36921870: fd fa fa fa 00 00 00 00 fa fa fa fa fa fa fa fa
  0x36921880: fa fa fa fa fa fa 00 00 00 04 fa fa fa fa fa fa
  0x36921890: fa fa fa fa fa fa fa fa fd fd fd fd fa fa fa fa
  0x369218a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x369218b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==3831==ABORTING

(cherry picked from commit 799334d)
LaakkonenJussi pushed a commit that referenced this pull request Jul 18, 2024
@igaw @LaakkonenJussi
wireguard: Initialize data structure properly
6de786a 
The info data structure should be properly initialized.

  #0  0x00007ffff7bd98f7 in __memmove_avx_unaligned () from /lib64/libc.so.6
  #1  0x00007ffff7fa4925 in mnl_attr_put () from /usr/lib64/libmnl.so.0
  #2  0x00007ffff7fa496f in mnl_attr_put_check () from /usr/lib64/libmnl.so.0
  #3  0x000000000040dd95 in wg_set_device (dev=0x480c70) at vpn/plugins/libwireguard.c:341
  #4  0x0000000000410dd7 in wg_connect (provider=0x47e100, task=0x0, if_name=0x0, cb=0x0, dbus_sender=0x0,
      user_data=0x0) at vpn/plugins/wireguard.c:336
  #5  0x0000000000413092 in vpn_connect (provider=0x47e100, cb=0x41e946 <connect_cb>,
      dbus_sender=0x481dd4 ":1.46", user_data=0x475a10) at vpn/plugins/vpn.c:632
  #6  0x000000000041ec52 in __vpn_provider_connect (provider=0x47e100, msg=0x475a10) at vpn/vpn-provider.c:1206
  #7  0x000000000041d488 in do_connect (conn=0x473ce0, msg=0x475a10, data=0x47e100) at vpn/vpn-provider.c:505
  #8  0x000000000041d4da in do_connect2 (conn=0x473ce0, msg=0x475a10, data=0x47e100) at vpn/vpn-provider.c:515
  #9  0x0000000000435691 in process_message (connection=0x473ce0, message=0x475a10,
      method=0x43bfe0 <connection_methods+160>, iface_user_data=0x47e100) at gdbus/object.c:259
  #10 0x00000000004371a5 in generic_message (connection=0x473ce0, message=0x475a10, user_data=0x47f340)
      at gdbus/object.c:1071
  #11 0x00007ffff7e4fc4d in ?? () from /usr/lib64/libdbus-1.so.3
  #12 0x00007ffff7e407a4 in dbus_connection_dispatch () from /usr/lib64/libdbus-1.so.3
  #13 0x000000000043306b in message_dispatch (data=0x473ce0) at gdbus/mainloop.c:72
  #14 0x00007ffff7eca9f7 in ?? () from /usr/lib64/libglib-2.0.so.0
  #15 0x00007ffff7ecdf88 in g_main_context_dispatch () from /usr/lib64/libglib-2.0.so.0
  #16 0x00007ffff7ece310 in ?? () from /usr/lib64/libglib-2.0.so.0
  #17 0x00007ffff7ece5e3 in g_main_loop_run () from /usr/lib64/libglib-2.0.so.0
  #18 0x0000000000419f6f in main (argc=1, argv=0x7fffffffec28) at vpn/main.c:275

(cherry picked from commit c8ed849)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@LaakkonenJussi LaakkonenJussi

Labels
enhancement New feature or request wip Work in progress
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@LaakkonenJussi