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

[QA 1.7.0] Admin actions for user management trigger 500 #5703

Closed
conorsch opened this issue Jan 14, 2021 · 5 comments · Fixed by #5705
Closed

[QA 1.7.0] Admin actions for user management trigger 500 #5703

conorsch opened this issue Jan 14, 2021 · 5 comments · Fixed by #5705

Comments

@conorsch
Copy link
Contributor

Description

When attempting to edit an existing user account via the Journalist Interface, I see a 500 error.

Steps to Reproduce

  1. Perform clean install of 1.7.0~rc1
  2. Via sdconfig, add zh_Hans & zh_Hant to supported languages (unsure if this is necessary, but that's what I did)
  3. Log in as admin user, attempt to add a new user for via the Journalist Interface.
  4. Log in as admin user, attempt to edit user config for logged in user (e.g. to modify totp code)
  5. Observe 500.

Expected Behavior

Can add new user successfully, or can edit existing user account successfully.

Actual Behavior

Server throws a 500. I opened a terminal in the prod appvm and screenshotted /var/log/apache2/journalist-error.log

sd-1 7 0-rc1-qa-500

@emkll
Copy link
Contributor

emkll commented Jan 14, 2021

I can reproduce this error in an upgrade testing scenario, cron-apt upgrade from 1.6.0 to 1.7.0-rc1, (zh_Hans & zh_Hant not enabled)

@emkll emkll added this to the 1.7.0 milestone Jan 14, 2021
@emkll
Copy link
Contributor

emkll commented Jan 14, 2021

The error here likely due to apparmor rules in recently refactored code:

Jan 14 15:09:48 app-prod kernel: [ 1152.117568] audit: type=1400 audit(1610636988.284:30): apparmor="DENIED" operation="open" profile="/usr/sbin/apache2" name="/var/www/securedrop/passphrases.py" pid=11575 comm="apache2" requested_mask="r" denied_mask="r" fsuid=33 ouid=33

@emkll emkll self-assigned this Jan 14, 2021
emkll added a commit that referenced this issue Jan 14, 2021
emkll added a commit that referenced this issue Jan 14, 2021
Fixes #5703

(cherry picked from commit 8231109)
emkll added a commit that referenced this issue Jan 14, 2021
emkll added a commit that referenced this issue Jan 14, 2021
@conorsch
Copy link
Contributor Author

Reopening simply to track backport to release branch.

@conorsch conorsch reopened this Jan 14, 2021
@conorsch
Copy link
Contributor Author

Backport was already open in #5706, that's now reviewed and merged.

It's worth mentioning that the ./securedrop-admin verify checks inside Tails did catch the AppArmor violation, but only after I ran through the relevant test plan components. So we may want to move the verify step down, chronologically, in the test plan, to maximize what it can catch.

@kushaldas
Copy link
Contributor

I noticed the similar apparmor error on source name generation too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants