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

Support TLS connection to HTTP Proxy #950

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

134130
Copy link

@134130 134130 commented Jul 18, 2024

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update
  • Go Version Update
  • Dependency Update

Description

  • Support TLS Connection between websocket client and http proxy.

cc. @adrianosela

Related Tickets & Documents

Added/updated tests?

  • Yes
  • No, and this is why: please replace this line with details on why tests
    have not been included
  • I need help with writing tests

Run verifications and test

  • make verify is passing
  • make test is passing

Comment on lines -278 to -290
// If needed, wrap the dial function to connect through a proxy.
if d.Proxy != nil {
proxyURL, err := d.Proxy(req)
if err != nil {
return nil, nil, err
}
if proxyURL != nil {
netDial, err = proxyFromURL(proxyURL, netDial)
if err != nil {
return nil, nil, err
}
}
}
Copy link
Author

Choose a reason for hiding this comment

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

Also need to set a deadline for the dialing to proxy, so moved the line up.

Comment on lines 88 to 132
type cstProxyServer struct{}

func (s *cstProxyServer) ServeHTTP(w http.ResponseWriter, req *http.Request) {
if req.Method != http.MethodConnect {
http.Error(w, "method not allowed", http.StatusMethodNotAllowed)
return
}

conn, _, err := w.(http.Hijacker).Hijack()
if err != nil {
http.Error(w, err.Error(), http.StatusInternalServerError)
return
}
defer conn.Close()

upstream, err := (&net.Dialer{}).DialContext(req.Context(), "tcp", req.URL.Host)
if err != nil {
_, _ = fmt.Fprintf(conn, "HTTP/1.1 502 Bad Gateway\r\n\r\n")
return
}
defer upstream.Close()

_, _ = fmt.Fprintf(conn, "HTTP/1.1 200 Connection established\r\n\r\n")

wg := sync.WaitGroup{}
wg.Add(2)
go func() {
defer wg.Done()
_, _ = io.Copy(upstream, conn)
}()
go func() {
defer wg.Done()
_, _ = io.Copy(conn, upstream)
}()
wg.Wait()
}

func newProxyServer() *httptest.Server {
return httptest.NewServer(&cstProxyServer{})
}

func newTLSProxyServer() *httptest.Server {
return httptest.NewTLSServer(&cstProxyServer{})
}

Copy link
Author

Choose a reason for hiding this comment

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

Previously, only had a temporary proxy implementation, and couldn't test the TLS proxy, so implemented it.

_, _ = fmt.Fprintf(conn, "HTTP/1.1 200 Connection established\r\n\r\n")

wg := sync.WaitGroup{}
wg.Add(2)
Copy link

@adrianosela adrianosela Jul 18, 2024

Choose a reason for hiding this comment

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

Suggestion: I think you probably want to exit as soon as one io.Copy hits EOF...

Here's two ways I think you can achieve this:

	var wg sync.WaitGroup
	defer wg.Wait()

	wg.Add(1)
	go func() {
		defer wg.Done()

		// abort blocked reads from upstream when done reading from conn
		defer upstream.SetDeadline(time.Now().Add(-1 * time.Hour))

		_, _ = io.Copy(upstream, conn)
	}()

	wg.Add(1)
	go func() {
		defer wg.Done()

		// abort blocked reads from conn when done reading from upstream
		defer conn.SetDeadline(time.Now().Add(-1 * time.Hour))

		_, _ = io.Copy(conn, upstream)
	}()

OR more simple - block on an unbuffered channel:

        done := make(chan struct{})

	go func() {
		_, _ = io.Copy(upstream, conn)
                done <- struct{}{}
	}()

	go func() {
		_, _ = io.Copy(conn, upstream)
               done <- struct{}{}
	}()

        <- done

Copy link
Author

Choose a reason for hiding this comment

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

Yes, I prefer the below. I used wg because It's just a test helper function.

I'll change to the below one.

Copy link

@adrianosela adrianosela left a comment

Choose a reason for hiding this comment

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

Looks great :D

Thanks so much for hopping on this so quickly. Im wondering what kind of testing was done (aside from unit tests).

@134130
Copy link
Author

134130 commented Jul 18, 2024

@adrianosela
Actually, I failed to found well-known proxies which supports TLS Listening.

I'm making HTTPS Proxy in my work, in golang, and I tested with this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Is https not supported? [feature] Support for proxying websocket through https proxy
2 participants