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

Migrate request context on mitmHandler #5

Open
hazcod opened this issue Sep 30, 2020 · 29 comments
Open

Migrate request context on mitmHandler #5

hazcod opened this issue Sep 30, 2020 · 29 comments
Labels
bug Something isn't working

Comments

@hazcod
Copy link

hazcod commented Sep 30, 2020

Hi, so I have a TLS listener with my mps proxy.
During handshake, I generate a variable and need to pass this to my .OnRequest(...) function.
However, anything I set on req Context with the following function is not propagated?

func getInsertor(proxy *mps.HttpProxy) http.Handler {
	return http.HandlerFunc(func(rw http.ResponseWriter, req * http.Request) {
		logger := log.WithField("ip", req.RemoteAddr).WithField("host", req.Host)

		testID, err := pkg.ExtractTestID(req)
		if testID <= 0 || err != nil {
			logger.WithError(err).Error("could not extract testid")
			_, _ = rw.Write([]byte("invalid credentials provided"))
			rw.WriteHeader(http.StatusForbidden)
			return
		}
		proxy.ServeHTTP(rw, req.WithContext(context.WithValue(req.Context(), "testid", testID)))
	})
}

	proxyInstance := mps.NewHttpProxy()
	mitmHandler := mps.NewMitmHandler()

	mitmHandler.OnRequest().DoFunc(func(req *http.Request, ctx *mps.Context) (*http.Request, *http.Response){
		testID, ok := req.Context().Value("testid").(uint64) // This will always be nil/zero
@hazcod
Copy link
Author

hazcod commented Sep 30, 2020

Same with ctx.Request.Context() or ctx.Context

@hazcod
Copy link
Author

hazcod commented Sep 30, 2020

And strangely enough, any headers set in getInsertor does not seem to propagate to the OnRequest.
Edit: I guess this is because getInsertor only occurs on the CONNECT call, and does not modify the actual request embedded within.

@telanflow
Copy link
Owner

telanflow commented Oct 1, 2020

You should instantiate mitmHandler as follows:

proxyInstance := mps.NewHttpProxy()

// Use the same mps.Context instance
mitmHandler := mps.NewMitmHandlerWithContext(proxyInstance.Ctx)

// Process the CONNECT request
proxyInstance.HandleConnect = mitmHandler

Because middleware is implemented by mps.Context, Filter is based middleware

As middleware is implemented by mps.Context, middleware and filter won't work if you don't use the same mps.Context, which is a bit bad, so I wanted to find a better way to middleware.

@hazcod
Copy link
Author

hazcod commented Oct 1, 2020

Ahh. not quite sure how I missed that, thanks @telanflow !
Loving the rewrite.

@hazcod hazcod closed this as completed Oct 1, 2020
@hazcod
Copy link
Author

hazcod commented Oct 1, 2020

@telanflow can you elaborate on which context I should set/get the value to get it to pass from CONNECT to the followup GET or POST request?
It doesn't seem to pass on either req.Context or ctx.Context.

I am using mitmHandler.OnRequest().DoFunc, but same with proxyInstance.OnRequest().DoFunc.

		if req.Method == http.MethodConnect {
			testID, err = pkg.ExtractTestID(req)
		} else {
		// if not, we really need a testID to be present in the context to continue
			var ok bool
			testID, ok = ctx.Context.Value("testid").(uint64)
			if !ok { err = errors.Errorf("invalid testid in context: %+v", req.Context().Value("testid")) }
		}

		if testID <= 0 || err != nil {
			logger.WithError(err).Error("could not extract testid")
			return req, newHttpResponse(req, http.StatusForbidden, "Forbidden", "invalid credentials provided")
		}

		if req.Method == http.MethodConnect {
			ctx.Context = context.WithValue(ctx.Context, "testid", testID)
			return req.WithContext(context.WithValue(req.Context(), "testid", testID)), nil
		}

@hazcod hazcod reopened this Oct 1, 2020
@hazcod
Copy link
Author

hazcod commented Oct 1, 2020

I tried

			req = req.WithContext(context.WithValue(req.Context(), "testid", testID))
			ctx = ctx.WithRequest(req)
			ctx.Context = context.WithValue(ctx.Context, "testid", testID)
			return req, nil

@telanflow
Copy link
Owner

telanflow commented Oct 7, 2020

I'm sorry, i went out to play on China National Day.

you can do that:

// set value
ctx.Context = context.WithValue(ctx.Context, "testid", testID)

// get value
ctx.Context.Value("testId")

Or

// set value
req.WithContext(context.WithValue(req.Context(), "testid", testID))

// Get value
req.Context().Value("testId")

@hazcod
Copy link
Author

hazcod commented Oct 7, 2020

@telanflow is it possible it won't propagate to the actual requests because the GET request is inside the CONNECT tunnel, hence a different proxy context perhaps?

e.g.

CONNECT website.com:443
// setting context

GET /foo (in CONNECT tunnel)
// getting context -> nil

I am doing it as you stipulated:

			ctx.Context = context.WithValue(ctx.Context, "testid", testID)

@telanflow
Copy link
Owner

yes, It's best to set it above the request Context

@hazcod
Copy link
Author

hazcod commented Oct 7, 2020

@telanflow can you perhaps elaborate? Not sure what you mean;

	proxyInstance.OnRequest().DoFunc(func(req *http.Request, ctx *mps.Context) (*http.Request, *http.Response) {
		logger := log.WithField("ip", req.RemoteAddr)

		var testID uint64
		var err error

		// if this is a CONNECT, scrape the testID from the request TLS metadata
		if req.Method == http.MethodConnect {
			testID, err = pkg.ExtractTestID(req)
		} else {
		// if not, we really need a testID to be present in the context to continue
			var ok bool
			testID, ok = ctx.Request.Context().Value("testid").(uint64)
			if !ok { err = errors.Errorf("invalid testid in context: %+v", req.Context().Value("testid")) }
		}

		if testID <= 0 || err != nil {
			logger.WithError(err).Error("could not extract testid")
			return req, newHttpResponse(req, http.StatusForbidden, "Forbidden", "invalid credentials provided")
		}

		// if this is a request to our proxy, set context and pass for the subsequent real requests to work
		if req.Method == http.MethodConnect {
			req = req.WithContext(context.WithValue(req.Context(), "testid", testID))
			ctx = ctx.WithRequest(req)
			ctx.Context = context.WithValue(ctx.Context, "testid", testID)
			return req, nil
		}
 ....
}

@telanflow
Copy link
Owner

telanflow commented Oct 7, 2020

Each request spawns a new Handler for the execution middleware (MITMHandler, TunnelHandler)

ctx := mitm.Ctx.WithRequest(req) 
resp, err := ctx.Next(req)

eg.

mps/mitm_handler.go

Lines 94 to 108 in 00583b5

// Standard net/http function. You can use it alone
func (mitm *MitmHandler) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
// execution middleware
ctx := mitm.Ctx.WithRequest(req)
resp, err := ctx.Next(req)
if err != nil && err != MethodNotSupportErr {
if resp != nil {
copyHeaders(rw.Header(), resp.Header, mitm.Ctx.KeepDestinationHeaders)
rw.WriteHeader(resp.StatusCode)
buf := mitm.buffer().Get()
_, err = io.CopyBuffer(rw, resp.Body, buf)
mitm.buffer().Put(buf)
}
return
}

mps/tunnel_handler.go

Lines 44 to 58 in 00583b5

// Standard net/http function. You can use it alone
func (tunnel *TunnelHandler) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
// execution middleware
ctx := tunnel.Ctx.WithRequest(req)
resp, err := ctx.Next(req)
if err != nil && err != MethodNotSupportErr {
if resp != nil {
copyHeaders(rw.Header(), resp.Header, tunnel.Ctx.KeepDestinationHeaders)
rw.WriteHeader(resp.StatusCode)
buf := tunnel.buffer().Get()
_, err = io.CopyBuffer(rw, resp.Body, buf)
tunnel.buffer().Put(buf)
}
return
}

@hazcod
Copy link
Author

hazcod commented Oct 7, 2020

But proxy context should pass on between those handlers, right?

@telanflow
Copy link
Owner

Take a look at the latest submission so that should work @hazcod

@telanflow telanflow added the bug Something isn't working label Oct 7, 2020
@hazcod
Copy link
Author

hazcod commented Oct 7, 2020

@telanflow thanks for the quick response, but still returns nil:

	proxyInstance.OnRequest().DoFunc(func(req *http.Request, ctx *mps.Context) (*http.Request, *http.Response) {
		logger := log.WithField("ip", req.RemoteAddr)

		var testID uint64
		var err error

		// if this is a CONNECT, scrape the testID from the request TLS metadata
		if req.Method == http.MethodConnect {
			testID, err = pkg.ExtractTestID(req)
		} else {
		// if not, we really need a testID to be present in the context to continue
			var ok bool
			log.Info(req.Context().Value("testid"))
			testID, ok = ctx.Context.Value("testid").(uint64)
			if !ok { err = errors.Errorf("invalid testid in context: %+v", req.Context().Value("testid")) }
		}

		if testID <= 0 || err != nil {
			logger.WithError(err).Error("could not extract testid")
			return req, newHttpResponse(req, http.StatusForbidden, "Forbidden", "invalid credentials provided")
		}

		// if this is a request to our proxy, set context and pass for the subsequent real requests to work
		if req.Method == http.MethodConnect {
			ctx.Context = context.WithValue(ctx.Context, "testid", testID)
			return req, nil
		}
...
}

Returns:

proxy_1       | [00] INFO[0004] validating fresh client                       test_id=595825677061881857 (CONNECT)
proxy_1       | [00] INFO[0004] <nil>                                        (GET)
proxy_1       | [00] ERRO[0004] could not extract testid                      error="invalid testid in context: <nil>" (GET)

@telanflow
Copy link
Owner

Is this row not getting the value?

log.Info(req.Context().Value("testid"))

Where do they set a testid?

one thing to note:
set testid middleware has to come in front of OnRequest

@hazcod
Copy link
Author

hazcod commented Oct 7, 2020

@telanflow I've split it up to make it more clear;

proxyInstance.OnRequest().DoFunc(func(req *http.Request, ctx *mps.Context) (*http.Request, *http.Response) {
		logger := log.WithField("ip", req.RemoteAddr).WithField("method", req.Method)

		if req.Method != http.MethodConnect {
			return req, nil
		}

		// if this is a CONNECT, scrape the testID from the request TLS metadata

		testID, err := pkg.ExtractTestID(req)
		if err != nil {
			logger.WithError(err).Error("could not extract testid")
			return req, newHttpResponse(req, http.StatusForbidden, "Forbidden", "invalid credentials provided")
		}

		ctx.Context = context.WithValue(ctx.Context, "testid", testID)
		return req, nil
	})

	proxyInstance.OnRequest().DoFunc(func(req *http.Request, ctx *mps.Context) (*http.Request, *http.Response) {
		logger := log.WithField("ip", req.RemoteAddr).WithField("method", req.Method)

		// if not, we really need a testID to be present in the context to continue
		var ok bool
		logger.Info(req.Context().Value("testid"), ctx.Context.Value("testid"))
		testID, ok := ctx.Context.Value("testid").(uint64)
		if !ok { err = errors.Errorf("invalid testid in context: %+v", req.Context().Value("testid")) }

		if testID <= 0 || err != nil {
			logger.WithError(err).Error("could not extract testid")
			return req, newHttpResponse(req, http.StatusForbidden, "Forbidden", "invalid credentials provided")
		}

		logger = logger.WithField("testid", testID)

		// if this is a request to our proxy, set context and pass for the subsequent real requests to work
		if req.Method == http.MethodConnect {
			return req, nil
		}

		return req, nil
	})

And the logs, showing the testID context being set the first time

proxy_1       | [00] INFO[0010] validating fresh client                       ip="172.19.0.1:33034" method=CONNECT test_id=595825677061881857
proxy_1       | [00] INFO[0010] <nil> 595825677061881857                      ip="172.19.0.1:33034" method=CONNECT
proxy_1       | [00] INFO[0010] <nil> <nil>                                   ip="172.19.0.1:33034" method=GET
proxy_1       | [00] ERRO[0010] could not extract testid                      error="invalid testid in context: <nil>" ip="172.19.0.1:33034" method=GET

@hazcod
Copy link
Author

hazcod commented Oct 7, 2020

@telanflow : this made me think, will Context be propagated through HandleConnect and HtttpHandler?

proxyInstance := mps.NewHttpProxy()
	mitmHandler := mps.NewMitmHandlerWithContext(proxyInstance.Ctx)
	proxyInstance.HandleConnect = mitmHandler
	proxyInstance.HttpHandler = mitmHandler

	proxyInstance.OnRequest().DoFunc(...)

@hazcod
Copy link
Author

hazcod commented Oct 8, 2020

Hmm, same result with:

	proxyInstance := mps.NewHttpProxy()
	proxyInstance.HandleConnect = mps.NewMitmHandlerWithContext(proxyInstance.Ctx)

	proxyInstance.UseFunc(func(req *http.Request, ctx *mps.Context) (*http.Response, error) {
...
       }

@hazcod
Copy link
Author

hazcod commented Oct 13, 2020

@telanflow sorry for bugging, but any idea?

@telanflow
Copy link
Owner

telanflow commented Oct 14, 2020

I'm sorry. I've been a little busy lately.

The code is as follows:

func main() {
	proxy := mps.NewHttpProxy()
	proxy.HandleConnect = mps.NewMitmHandlerWithContext(proxy.Ctx)

	proxy.OnRequest().DoFunc(func(req *http.Request, ctx *mps.Context) (*http.Request, *http.Response) {
		// request Context with value
		reqCtx := context.WithValue(req.Context(), "testid", "1")
		req = req.WithContext(reqCtx)

		// mps.Context with value
		ctx.Context = context.WithValue(ctx.Context, "testid", "2")

		return req, nil
	})

	proxy.OnRequest().DoFunc(func(req *http.Request, ctx *mps.Context) (*http.Request, *http.Response) {
		// get req.Context() value
		reqVal := req.Context().Value("testid")
		log.Printf("Method: %s URL: %s req:testid: %v", req.Method, req.URL, reqVal)

		// get mps.Context.Context value
		ctxVal := ctx.Context.Value("testid")
		log.Printf("Method: %s URL: %s ctx:testid: %v", req.Method, req.URL, ctxVal)

		return req, nil
	})

	_ = http.ListenAndServe(":8888", proxy)
}

logs:

2020/10/14 17:01:25 Method: CONNECT URL: //httpbin.org:443 req:testid: 1
2020/10/14 17:01:25 Method: CONNECT URL: //httpbin.org:443 ctx:testid: 2
2020/10/14 17:01:25 Method: GET URL: https://httpbin.org:443/get req:testid: 1
2020/10/14 17:01:25 Method: GET URL: https://httpbin.org:443/get ctx:testid: 2

@hazcod
Copy link
Author

hazcod commented Oct 14, 2020

@telanflow thanks for the support, but this not seem to propagate with your latest mps release.
Could it be because of my TLS listener with http.Serve(liTLS, proxyInstance)?

	liTLS := tls.NewListener(li, &tls.Config{
		Certificates:             []tls.Certificate{*serverCrt, caCert},
		ClientAuth:               tls.RequireAndVerifyClientCert,
		ClientCAs:                clientCACertPool,
		PreferServerCipherSuites: true,
		MinVersion:               tls.VersionTLS12,
		MaxVersion:               tls.VersionTLS13,
		Renegotiation:            tls.RenegotiateNever,
		SessionTicketsDisabled:   true,
		NextProtos:               []string{"http/1.1", "http/1.0"},
	})

Because I get context.Background() for req.Context() and ctx.Context() in the second DoFunc: WARN[0002] testid not found in context ip="172.19.0.1:51254" method=GET proxy_ctx=context.Background req_ctx=context.Background testid=0

EDIT: in your case, you are -always- setting testID because your are not skipping the first handler if is -not- CONNECT.

try with:

func main() {
	proxy := mps.NewHttpProxy()
	proxy.HandleConnect = mps.NewMitmHandlerWithContext(proxy.Ctx)

	proxy.OnRequest().DoFunc(func(req *http.Request, ctx *mps.Context) (*http.Request, *http.Response) {
                if req.Method != http.MethodConnect { return req, nil }

		// request Context with value
		reqCtx := context.WithValue(req.Context(), "testid", "1")
		req = req.WithContext(reqCtx)

		// mps.Context with value
		ctx.Context = context.WithValue(ctx.Context, "testid", "2")

		return req, nil
	})

	proxy.OnRequest().DoFunc(func(req *http.Request, ctx *mps.Context) (*http.Request, *http.Response) {
                if req.Method == http.MethodConnect { return req, nil }

		// get req.Context() value
		reqVal := req.Context().Value("testid")
		log.Printf("Method: %s URL: %s req:testid: %v", req.Method, req.URL, reqVal)

		// get mps.Context.Context value
		ctxVal := ctx.Context.Value("testid")
		log.Printf("Method: %s URL: %s ctx:testid: %v", req.Method, req.URL, ctxVal)

		return req, nil
	})

	_ = http.ListenAndServe(":8888", proxy)
}

@telanflow
Copy link
Owner

you should know that an HTTP request is executed sequentially to the middleware, and if middleware 1 skips it, middleware 2 will not get the value.

GET  -> middleware1 (context set value) -> middleware2 (get context value)

POST -> middleware1 (jump) -> middleware2 (not get context value)

CONNECT -> middleware1 -> middleware2

You can set MPS Proxy as a property of another object to uniformly set your own values.

package main

import (
	"context"
	"errors"
	"github.com/telanflow/mps"
	"log"
	"net/http"
	"os"
	"os/signal"
	"syscall"
)

type Srv struct {
	server *http.Server
	proxyHandler *mps.HttpProxy

	ctx context.Context
	signChan chan os.Signal
}

func NewSrv(addr string) *Srv {
	proxyHandler := mps.NewHttpProxy()
	proxyHandler.HandleConnect = mps.NewMitmHandlerWithContext(proxyHandler.Ctx)

	return &Srv{
		signChan:     make(chan os.Signal),
		proxyHandler: proxyHandler,
		ctx: context.Background(),
		server: &http.Server{
			Addr: addr,
			Handler: proxyHandler,
		},
	}
}

func (s *Srv) Run() {
	// proxy on request
	s.proxyHandler.OnRequest().DoFunc(s.handleRequest1)
	s.proxyHandler.OnRequest().DoFunc(s.handleRequest2)

	// listen quit notify
	signal.Notify(s.signChan, syscall.SIGINT, syscall.SIGKILL, syscall.SIGTERM, syscall.SIGQUIT)

	// start server
	go func() {
		log.Println("proxy server started")
		err := s.server.ListenAndServe()
		if errors.Is(err, http.ErrServerClosed) {
			return
		}

		if err != nil {
			s.signChan <- syscall.SIGKILL
			log.Fatalf("Http Fail: %v", err)
		}
	}()

	// stop
	<-s.signChan
	s.server.Shutdown(context.Background())
	log.Fatal("proxy server exit.")
}

// Handle Request 1
func (s *Srv) handleRequest1(req *http.Request, ctx *mps.Context) (*http.Request, *http.Response) {
	if req.Method != http.MethodConnect { return req, nil }

	// mps.Context with value
	s.ctx = context.WithValue(s.ctx, "testid", "3")
	return req, nil
}

// Handle Request 2
func (s *Srv) handleRequest2(req *http.Request, ctx *mps.Context) (*http.Request, *http.Response) {
	if req.Method == http.MethodConnect { return req, nil }

	// get mps.Context.Context value
	ctxVal := s.ctx.Value("testid")
	log.Printf("Method: %s URL: %s s.ctx:testid: %v", req.Method, req.URL, ctxVal)
	return req, nil
}

func main() {
	// start proxy server
	srv := NewSrv("127.0.0.1:8888")
	srv.Run()
}

logs:

2020/10/14 21:43:02 proxy server started
2020/10/14 21:43:10 Method: GET URL: https://httpbin.org:443/get s.ctx:testid: 3
2020/10/14 21:43:52 proxy server exit.

@telanflow
Copy link
Owner

Sorry, I'm not good at English, haha

@hazcod
Copy link
Author

hazcod commented Oct 14, 2020

@telanflow but wouldn't that create a race condition on Srv mps.Context?

Also, the first middleware will only need to run on CONNECT while a second can only verify a non-CONNECT.

@telanflow
Copy link
Owner

telanflow commented Oct 15, 2020

I'm sorry, I don't really understand what you want to do. Can you describe it?

can you give a complete example of a failed requirement?

@hazcod

@hazcod
Copy link
Author

hazcod commented Oct 15, 2020

@telanflow yeah sorry for not specifying myself clearly enough.
This is the problem case: https://gist.github.com/hazcod/20e5050c5098bcb1d4da087254b0821c
So test it like this:

% go run main.go
INFO[0000] running                                       listener="127.0.0.1:9999"

Issue a test request:

% ALL_PROXY="http://localhost:9999" curl -k -L https://google.be/
out of scope request%  

And you'll see the issue:

INFO[0001] request                                       method=CONNECT url="//google.be:443"
INFO[0001] set testid                                    ip="127.0.0.1:58652" method=CONNECT testid=foo
INFO[0001] request                                       method=GET url="https://google.be:443/"
WARN[0001] testid not found in context                   ip="127.0.0.1:58652" method=GET proxy_ctx=context.Background req_ctx=context.Background testid=0

@hazcod
Copy link
Author

hazcod commented Oct 15, 2020

but if you would use your Srv solution, if you have concurrent requests, they can both write/read to the same mps.Context object, causing a race condition.

@telanflow
Copy link
Owner

telanflow commented Oct 16, 2020

you should be aware of the HTTPS proxy process:

1 step:
    brower <-> CONNECT request <-> mps (SSL)
2 step:
    brower <-> (GET | POST and more method) <-> mps <-> origin

the lifetime of the Context and the request binding, you can't get the Context from the last request in the next request.

CONNECT request【Context 1】 -> mps

GET request【Context 2】 -> mps

you can avoid concurrency problems by lock

@hazcod
Copy link
Author

hazcod commented Oct 16, 2020

Hmm, but if the CONNECT connection stays open the lock will never release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants