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

securedrop-admin --uninstall for staging and prod #489

Merged
merged 7 commits into from
Mar 11, 2020

Conversation

emkll
Copy link
Contributor

@emkll emkll commented Mar 5, 2020

Status

Ready for review

Description of Changes

Fixes #483

Testing

Dev environment

  • make all and make clean completes without error

Staging (or prod) environment

  • Build dom0 rpm on this branch
  • dom0 rpm installs correctly
  • Configure the instance (config.json and sd-journalist.sec) and run securedrop-admin --apply
  • securedrop-admin --uninstall completes without error
  • Files are absent from /srv/salt and dom0 packages are uninstalled

Checklist

If you have made code changes

  • Linter (make flake8) passes in the development environment (this box may
    be left unchecked, as flake8 also runs in CI)

emkll added 2 commits March 9, 2020 12:40
Will remove all files and packages in a staging or prod setting
config.json (containing instance configuration and Journalist Interface ATHS) and sd-journalist.sec (submission private key) will *not* be deleted, they should be found in two locations (unless they have been also copied elsewhere):
- /usr/share/securedrop-workstation/dom0-config/
- /srv/salt/sd/
This will ensure idempotency of delete operations when invoking `sd-clean-all` in staging and production scenarios.
@emkll emkll force-pushed the 483-clean-staging-and-prod branch from c66fd6b to bc9b0a7 Compare March 9, 2020 16:41
@conorsch
Copy link
Contributor

conorsch commented Mar 9, 2020

Ran through the dev-scenario test plan, looks good. While running make clean, I encountered #477, but that's unrelated to the diff in this PR.

@zenmonkeykstop
Copy link
Contributor

zenmonkeykstop commented Mar 10, 2020

Prod environment:

  • Build dom0 rpm on this branch
  • dom0 rpm installs correctly
  • Configure the instance (config.json and sd-journalist.sec) and run securedrop-admin --apply
    FAILS - sd-whonix can't start. complains about a possible DisableNetwork 1 line in config but the problem seems to actually be that /etc/torrc.d/95_whonix.conf is being included twice, pulling in the same HidServAuth definition from /rw/usrlocal/etc/torrc.d/50_user.conf - fixing this manually and continuing...
  • securedrop-admin --uninstall completes without error
  • Files are absent from /srv/salt and dom0 packages are uninstalled
    FAILS /srv/salt/sd still present containing config.json and sd-journalist.sec. (qubes-template-securedrop-workstation-buster-noarch package listed as available but not installed.)

@emkll
Copy link
Contributor Author

emkll commented Mar 10, 2020

Thanks @zenmonkeykstop for the review. Note that the presence of the config and the private key in /srv/salt/sd is expected behaviour (see https://github.com/freedomofpress/securedrop-workstation/pull/489/files#diff-332a99bbf5ab27f1effaf2cf03d411ceR115-R117) . However, it seems like since they will be in /usr/share/securedrop-workstation-config as well, and that the installer will re-copy them in place, it might make sense to delete those with this script. Looking into the Whonix issue you've described right now.

emkll added 2 commits March 10, 2020 08:28
Since it's automatically copied from /usr/share, we can delete the duplicate in /srv/salt/sd
Copy link
Contributor

@zenmonkeykstop zenmonkeykstop left a comment

Choose a reason for hiding this comment

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

I don't think this will fix it tbh, the duplicate comes in via /etc/tor/torrc, as there's an include there for both /etc/torrc.d/ and /etc/torrc.d/95_whonix.conf, which pulls in 95_whonix.conf twice and thus the user conf twice as well

@emkll
Copy link
Contributor Author

emkll commented Mar 10, 2020

@zenmonkeykstop I've pushed up two commits and this is now ready for re-review. I have tested it twice and it is working locally for me. I am using v2 onion services in my config. Are you using v3?

Here are my config files in sd-whonix (after running make clean / make all on b6b721a):

user@host:~$ cat /etc/tor/torrc
## Do not edit this file!
## Please create and add modifications to the following file instead:
## /usr/local/etc/torrc.d/50_user.conf

%include /etc/torrc.d/
user@host:~$ ls /etc/tor
tor/     torrc.d/ 
user@host:~$ ls /etc/torrc.d/
95_whonix.conf
user@host:~$ cat /etc/torrc.d/95_whonix.conf 
## Do not edit this file!
## Please create and add modifications to the following file instead:
## /usr/local/etc/torrc.d/50_user.conf

#### meta start
#### project Whonix
#### category tor and usability
#### gateway_only yes
#### description

%include /usr/local/etc/torrc.d/

#### meta end
user@host:~$ ls /usr/local/etc/torrc.d/
40_tor_control_panel.conf  50_user.conf  backup
user@host:~$ ls /usr/local/etc/torrc.d/
40_tor_control_panel.conf  50_user.conf  backup
user@host:~$ cat /usr/local/etc/torrc.d/50_user.conf 
# Tor user specific configuration file
#
# Add user modifications below this line:
############################################
### BEGIN securedrop-workstation ###
HidServAuth <something>.onion <hidservauth>
### END securedrop-workstation ###
user@host:~$ cat /usr/local/etc/torrc.d/40_tor_control_panel.conf 
# This file is generated by and should ONLY be used by anon-connection-wizard.
# User configuration should go to /usr/local/etc/torrc.d/50_user.conf, not here. Because:
#    1. This file can be easily overwritten by anon-connection-wizard.
#    2. Even a single character change in this file may cause error.
# However, deleting this file will be fine since a new plain file will be generated the next time you run anon-connection-wizard.
DisableNetwork 0

@zenmonkeykstop
Copy link
Contributor

Yeah I don't think this is related to your change, it could be a difference in the prod install vs make all. Rerunning it now to see if I can duplicate. If so, will run with your changes as well and see if that corrects. If not, it will be a separate prod-only issue.

@zenmonkeykstop
Copy link
Contributor

The torrc error is still there with the updated RPM, so I think it's safe to say it's unrelated.

@conorsch
Copy link
Contributor

Ran through the dev scenario test plan again, this time with v2 Onion Service config in config.json, based on discussion in #491. Good news is, the rc.local hotfix @emkll added appears to do the job.

See also https://github.com/freedomofpress/securedrop-workstation/tree/491-config-tests-for-whonix ; those new tests are all passing for me locally on this branch, but were failing on master with v2 onions. Consider squashing and pulling into this branch if anyone agrees.

kushaldas
kushaldas previously approved these changes Mar 11, 2020
Copy link
Contributor

@kushaldas kushaldas left a comment

Choose a reason for hiding this comment

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

Worked as expected. Approved from my side.

Dev environment

  • make all and make clean completes without error

Staging (or prod) environment

  • Build dom0 rpm on this branch
  • dom0 rpm installs correctly
  • Configure the instance (config.json and sd-journalist.sec) and run securedrop-admin --apply
  • securedrop-admin --uninstall completes without error
  • Files are absent from /srv/salt and dom0 packages are uninstalled

@kushaldas
Copy link
Contributor

See also https://github.com/freedomofpress/securedrop-workstation/tree/491-config-tests-for-whonix ; those new tests are all passing for me locally on this branch, but were failing on master with v2 onions. Consider squashing and pulling into this branch if anyone agrees.

@conorsch these look good to me, we can bring them here before merging.

Conor Schaefer added 2 commits March 11, 2020 09:52
If the torrc end state is broken on sd-whonix, we're sure to encounter it
in various failures, but having an explicit test is well worth it.
It's especially valuable to ask tor to perform the verification, given
the include-heavy config used by Whonix: there's no single torrc file to
inspect and reason about, but rather a chain of includes. Let's ask tor
to assemble and assert it's valid.
The specific problem of multiple includes causing recursion in imports
for the tor config broke v2 onion services for the workstation (and
whonix-gw overall). Let's monitor for regressions on that specific
config state.
@zenmonkeykstop
Copy link
Contributor

In prod, as per #492, Whonix Qubes updates fail after the dom0 rpm uninstalled, as a symlink /srv/salt/_tops/base/sd-workstation.top is not removed.

Run clean-salt as part of securedrop-admin uninstall action
@conorsch
Copy link
Contributor

Re-confirming that the dev scenario works well, with all tests passing, after the addition of the clean-salt script.

@emkll
Copy link
Contributor Author

emkll commented Mar 11, 2020

thanks @zenmonkeykstop , good catch on the topfile. I've just retested this on a clean install, the issue should now be fixed, note that there will be some warnings in the standard output of the securedrop-admin command due to files being first deleted by clean-salt, and then by the dom0 package uninstall

@zenmonkeykstop
Copy link
Contributor

zenmonkeykstop commented Mar 11, 2020

Prod install just failed with the following:

Traceback (most recent call last):
  File "/usr/bin/securedrop-admin", line 67, in provision_all
    subprocess.check_call([os.path.join(SCRIPTS_PATH, "scripts/provision-all")])
  File "/usr/lib64/python3.5/subprocess.py", line 271, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['/usr/share/securedrop-workstation-dom0-config/scripts/provision-all']' returned non-zero exit status 20

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/bin/securedrop-admin", line 151, in <module>
    main()
  File "/usr/bin/securedrop-admin", line 131, in main
    provision_all()
  File "/usr/bin/securedrop-admin", line 69, in provision_all
    raise SDAdminException("Error during provision-all")
__main__.SDAdminException: Error during provision-all

Rerunning it as this could well just be an issue with the state of Qubes (this is no longer a fresh install).

Copy link
Contributor

@zenmonkeykstop zenmonkeykstop left a comment

Choose a reason for hiding this comment

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

Did prod install:

  • Fresh install works (modulo another random qrexec timeout on sd-logs which is unrelated to this change`
  • uninstall works, including cleanup
  • whonix-* VMs can be upgraded after uninstall.

LGTM!

@zenmonkeykstop zenmonkeykstop merged commit be31ef7 into master Mar 11, 2020
@legoktm legoktm deleted the 483-clean-staging-and-prod branch May 28, 2024 15:25
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.

Make clean does not work in staging or prod environments
4 participants