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

Make Redash FIPS Compliant by adding useforsecurity=False flag to md5 hashes #7049

Merged
merged 2 commits into from
Jul 11, 2024

Conversation

ezraodio1
Copy link
Contributor

What type of PR is this?

  • Refactor
  • Feature
  • Bug Fix
  • New Query Runner (Data Source)
  • New Alert Destination
  • Other

Description

Set usedforsecurity=False for md5 hashes. This is a requirement for FIPS compliance.

How is this tested?

  • Unit tests (pytest, jest)
  • E2E Tests (Cypress)
  • Manually
  • N/A

Related Tickets & Documents

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

@eradman
Copy link
Collaborator

eradman commented Jul 10, 2024

The useforsecurity flag was made available in Python 3.9. Everything works after enabling FIPS!

sudo apt-get -qq install crypto-policies
sudo fips-mode-setup --enable
sudo reboot
...
make up
make create_database

Tests also pass with FIPS enabled.

No adjustment is requried for PostgreSQL since scram-sha-256 was set as the default authentication method since 14.0

@justinclift
Copy link
Member

Interesting. Looks like the backend unit tests are failing, but I'm not real sure that's anything to do with this PR:

 45.88   • Installing xlsxwriter (1.2.2)
63.95 Warning: The file chosen for install of requests 2.32.0 (requests-2.32.0-py3-none-any.whl) is yanked.
Reason for being yanked: Yanked due to conflicts with CVE-2024-35195 mitigation

@eradman
Copy link
Collaborator

eradman commented Jul 10, 2024

Interesting. Looks like the backend unit tests are failing, but I'm not real sure that's anything to do with this PR:

 45.88   • Installing xlsxwriter (1.2.2)
63.95 Warning: The file chosen for install of requests 2.32.0 (requests-2.32.0-py3-none-any.whl) is yanked.
Reason for being yanked: Yanked due to conflicts with CVE-2024-35195 mitigation

Yeah, we need to update these dependencies since an upstream PIP module was yanked. After that's fixed we can rebase this change

@eradman eradman mentioned this pull request Jul 11, 2024
5 tasks
@justinclift
Copy link
Member

Looks like it'll probably pass now that the requests module update was merged. 😄

Copy link
Member

@justinclift justinclift left a comment

Choose a reason for hiding this comment

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

Looks good to me. 😄

@justinclift justinclift merged commit d9282b2 into getredash:master Jul 11, 2024
11 checks passed
@justinclift
Copy link
Member

Thanks for getting this done @ezraodio1, and taking care of the requests problem @eradman. 😄

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.

3 participants