Add admin_required to admin_add_user

[garrettr]

Access to the admin_add_user view should be restricted to administrators
only. Unfortunately, I accidentally omitted the @admin_required
decorator for this view. Since @admin_required encapsulates
@login_required, this means that anybody with access to the Document
Interface would be able to create a user accounts.

This is a fix for the issue reported on Bugtraq: http://seclists.org/bugtraq/2015/Apr/8

This is a security-high vulnerability and will be sent out in an immediate auto-update to securedrop-app-code by EOD today.

[garrettr]

Confirmed that this PR fixes the issue reported.

[garrettr]

This description of the vulnerability and its impact was sent in reply to the original email on Bugtraq. It appears to be awaiting moderation on that mailing list.

---

Hello, SecureDrop lead developer here. We are investigating this
report now. This appears to be a valid bug in the admin authentication
for the SecureDrop document interface. We are preparing a fix and will
be pushing it out ASAP.

However, this bug does not mean that production SecureDrop instances
are exploitable. It is a bug in the Document Interface, which is only
available as a "stealth" Authenticated Tor Hidden Service (ATHS) [0]
in production installations. This means that the Tor Hidden Service
cannot be accessed without a HidServAuth "cookie", which is a key used
to encrypt the descriptor that is uploaded to the directory servers.

[0] https://gitweb.torproject.org/torspec.git/tree/rend-spec.txt#n924

The HidServAuth cookie is provisioned by Tor during installation and
is carefully shared among the journalists who are given access to the
instance. FPF does not retain a copy of this cookie. This means that
the claims in the article that we can "backdoor" production instances
are false.

Additionally, even if an attacker were able to obtain the HidServAuth
cookie and leverage this vulnerability to create a user account, they
would still be unable to access the plaintext of submissions sent by
sources or replies sent by journalists. The submissions are encrypted
with a public key (GPG RSA-4096), and the corresponding private key is
kept in cold storage and only accessed on an airgap by authorized
users at each instance. The replies are encrypted with per-source
public keys, and the private keys can only be decrypted with the
source's unique codename (a strong Diceware passphrase).

The vulnerability in question was introduced as part of one of the
fixes for our last security audit (specifically iSEC-14FTC-003-10,
resolved in [1]). Since this code was written in response to the
audit, it was not subsequently audited itself prior to release. This
vulnerability was due to developer error and we are working to correct
it as quickly as possible. In addition, we are engaging a third party
to audit the changes between the last audit and the current release to
make sure no other bugs are present.

[1] #522

Sincerely,
Garrett Robinson
SecureDrop Lead Developer
Freedom of the Press Foundation

[garrettr]

• Cherry-pick this to develop once it has landed on master.

[PeteLawler]

Does kinda leave one wondering why commits as responses to audits aren't themselves audited, that's to say why it takes X period of time for it to happen. This can only lead one to believe that any fix, or additions, after an audit is questionable.

[Taipo]

Need more code reviewers I would think.

[garrettr]

@Taipo I would agree. One of the nice things about open source is that anybody can review our code at any time 😁

But I also think we should develop a mandatory code review policy to lessen the chance of this happening in the future. At the moment, I am the only person who reviews all of the code that gets merged, and I am the only person who reviews the vast majority of the code that I write. Unfortunately, I am a fallible human and I make mistakes like the one that has happened here.

[Taipo]

I am the only person who reviews all of the code that gets merged, and I am the only person who reviews the vast majority of the code that I write. Unfortunately, I am a fallible human and I make mistakes like the one that has happened here.

I figured this might be what is happening. Happy to help out again where I can.

[garrettr]

Now testing package auto-upgrade from 0.3.1 to 0.3.2.

[garrettr]

@Taipo Thanks :) We've also done a bad job engaging with the open source community in recent months, and that's something I'd like to improve going forward.

[garrettr]

The fix has been packages as 0.3.2 and pushed to the production update server. The newest code is on master, tagged 0.3.2.

[garrettr]

The fix has been merged back into develop in 7e7a062 for inclusion in future releases.