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

oauth2_proxy: Backport security fix (CVE-2017-1000070) #86108

Merged
merged 1 commit into from Apr 27, 2020

Conversation

knl
Copy link
Contributor

@knl knl commented Apr 27, 2020

Motivation for this change

Since 20.03 still uses old oauth2_proxy (3.2.0), which is not compatible
with the newest one (5.1.0), this change backports an important security
fix to 3.2.0:

oauth2-proxy/oauth2-proxy@a316f8a

The vulnerability is an open redirect, where a bad actor can redirect a
session to another domain using /\ in redirect URIs.

The patch has been obtained by running:

git format-patch a316f8a06f3c0ca2b5fc5fa18a91781b313607b2\^..a316f8a06f3c0ca2b5fc5fa18a91781b313607b2

on https://github.com/pusher/oauth2_proxy repository clone.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@knl
Copy link
Contributor Author

knl commented Apr 27, 2020

This PR is the result of the discussion in #79840

@flokli @arianvp please take a look

@arianvp
Copy link
Member

arianvp commented Apr 27, 2020

Lgtm

@arianvp
Copy link
Member

arianvp commented Apr 27, 2020 via email

@Mic92
Copy link
Member

Mic92 commented Apr 27, 2020

Is it possible to use https://github.com/oauth2-proxy/oauth2-proxy/commit/a316f8a06f3c0ca2b5fc5fa18a91781b313607b2.patch with fetchpatch? You can use
pass excludes = [ "CHANGELOG.md" ]; to avoid potential diff conflicts.

@knl knl force-pushed the security-fix-oauth2_proxy branch from df66b71 to cf08beb Compare April 27, 2020 08:50
@knl knl changed the title oauth2_proxy: Backport security fix oauth2_proxy: Backport security fix (CVE-2017-1000070) Apr 27, 2020
@knl knl force-pushed the security-fix-oauth2_proxy branch from cf08beb to b6c591c Compare April 27, 2020 08:59
@knl
Copy link
Contributor Author

knl commented Apr 27, 2020

Is it possible to use https://github.com/oauth2-proxy/oauth2-proxy/commit/a316f8a06f3c0ca2b5fc5fa18a91781b313607b2.patch with fetchpatch? You can use
pass excludes = [ "CHANGELOG.md" ]; to avoid potential diff conflicts.

TIL. Thanks, done.

@arianvp
Copy link
Member

arianvp commented Apr 27, 2020

Could you remove the GitHub handles from the commit message? Otherwise I'll get s notification from every fork of nixpkgs on GitHub for the coming weeks due to Githubs completely broken notification policy

Since 20.03 still uses old oauth2_proxy (3.2.0), which is not compatible
with the newest one (5.1.0), this change backports an important security
fix to 3.2.0:

oauth2-proxy/oauth2-proxy@a316f8a

The vulnerability is an open redirect, where a bad actor can redirect a
session to another domain using `/\` in redirect URIs.
@knl knl force-pushed the security-fix-oauth2_proxy branch from b6c591c to 92ab877 Compare April 27, 2020 13:56
@Mic92
Copy link
Member

Mic92 commented Apr 27, 2020

Result of nixpkgs-review pr 86108 1

@Mic92 Mic92 merged commit 7d0b089 into NixOS:release-20.03 Apr 27, 2020
@knl knl deleted the security-fix-oauth2_proxy branch April 28, 2020 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants