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

[2.6.0-rc1] Application Server's SSH alias is configured incorrectly #6847

Closed
cfm opened this issue Jun 14, 2023 · 2 comments · Fixed by #6848
Closed

[2.6.0-rc1] Application Server's SSH alias is configured incorrectly #6847

cfm opened this issue Jun 14, 2023 · 2 comments · Fixed by #6848
Assignees

Comments

@cfm
Copy link
Member

cfm commented Jun 14, 2023

Description

With production VMs, where the Application Server's hostname is app-prod, ssh app-prod fails with Host unreachable, because its SSH alias is templated incorrectly:

amnesia@amnesia:~$ cat .ssh/config

Host app {'cmd': ['awk', '-v', 'FS=app_hostname: ', 'NF>1{print $2}', 'group_vars/all/site-specific'], 'stdout': 'app-prod', 'stderr': '', 'rc': 0, 'start': '2023-06-14 22:41:24.964450', 'end': '2023-06-14 22:41:24.966487', 'delta': '0:00:00.002037', 'changed': False, 'stdout_lines': ['app-prod'], 'stderr_lines': [], 'failed': False}
  User vagrant
  Hostname ul2koiedlodxd3ysr2q767wb5gc5wg5p4e77ctskqonozpln4hqjctqd.onion
  ProxyCommand /bin/nc -X 5 -x 127.0.0.1:9050 %h %p
  
Host mon mon-prod
  User vagrant
  Hostname bp337l47cwszbvcuouhqlfdmrok3uztc5wacgoel2qscvpbdvbkiktqd.onion
  ProxyCommand /bin/nc -X 5 -x 127.0.0.1:9050 %h %p

Steps to Reproduce

  1. Install SecureDrop 2.6.0-rc1 on production VMs via the usual securedrop-admin {sdconfig,install,tailsconfig} incantation.
  2. Run ssh app-prod, or select SSH into the App Server from the SecureDrop menu.

Expected Behavior

An SSH connection is established.

Actual Behavior

amnesia@amnesia:~$ ssh app-prod
nc: connection failed, SOCKSv5 error: Host unreachable
kex_exchange_identification: Connection closed by remote host
Connection closed by UNKNOWN port 65535

Via the SecureDrop menu, the terminal window opens, hangs, and closes.

Comments

I've checked that this is not an obstruction due to SSH host-key verification, which can be accepted normally in the terminal window opened by the SecureDrop menu.

@cfm cfm added this to the SecureDrop 2.6.0 milestone Jun 14, 2023
@cfm
Copy link
Member Author

cfm commented Jun 14, 2023

The incorrect ~/.ssh/config persists after a subsequent run of securedrop-admin tailsconfig.

@cfm
Copy link
Member Author

cfm commented Jun 14, 2023

The following patch allows securedrop-admin tailsconfig to correct ~/.ssh/config:

--- a/install_files/ansible-base/roles/tails-config/tasks/install_shell_extension.yml
+++ b/install_files/ansible-base/roles/tails-config/tasks/install_shell_extension.yml
@@ -1,4 +1,7 @@
 ---
+- name: Import variables
+  include_vars: "group_vars/all/site-specific"
+
 - name: Check for v3 Source Interface file
   stat:
     path: app-sourcev3-ths
@@ -26,18 +29,6 @@
   register: journalistv3_interface_lookup_result
   when: v3_source_file.stat.exists == true
 
-- name: Look up app server hostname
-  command: "awk -v FS='app_hostname: ' 'NF>1{print $2}' group_vars/all/site-specific"
-  changed_when: false
-  register: app_server_lookup_result
-  when: site_specific_file.stat.exists == true
-
-- name: Look up mon server hostname
-  command: "awk -v FS='monitor_hostname: ' 'NF>1{print $2}' /home/amnesia/Persistent/securedrop/install_files/ansible-base/group_vars/all/site-specific"
-  changed_when: false
-  register: mon_server_lookup_result
-  when: site_specific_file.stat.exists == true
-
 - name: Create the SecureDrop GNOME Shell Extension directories
   file:
     state: directory
@@ -87,14 +78,6 @@
   set_fact:
     journalist_iface: "{{ journalistv3_interface_lookup_result }}"
 
-- name: Set the right variable for app server hostname
-  set_fact:
-    app_hostname: "{{ app_server_lookup_result }}"
-
-- name: Set the right variable for app server hostname
-  set_fact:
-    mon_hostname: "{{ mon_server_lookup_result }}"
-
 - name: Assemble interface information for extension
   set_fact:
     _securedrop_extension_info:
@@ -102,8 +85,6 @@
         filename: extension.js
         source_interface_address: "{{ source_iface.stdout }}"
         journalist_interface_address: "{{ journalist_iface.stdout }}"
-        app_hostname: "{{ app_hostname.stdout }}"
-        mon_hostname: "{{ mon_hostname.stdout }}"
 
 - name: Create SecureDrop extension
   become: yes
--- a/install_files/ansible-base/roles/tails-config/templates/extension.js.in
+++ b/install_files/ansible-base/roles/tails-config/templates/extension.js.in
@@ -19,8 +19,8 @@ const Domain = Gettext.domain(GETTEXT_DOMAIN)
 
 const source_interface_address = "{{ item.0.source_interface_address }}";
 const journalist_interface_address = "{{ item.0.journalist_interface_address }}";
-const app_server_hostname = "{{ item.0.app_hostname }}";
-const mon_server_hostname = "{{ item.0.mon_hostname }}";
+const app_server_hostname = "{{ app_hostname }}";
+const mon_server_hostname = "{{ monitor_hostname }}";
 
 const _ = Domain.gettext;

What feels like a data race between the create_ssh_aliases.yml and install_shell_extension.yml tasks is really just a bit of cross-talk, in which the former refers directly to site-specific values in—

hostnames:
app: "{{ app_hostname }}"
mon: "{{ monitor_hostname }}"

—and then app_hostname, but not monitor_hostname, is overridden in the global namespace by:

- name: Assemble interface information for extension
set_fact:
_securedrop_extension_info:
- src: extension.js.in
filename: extension.js
source_interface_address: "{{ source_iface.stdout }}"
journalist_interface_address: "{{ journalist_iface.stdout }}"
app_hostname: "{{ app_hostname.stdout }}"
mon_hostname: "{{ mon_hostname.stdout }}"

The solution is to just not redefine the {app,monitor}_hostname facts.

Why didn't we catch this in testing #6712? I've just confirmed that my Admin Workstation on that branch did (does) have the Python-dictionary junk templated into ~/.ssh/config. But because my and our test servers had the more-usual app and mon hostnames, only those aliases had to be present and correct, and the trailing junk was silently ignored. If we had testinfra on the Tails side, I think this would be a good candidate for a test case.

@cfm cfm moved this to In Progress in SecureDrop dev cycle Jun 14, 2023
@cfm cfm moved this from In Progress to Ready For Review in SecureDrop dev cycle Jun 14, 2023
@cfm cfm self-assigned this Jun 14, 2023
@cfm cfm mentioned this issue Jun 15, 2023
21 tasks
@github-project-automation github-project-automation bot moved this from Ready For Review to Done in SecureDrop dev cycle Jun 15, 2023
@legoktm legoktm mentioned this issue Jun 20, 2023
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
1 participant