Skip to content

Commit

Permalink
More robustly check for TLS support for portal backends
Browse files Browse the repository at this point in the history
I started this because I noticed a connection failure log in the demo
mode that I wanted to get rid of.

Now we don't check for TLS before it's physically possible for the
backend server to have started. I actually ended up making the TCP proxy
implementation more robust than the HTTP one because that was the only
way to do it in that case, even though we probably don't have anyone
using TCP proxy without a FixedPort anyway.

Now there's only one unlikely edge case kind of HTTP server backend that
we will fail to detect HTTPS for.
  • Loading branch information
fsmv committed Jul 5, 2024
1 parent 5aab2c4 commit f900c56
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 27 deletions.
31 changes: 20 additions & 11 deletions portal/embedportal/http_proxyserv.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@ func (p *httpProxy) Register(
func (p *httpProxy) saveForwarder(clientAddr string, lease *gate.Lease,
request *gate.RegisterRequest) error {

var protocol string
addrPort := fmt.Sprintf("%v:%v", clientAddr, lease.Port)

// TODO: when assimilate supports writing cert files, maybe make it so
Expand Down Expand Up @@ -131,22 +130,32 @@ func (p *httpProxy) saveForwarder(clientAddr string, lease *gate.Lease,
}
}

// Detect TLS support
conn, err := tls.Dial("tcp", addrPort, conf)
if err == nil {
conn.Close()
protocol = "https://"
} else {
if len(request.CertificateRequest) != 0 {
log.Printf("Error: failed to connect using TLS for the %v backend. When certificate_request is used, you must serve with that certificate. Message: %v",
lease.Pattern, err)
// Detect TLS support for FixedPort backends, if we don't have a FixedPort set
// then the server cannot be already running and won't run until we return
// this RPC.
protocol := "http://"
if request.FixedPort != 0 {
conn, err := tls.Dial("tcp", addrPort, conf)
if err == nil {
conn.Close()
protocol = "https://"
} else {
log.Printf("Warning: TLS is not supported for the %v backend. This internal traffic will not be encrypted. Message: %v",
lease.Pattern, err)
protocol = "http://"
}
}
// If there was a cert request then we require HTTPS. Most go clients will use
// the gate API and will be using cert requests.
//
// So this covers detecting TLS for go usecases and above we checked for most
// third-party usecases which will not be able to register with the portal
// API.
// TODO: maybe do more robust TLS checking with the same state machine idea as
// in tcp_proxyserv.go, but it's niche, it would only help non-FixedPort
// non-CertificateRequest backends that do use their own cert.
if len(request.CertificateRequest) != 0 {
protocol = "https://"
}

backend, err := url.Parse(protocol + addrPort)
if err != nil {
Expand Down
75 changes: 59 additions & 16 deletions portal/embedportal/tcp_proxyserv.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"net"
"strings"
"sync"
"sync/atomic"

"ask.systems/daemon/portal/gate"
)
Expand Down Expand Up @@ -73,20 +74,36 @@ func (p *tcpProxy) Register(clientAddr string, request *gate.RegisterRequest) (*
return lease, nil
}

func handleConnection(publicConn net.Conn, serverAddress string, tlsConf *tls.Config, quit chan struct{}) {
func handleConnection(publicConn net.Conn, serverAddress string, tlsCheck *tlsChecker, quit chan struct{}) {
var privateConn net.Conn
var err error
if tlsConf != nil {
privateConn, err = tls.Dial("tcp", serverAddress, tlsConf)
} else {
switch tlsCheck.State() {
case tlsState_UNKNOWN:
privateConn, err = tls.Dial("tcp", serverAddress, tlsCheck.Conf)
if err != nil {
tlsErr := err
privateConn, err = net.Dial("tcp", serverAddress)
if err == nil {
tlsCheck.TCPOnly()
log.Printf("Warning: TLS is not supported for the %v TCP backend. This internal traffic will not be encrypted. Message: %v",
serverAddress, tlsErr)
}
} else {
tlsCheck.TLSOnly()
}
case tlsState_TLS_ONLY:
privateConn, err = tls.Dial("tcp", serverAddress, tlsCheck.Conf)
case tlsState_TCP_ONLY:
privateConn, err = net.Dial("tcp", serverAddress)
}

if err != nil {
log.Printf("Failed to connect to TCP Proxy backend (for client %v): %v",
publicConn.RemoteAddr(), err)
publicConn.Close()
return
}

go func() {
<-quit // when we quit, close all the connections
// TODO: do we want to have a timeout for graceful stopping?
Expand Down Expand Up @@ -117,6 +134,39 @@ func handleConnection(publicConn net.Conn, serverAddress string, tlsConf *tls.Co
publicConn.RemoteAddr(), serverAddress, privateConn.LocalAddr())
}

// Conf is read only once set. Tracks a simple state machine atomically.
//
// States:
// 1. Unknown if there's TLS support (try running TLS, fallback to non TLS)
// 2. Has TLS (only try TLS)
// 3. No TLS (only try non-TLS)
type tlsChecker struct {
Conf *tls.Config
state atomic.Int32
}

type tlsState int32

const (
tlsState_UNKNOWN tlsState = iota
tlsState_TLS_ONLY
tlsState_TCP_ONLY
)

// Starts in the UNKNOWN state and transitions to one of the other states
// exactly once when support is reported.
func (c *tlsChecker) State() tlsState {
return tlsState(c.state.Load())
}

func (c *tlsChecker) TCPOnly() {
c.state.CompareAndSwap(int32(tlsState_UNKNOWN), int32(tlsState_TCP_ONLY))
}

func (c *tlsChecker) TLSOnly() {
c.state.CompareAndSwap(int32(tlsState_UNKNOWN), int32(tlsState_TLS_ONLY))
}

func startTCPForward(tlsListener net.Listener, serverAddress string, quit chan struct{}) {
go func() {
<-quit // stop listening when we quit
Expand All @@ -126,17 +176,10 @@ func startTCPForward(tlsListener net.Listener, serverAddress string, quit chan s
// TODO: when assimilate supports certificate requests, make it so we
// verify using the portal root CA (and maybe system CAs) when the cert
// request was used.
conf := &tls.Config{
InsecureSkipVerify: true,
}
// Detect TLS support
conn, err := tls.Dial("tcp", serverAddress, conf)
if err == nil {
conn.Close()
} else {
log.Printf("Warning: TLS is not supported for the %v TCP backend. This internal traffic will not be encrypted. Message: %v",
serverAddress, err)
conf = nil
tlsCheck := &tlsChecker{
Conf: &tls.Config{
InsecureSkipVerify: true,
},
}

go func() {
Expand All @@ -150,7 +193,7 @@ func startTCPForward(tlsListener net.Listener, serverAddress string, quit chan s

// Use a goroutine just to not wait until the Dial is done before we
// can accept connections again
go handleConnection(publicConn, serverAddress, conf, quit)
go handleConnection(publicConn, serverAddress, tlsCheck, quit)
}
tlsListener.Close()
}()
Expand Down

0 comments on commit f900c56

Please sign in to comment.