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

chore: Authentication negotiation for Kerberos #3430

Merged
merged 1 commit into from
Jul 22, 2022

Conversation

PeterSchafer
Copy link
Collaborator

@PeterSchafer PeterSchafer commented Jul 1, 2022

What does this PR do?

This PR introduces improved SPNEGO/Negotiate support for Proxy Authentication. It enables NTLM authentication by enabling multiple CONNECT messages being exchanged between CLIv2 and an upstream Proxy Server.

To achieve this, it dynamically extends the Go http.Transport implementation by decorating the accessor for TCP connections.

Where should the reviewer start?

A good start is top down from proxy/proxy.go and then along the different levels of abstraction.

How should this be manually tested?

In the Windows test environment on AWS, run either

Kerberos Authentication: snyk_windows_amd64.exe --proxy=http://EC2AMAZ-vuHlNQ.hammer2.snyk.io:3128 --proxy-negotiate woof -d
NTLM Authentication: snyk_windows_amd64.exe --proxy=http://proxy.hammer2.snyk.io:3128 --proxy-negotiate woof -d

Any background context you want to provide?

https://www.ietf.org/rfc/rfc4559.txt

@PeterSchafer PeterSchafer requested a review from a team as a code owner July 1, 2022 16:37
@PeterSchafer PeterSchafer marked this pull request as draft July 1, 2022 16:37
@PeterSchafer PeterSchafer force-pushed the chore/cliv2_proxy_negotiate branch 4 times, most recently from 1f07579 to 6410599 Compare July 8, 2022 15:41
@PeterSchafer PeterSchafer marked this pull request as ready for review July 20, 2022 17:03
cliv2/internal/proxy/proxy.go Outdated Show resolved Hide resolved
cliv2/internal/proxy/proxy.go Outdated Show resolved Hide resolved
cliv2/internal/proxy/proxy_test.go Outdated Show resolved Hide resolved
cliv2/internal/httpauth/proxy_authenticator.go Outdated Show resolved Hide resolved
cliv2/internal/httpauth/spnego_test.go Show resolved Hide resolved
cliv2/internal/httpauth/proxy_authenticator.go Outdated Show resolved Hide resolved
cliv2/internal/proxy/proxy.go Show resolved Hide resolved
cliv2/internal/httpauth/proxy_authenticator.go Outdated Show resolved Hide resolved
cliv2/internal/httpauth/httpauth.go Show resolved Hide resolved
* not fully working yet!!!
* missing: negotiation integration in httpauth.Authenticationhandler

Signed-off-by: Peter Schäfer <101886095+PeterSchafer@users.noreply.github.com>

wip: replace go-spnego lib

chore: add missing “Negotiate” in authorization header

Signed-off-by: Peter Schäfer <101886095+PeterSchafer@users.noreply.github.com>

chore: implement negotiation for windows

* Changed SpnegoProvider interface and implemented updating security context
* fixed issue when sending data

Signed-off-by: Peter Schäfer <101886095+PeterSchafer@users.noreply.github.com>

chore: ensure to free up ressources in authhandler

Signed-off-by: Peter Schäfer <101886095+PeterSchafer@users.noreply.github.com>

chore: remove comments after going trhough them

Signed-off-by: Peter Schäfer <101886095+PeterSchafer@users.noreply.github.com>

chore: support -d

Signed-off-by: Peter Schäfer <101886095+PeterSchafer@users.noreply.github.com>

chore: adapt debug logging

Signed-off-by: Peter Schäfer <101886095+PeterSchafer@users.noreply.github.com>

chore: Reduce messages send during negotiation

* removed initial state for negotiate since it is not necessary in the current implementation
* made state private

Signed-off-by: Peter Schäfer <101886095+PeterSchafer@users.noreply.github.com>

chore: fixed some typos and simplified naming

Signed-off-by: Peter Schäfer <101886095+PeterSchafer@users.noreply.github.com>

chore: implemented spnego_nonwindows

* including some minor additions and cleanup

Signed-off-by: Peter Schäfer <101886095+PeterSchafer@users.noreply.github.com>

chore: add missing import

Signed-off-by: Peter Schäfer <101886095+PeterSchafer@users.noreply.github.com>

chore: refactoring and handling edge cases

* refactoring for better testability and readability
* handling edge cases

Signed-off-by: Peter Schäfer <101886095+PeterSchafer@users.noreply.github.com>

chore: add src file dependency to makefile

to avoid cleaning

Signed-off-by: Peter Schäfer <101886095+PeterSchafer@users.noreply.github.com>

chore: go mod tidy

Signed-off-by: Peter Schäfer <101886095+PeterSchafer@users.noreply.github.com>

chore: moved proxy_authenticator to httpauth

* improved error Handling during authentication

Signed-off-by: Peter Schäfer <101886095+PeterSchafer@users.noreply.github.com>

chore: throwing error when cycle max reached

Signed-off-by: Peter Schäfer <101886095+PeterSchafer@users.noreply.github.com>

chore: fix failing tests

Signed-off-by: Peter Schäfer <101886095+PeterSchafer@users.noreply.github.com>

chore: remove outdated tests

Signed-off-by: Peter Schäfer <101886095+PeterSchafer@users.noreply.github.com>

chore: added logging that indicates the token type

Signed-off-by: Peter Schäfer <101886095+PeterSchafer@users.noreply.github.com>

chore: close connection only if not nil

Signed-off-by: Peter Schäfer <101886095+PeterSchafer@users.noreply.github.com>

chore: improved error message

Signed-off-by: Peter Schäfer <101886095+PeterSchafer@users.noreply.github.com>

chore: incorporate feedback from review

Signed-off-by: Peter Schäfer <101886095+PeterSchafer@users.noreply.github.com>
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.

2 participants