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

convert_document_to_journalist_interface migration breaks Tor when restoring from a pre-0.4 backup #2484

Open
msheiny opened this issue Oct 26, 2017 · 2 comments
Labels
bug goals: packaging good first issue Hacktoberfest Issues suitable for the annual Hacktoberfest organized by Digital Ocean

Comments

@msheiny
Copy link
Contributor

msheiny commented Oct 26, 2017

This is a good first issue for new contributors to take on, if you have any questions, please ask on the task or in our Gitter room! See "Proposed fix" at the bottom of the description for what to do.

Bug

Description

During the backup procedure (see install_files/ansible-base/roles/backup/files/backup.py ) - we grab a couple of key files/folders from an old instance and then a restore script will lay these down upon the new server. In particular for this bug, we are grabbing /etc/tor/torrc/ and /var/lib/tor/services.

When an admin uses our backup against a pre 0.4 instance, restores to a post 0.4 instance, and then waits approximately 6 weeks for the next securedrop dpkg to be released, their instance will come back with a misconfigured tor setup for the journalist interface.

Steps to Reproduce

  • Take an existing running pre-0.4 SecureDrop instance and take a backup. Specifically, all you need is /etc/tor/torrc and a tar of everything under /var/lib/tor/services.
  • Install a post 0.4.x version of securedrop to another server
  • Restore the relevant tor files from the old SD to the new SD
  • Verify you can get to the journalist interface
  • Attempt to perform a force-install of securedrop again from the apt repos
  • Verify you can NOT get to the journalist interface

Expected Behavior

An apt upgrade doesn't break tor and updates the server-side code without incident.

Actual Behavior

A SecureDrop torrc that has been migrated from a pre-0.4 box looks something like this:

[...]
HiddenServiceDir /var/lib/tor/services/source
HiddenServicePort 80 127.0.0.1:80

HiddenServiceDir /var/lib/tor/services/document
HiddenServicePort 80 127.0.0.1:8080
HiddenServiceAuthorizeClient stealth journalist
[...]

and the listing of directories under /var/lib/tor/services looks like this:

/var/lib/tor/services/
├── journalist 
├── document
├── source
└── ssh

You might be asking above -- well why the hell is there a journalist AND a document folder?. Because journalist was created on a new ansible run, and then a restore process merely brought over the document folder but we didnt have any logic to handle any existing journalist folder here.

So, AFTER a package update from securedrop services the torrc looks like this:


[...]
HiddenServiceDir /var/lib/tor/services/source
HiddenServicePort 80 127.0.0.1:80

HiddenServiceDir /var/lib/tor/services/journalist
HiddenServicePort 80 127.0.0.1:8080
HiddenServiceAuthorizeClient stealth journalist
[...]

and the listing of directories under /var/lib/tor/services looks like this:

/var/lib/tor/services/
├── journalist 
│      └── document
├── source
└── ssh

Basically the journalist interface ends up reverting to what was originally installed from securedrop playbook before the restore process.

Comments

The problem here is that the migration logic that runs as part of the debian pkg preinst scripts (see install_files/securedrop-app-code/DEBIAN/preinst) do not take into account an existing journalist directory on a system and therefore end up breaking the configuration. The best thing to do would be to update that logic to perform an rsync -av --delete instead of a move. Note the problematic logic below:

function convert_document_to_journalist_interface() {
    # Helper function to migrate the old "Document Interface" config files
    # to the new "Journalist Interface" naming scheme. Affects tor and apache.

    if [[ -d /var/lib/tor/services/document ]]; then
        # Move tor service hostname and keys.
        mv /var/lib/tor/services/document /var/lib/tor/services/journalist
        # Update torrc to use new tor service hostname directory.
        perl -pi -e 's#^(HiddenServiceDir /var/lib/tor/services/)document#$1journalist#' /etc/tor/torrc
        # Bounce tor service.
        service tor restart
    fi

    if [[ -f /etc/apache2/sites-available/document.conf ]]; then
        # Stop apache prior to migrating vhost.
        service apache2 stop
        # Convert apache vhost settings to use the new interface name.
        perl -p -e 's/document/journalist/' /etc/apache2/sites-available/document.conf \
            > /etc/apache2/sites-available/journalist.conf
        rm -f /etc/apache2/sites-available/document.conf
        rm -f /etc/apache2/sites-enabled/document.conf
        # Enable new site; don't start apache, since `postinst` will start it.
        a2ensite journalist
    fi
}

Proposed fix

Remove the convert_document_to_journalist_interface function from https://github.com/freedomofpress/securedrop/blob/7e38090789a94c1398ac858585fd85ead238f396/install_files/securedrop-app-code/debian/preinst.

@msheiny msheiny added the bug label Oct 26, 2017
@ghost ghost added the goals: packaging label Dec 4, 2017
@eloquence eloquence added this to the 0.7 milestone Apr 5, 2018
@redshiftzero redshiftzero removed this from the 0.7 milestone Apr 23, 2018
@eloquence
Copy link
Member

So, as I understand it, we deferred this from the sprint and from 0.7 because it impacts only pre-0.4 instances. Frankly, I am not aware of a single pre-0.4 instance in the wild at this point, and running such an old version would be inadvisable for security reasons alone. So it's unclear to me whether any effort here is worthwhile at this point. Demoting to Long Term Backlog for now, but please let me know if there are compelling reasons to fix this at all at this point.

@eloquence eloquence added this to the Long Term Product Backlog milestone May 2, 2018
@legoktm
Copy link
Member

legoktm commented Sep 30, 2022

I think the main thing we should do to resolve this is remove the document -> journalist migration code https://github.com/freedomofpress/securedrop/blob/7e38090789a94c1398ac858585fd85ead238f396/install_files/securedrop-app-code/debian/preinst

This will hopefully cause anyone trying to restore a pre-0.4 instance today to fail very loudly rather than subtly breaking tor.

@legoktm legoktm changed the title deb pkg scripts break tor interfaces on SD instance prev. restored from backup convert_document_to_journalist_interface migration breaks Tor when restoring from a pre-0.4 backup Sep 30, 2022
@legoktm legoktm added good first issue Hacktoberfest Issues suitable for the annual Hacktoberfest organized by Digital Ocean labels Sep 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug goals: packaging good first issue Hacktoberfest Issues suitable for the annual Hacktoberfest organized by Digital Ocean
Projects
None yet
Development

No branches or pull requests

4 participants