freedomofpress / securedrop

GitHub repository for the SecureDrop whistleblower platform. Do not submit tips here!
https://securedrop.org/
Other
3.62k stars 686 forks source link

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

Open msheiny opened 6 years ago

msheiny commented 6 years ago

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

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.

eloquence commented 6 years ago

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.

legoktm commented 2 years ago

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.