-
Notifications
You must be signed in to change notification settings - Fork 537
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
Feat/http connect proxy support #497
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,84 @@ | ||
package mqtt | ||
|
||
import ( | ||
"bufio" | ||
"fmt" | ||
"net" | ||
"net/http" | ||
"net/url" | ||
|
||
"golang.org/x/net/proxy" | ||
) | ||
|
||
// httpProxy is a HTTP/HTTPS connect capable proxy. | ||
type httpProxy struct { | ||
host string | ||
haveAuth bool | ||
username string | ||
password string | ||
forward proxy.Dialer | ||
} | ||
|
||
func (s httpProxy) String() string { | ||
return fmt.Sprintf("HTTP proxy dialer for %s", s.host) | ||
} | ||
|
||
func newHTTPProxy(uri *url.URL, forward proxy.Dialer) (proxy.Dialer, error) { | ||
s := new(httpProxy) | ||
s.host = uri.Host | ||
s.forward = forward | ||
if uri.User != nil { | ||
s.haveAuth = true | ||
s.username = uri.User.Username() | ||
s.password, _ = uri.User.Password() | ||
} | ||
|
||
return s, nil | ||
} | ||
|
||
func (s *httpProxy) Dial(_, addr string) (net.Conn, error) { | ||
reqURL := url.URL{ | ||
Scheme: "https", | ||
Host: addr, | ||
} | ||
|
||
req, err := http.NewRequest("CONNECT", reqURL.String(), nil) | ||
if err != nil { | ||
return nil, err | ||
} | ||
req.Close = false | ||
if s.haveAuth { | ||
req.SetBasicAuth(s.username, s.password) | ||
} | ||
req.Header.Set("User-Agent", "paho.mqtt") | ||
|
||
// Dial and create the client connection. | ||
c, err := s.forward.Dial("tcp", s.host) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
err = req.Write(c) | ||
if err != nil { | ||
_ = c.Close() | ||
return nil, err | ||
} | ||
|
||
resp, err := http.ReadResponse(bufio.NewReader(c), req) | ||
if err != nil { | ||
_ = c.Close() | ||
return nil, err | ||
} | ||
_ = resp.Body.Close() | ||
if resp.StatusCode != http.StatusOK { | ||
_ = c.Close() | ||
return nil, fmt.Errorf("proxied connection returned an error: %v", resp.Status) | ||
} | ||
|
||
return c, nil | ||
} | ||
|
||
func init() { | ||
proxy.RegisterDialerType("http", newHTTPProxy) | ||
proxy.RegisterDialerType("https", newHTTPProxy) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -77,7 +77,7 @@ func openConnection(uri *url.URL, tlsc *tls.Config, timeout time.Duration, heade | |
return nil, err | ||
} | ||
|
||
tlsConn := tls.Client(conn, tlsc) | ||
tlsConn := tls.Client(conn, tlsConfigWithSni(uri, tlsc)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fine with this - I don't think it will break existing code. However I am going to have a think about the approach because I wonder if using a callback (say There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Didn't see a reference to this callback on clone of master, if this is more aligned with idiomatic use of the library then it fits better (this can then be an SNI aware connection callback impl) - as long as it's done prior to the handshake should be fine - which looking at the name of it should be the case. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Its not something that currently exists (would be a new callback). However it would solve solve some other issues too (currently there is only one *tls.Config but you can specify multiple brokers to attempt connections to). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it might be a good idea to have such a call back exposed ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm happy to add the callback (might be faster as this will touch a number of files). Are you happy to do a demo? (I don't use proxies so it's not something I can really add!). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh I think we got our wires crossed, I've already done the callback unless you think I missed something. |
||
|
||
err = tlsConn.Handshake() | ||
if err != nil { | ||
|
@@ -89,3 +89,14 @@ func openConnection(uri *url.URL, tlsc *tls.Config, timeout time.Duration, heade | |
} | ||
return nil, errors.New("unknown protocol") | ||
} | ||
|
||
func tlsConfigWithSni(uri *url.URL, conf *tls.Config) *tls.Config { | ||
tlsConfig := conf | ||
if tlsConfig.ServerName == "" { | ||
// Ensure SNI is set appropriately - make a copy to avoid polluting argument or default. | ||
c := tlsConfig.Clone() | ||
c.ServerName = uri.Hostname() | ||
tlsConfig = c | ||
} | ||
return tlsConfig | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if it's best to include this (httpProxy) in the library or get the user to do it in their code? (it does not look there is anything stopping them). Doing it here complicates changes if the user has specific requirements not met by this code; perhaps it would be better to create a proxy example that includes this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, nothing stopping them its just going to be more code to maintain by each consumer - however I can see the argument for this being not the responsibility of the library, in which case an example might suffice too.