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

ext.auth: return nil instead of OkConnect #542

Merged
merged 1 commit into from
Jul 25, 2024
Merged

Conversation

aibaars
Copy link
Contributor

@aibaars aibaars commented Jul 22, 2024

This allows falling through to a next ConnectHandler is authentication succeeds making if easier to combine with other ConnectHandlers.

Before this change the following code would not running the proxy in MITM mode, but instead directly connect to the remote host.

auth.ProxyBasic(proxy)
proxy.OnRequest().HandleConnect(goproxy.AlwaysMitm)

@elazarl Another problem with the example above is that the AlwaysMitm applies all request handlers when roundtripping including the auth.Basic handler. This means that for a CONNECT request the authentication will be checked twice, once by the auth.BasicConnect handler (good) and again by the auth.Basic one (bad). Somehow the auth.Basic handler should only be applied if there was no previous CONNECT. I'm not sure what the best way is to implement that. I've used the UserData field in some application code, like the example below, but perhaps there is a better way. In any case it would be great if the goproxy library made implementing basic authentication a bit easier. What do you think?

func notAlreadyAuthenticated() goproxy.ReqConditionFunc {
    return func(req *http.Request, ctx *goproxy.ProxyCtx) bool {
        return ctx.UserData == nil
    }
}
func newproxy() {
    ...
    const realm = "HTTP Proxy"
    f := func(user, pwd string) bool {
        validUser := stringCompare(user, cfg.ProxyAuth.Username)
        validPass := stringCompare(pwd, cfg.ProxyAuth.Password)
        return validUser && validPass
    }
    // Do basic authentication for all requests, except roundtripped requests following an authenicated CONNECT request
    proxy.OnRequest(notAlreadyAuthenticated()).Do(auth.Basic(realm, f))
    proxy.OnRequest().HandleConnectFunc(
        func(host string, ctx *goproxy.ProxyCtx) (*goproxy.ConnectAction, string) {
            action, host := auth.BasicConnect(realm, f).HandleConnect(host, ctx)
            if action == goproxy.RejectConnect {
                return action, host
            }
            // The `auth.BasicConnect` returns an `OkConnect` todo item, which connects to the host without the mitm feature and request filtering.
            // To ensure the mitm feature is enabled, we return `nil` as todo action to let another handler handle the CONNECT request.
            // The `UserData` field is set to `true` to indicate that the request has been authenticated. This is used to prevent subsequent HTTP requests
            // on the "connection" from being authenticated again.
            ctx.UserData = true
            return nil, host
        })
    proxy.OnRequest().HandleConnect(goproxy.AlwaysMitm)
    ...
}

This allows falling through to a next `ConnectHandler` is authentication succeeds making if easier to combine with other `ConnectHandler`s.
@elazarl elazarl merged commit 24b5019 into elazarl:master Jul 25, 2024
1 check passed
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