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.

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

DNS improvements #407

Closed
wants to merge 5 commits into from
Closed

Conversation

cleberb
Copy link
Contributor

@cleberb cleberb commented Oct 4, 2023

  • Changed format of variables dns and fallback_dns from string to list
  • Ensure that changes to /etc/systemd/resolved.conf only occur if resolved is enabled
  • Ensure that changes to /etc/resolv.conf only occur if resolved or Network-Manager is disabled or Network-Manager is properly configured with the dns=none parameter

@cleberb cleberb requested a review from konstruktoid as a code owner October 4, 2023 14:47
@cleberb
Copy link
Contributor Author

cleberb commented Oct 4, 2023

TODO: Create variable/task network_manager_dns_mode. Which can guarantee fixed DNS definition on RedHat family systems.

@konstruktoid
Copy link
Owner

Thanks again!
I'll start testing when the todo is done.

Copy link
Owner

@konstruktoid konstruktoid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Waiting for TODO.

@cleberb
Copy link
Contributor Author

cleberb commented Oct 9, 2023

I'm not sure if the name of this variable (network_manager_dns_none) was the best choice. Recommendations are welcome.

@konstruktoid
Copy link
Owner

need some conflicts resolved

@cleberb
Copy link
Contributor Author

cleberb commented Oct 27, 2023

need some conflicts resolved

In this case, it wouldn't be just you who would be allowed to resolve:

image

@konstruktoid
Copy link
Owner

Sure, otherwise you could rebase it

- Changed format of variables `dns` and `fallback_dns` from string to list
- Ensure that changes to `/etc/systemd/resolved.conf` only occur if `resolved` is enabled
- Ensure that changes to `/etc/resolv.conf` only occur if `resolved` or `Network-Manager` is disabled or `Network-Manager` is properly configured with the `dns=none` parameter
@konstruktoid
Copy link
Owner

konstruktoid commented Oct 29, 2023

Things are not working correctly with the below diff, but it works better.
Please use molecule test to check any changes and variables, it does speed things up alot.

diff --git a/README.md b/README.md
index e6f45c1..1f928b7 100644
--- a/README.md
+++ b/README.md
@@ -213,16 +213,23 @@ dnssec: allow-downgrade
 fallback_dns:
   - 9.9.9.9
   - 1.0.0.1
-network_manager_dns_none: true
+network_manager_dns: false
 
 IPv4 and IPv6 addresses to use as system and fallback DNS servers.
 
-If `dnssec` is set to "allow-downgrade" DNSSEC validation is attempted, but if the server does not support DNSSEC properly, DNSSEC mode is automatically disabled.
+If `dnssec` is set to "allow-downgrade" DNSSEC validation is attempted, but if
+the server does not support DNSSEC properly, DNSSEC mode is automatically
+disabled.
 
-If `dns_over_tls` is `true`, all connections to the server will be encrypted if the DNS server supports DNS-over-TLS and has a valid certificate.
+If `dns_over_tls` is `true`, all connections to the server will be encrypted if
+the DNS server supports DNS-over-TLS and has a valid certificate.
 
-If `network_manager_dns_none` is `true`, the Network-Manager will be prevented from modifying the `/etc/resolv.conf` file, ensuring a fixed DNS configuration, even using a DHCP client. This configuration will only be applied if there are addresses defined in the `dns` variable. This option will be more relevant for systems in the "RedHat" family, which do not enable `resolved` by default.
+If `network_manager_dns` is `false`, the Network-Manager will be prevented from
+modifying the `/etc/resolv.conf` file, ensuring a fixed DNS configuration, even
+when using a DHCP client. This configuration will only be applied if there are
+addresses defined in the `dns` variable. This option will be more relevant for
+systems in the "RedHat" family, which do not enable `resolved` by default.
 
 [systemd](https://github.com/konstruktoid/hardening/blob/master/systemd.adoc#etcsystemdresolvedconf)
 option.
diff --git a/defaults/main/dns.yml b/defaults/main/dns.yml
index 5c1b64e..96e763a 100644
--- a/defaults/main/dns.yml
+++ b/defaults/main/dns.yml
@@ -7,4 +7,4 @@ dnssec: allow-downgrade
 fallback_dns:
   - 9.9.9.9
   - 1.0.0.1
-network_manager_dns_none: true
+network_manager_dns: false
diff --git a/handlers/main.yml b/handlers/main.yml
index f0444bf..1ab531f 100644
--- a/handlers/main.yml
+++ b/handlers/main.yml
@@ -169,7 +169,8 @@
   register: update_grub2
   changed_when: update_grub2.rc == 0
 
-- name: Restart Network-Manager
+- name: Restart NetworkManager
+  become: true
   ansible.builtin.systemd:
     name: NetworkManager
     state: restarted
diff --git a/molecule/default/molecule.yml b/molecule/default/molecule.yml
index 0fb7d9f..6cd786d 100644
--- a/molecule/default/molecule.yml
+++ b/molecule/default/molecule.yml
@@ -45,6 +45,7 @@ provisioner:
         block_blacklisted: true
         disable_wireless: true
         install_aide: false
+        network_manager_dns: true
         sshd_admin_net:
           - 0.0.0.0/0
         sshd_allow_groups:
diff --git a/molecule/default/verify.yml b/molecule/default/verify.yml
index 7c64e40..156d6f0 100644
--- a/molecule/default/verify.yml
+++ b/molecule/default/verify.yml
@@ -665,6 +665,11 @@
       loop_control:
         label: "{{ item.path }}"
 
+    - name: Stat systemd resolved.conf
+      ansible.builtin.stat:
+        path: /etc/systemd/resolved.conf
+      register: resolved_conf
+
     - name: Verify systemd resolved.conf
       become: true
       ansible.builtin.shell: grep "^{{ item }}$" /etc/systemd/resolved.conf
@@ -676,7 +681,7 @@
         - FallbackDNS={{ fallback_dns | join(' ') }}
         - DNSSEC={{ dnssec }}
         - DNSOverTLS={{ dns_over_tls }}
-      when: ansible_os_family != "RedHat"
+      when: resolved_conf.stat.exists
 
     - name: Verify resolv.conf
       become: true
@@ -685,8 +690,6 @@
       failed_when: resolv_conf.rc != 0
       changed_when: resolv_conf.rc != 0
       loop: "{{ ['nameserver '] | product(dns) | map('join') | list }}"
-      when:
-        - ansible_os_family == "RedHat"
 
     - name: Verify systemd timesyncd.conf
       become: true
diff --git a/tasks/dns.yml b/tasks/dns.yml
index 4af2598..479e718 100644
--- a/tasks/dns.yml
+++ b/tasks/dns.yml
@@ -1,5 +1,4 @@
 ---
-
 - name: Populate service facts
   become: true
   ansible.builtin.service_facts:
@@ -31,9 +30,6 @@
   when:
     - dns is defined and dns | length > 0
     - '"systemd-resolved.service" in services'
-    - services["systemd-resolved.service"]["status"] == "enabled"
-    - resolv_conf.stat.islnk is defined and resolv_conf.stat.islnk | bool
-    - resolv_conf.stat.lnk_source is defined and resolv_conf.stat.lnk_source == "/run/systemd/resolve/stub-resolv.conf"
   tags:
     - dns
     - resolved
@@ -49,7 +45,8 @@
     - dns
     - networkmanager
 
-- name: Ensure Network-Manager not modify /etc/resolv.conf
+- name: Ensure Network-Manager do not modify /etc/resolv.conf
+  become: true
   community.general.ini_file:
     path: /etc/NetworkManager/NetworkManager.conf
     section: main
@@ -58,11 +55,11 @@
     no_extra_spaces: true
     create: false
   notify:
-    - "Restart Network-Manager"
+    - "Restart NetworkManager"
   when:
     - dns is defined and dns | length > 0
     - networkmanager_conf.stat.exists is defined and networkmanager_conf.stat.exists | bool
-    - network_manager_dns_none is defined and network_manager_dns_none | bool
+    - not network_manager_dns | bool
   tags:
     - dns
     - networkmanager
@@ -83,6 +80,7 @@
       register: networkmanager_conf_content
       when:
         - '"NetworkManager.service" in services and services["NetworkManager.service"]["status"] not in ("disabled", "unknown")'
+        - networkmanager_conf.stat.exists
 
     - name: Read /etc/resolv.conf
       become: true

@cleberb
Copy link
Contributor Author

cleberb commented Oct 31, 2023

I believe this change has to be redone, the initial objective of the task is to activate systemd resolved. The change I made, which was complex, aimed to define fixed DNS, through systemd or network-manager. As soon as possible I will simplify the changes.

@konstruktoid
Copy link
Owner

Perhaps just start switching the variables to lists, similar to your previous PR?

@cleberb cleberb closed this Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants