Skip to content

Commit 952cd28

Browse files
committed
Fix issue where exposing server port would result in invalid ip address, add option to link to allow host header to be used for template determination
1 parent 95d4f48 commit 952cd28

File tree

8 files changed

+62
-21
lines changed

8 files changed

+62
-21
lines changed

internal/client/handlers/remoteforward.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -121,11 +121,11 @@ func handleData(rf internal.RemoteForwardRequest, proxyCon net.Conn, sshConn ssh
121121

122122
drtMsg := internal.ChannelOpenDirectMsg{
123123

124-
Raddr: rf.BindAddr,
125-
Rport: rf.BindPort,
124+
Raddr: originatorAddress,
125+
Rport: uint32(originatorPortInt),
126126

127-
Laddr: originatorAddress,
128-
Lport: uint32(originatorPortInt),
127+
Laddr: rf.BindAddr,
128+
Lport: rf.BindPort,
129129
}
130130

131131
log.Printf("formed drtMsg: %+v", drtMsg)

internal/server/commands/connect.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@ func (c *connect) Run(user *users.User, tty io.ReadWriter, line terminal.ParsedL
4343
}
4444

4545
if len(line.Arguments) < 1 {
46-
return fmt.Errorf(c.Help(false))
46+
47+
return fmt.Errorf("%s", c.Help(false))
4748
}
4849

4950
shell, _ := line.GetArgString("shell")

internal/server/commands/link.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ func (l *link) ValidArgs() map[string]string {
4141
"stdio": "Use stdin and stdout as transport, will disable logging, destination after stdio:// is ignored",
4242
"http": "Use http polling as the underlying transport",
4343
"https": "Use https polling as the underlying transport",
44+
"use-host-header": "Use the client supplied host header as the callback address",
4445
"shared-object": "Generate shared object file",
4546
"fingerprint": "Set RSSH server fingerprint will default to server public key",
4647
"garble": "Use garble to obfuscate the binary (requires garble to be installed)",
@@ -154,6 +155,8 @@ func (l *link) Run(user *users.User, tty io.ReadWriter, line terminal.ParsedLine
154155
buildConfig.ConnectBackAdress = webserver.DefaultConnectBack
155156
}
156157

158+
buildConfig.UseHostHeader = line.IsSet("use-host-header")
159+
157160
tt := map[string]bool{
158161
"tls": line.IsSet("tls"),
159162
"wss": line.IsSet("wss"),

internal/server/data/downloads.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ type Download struct {
2525
Version string
2626
FileSize float64
2727

28+
// when generating the template use the host header
29+
UseHostHeader bool
30+
2831
// Where to download the file to
2932
WorkingDirectory string
3033
}

internal/server/handlers/forwardserverport.go

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,23 @@ var (
2020
remoteForwards = map[string]ssh.Channel{}
2121
)
2222

23+
type chanAddress struct {
24+
Port uint32
25+
IP string
26+
}
27+
28+
func (c *chanAddress) Network() string {
29+
return "remote_forward_tcp"
30+
}
31+
32+
func (c *chanAddress) String() string {
33+
return net.JoinHostPort(c.IP, fmt.Sprintf("%d", c.Port))
34+
}
35+
2336
type chanConn struct {
24-
channel ssh.Channel
25-
drtMsg internal.ChannelOpenDirectMsg
37+
channel ssh.Channel
38+
localAddr chanAddress
39+
remoteAddr chanAddress
2640
}
2741

2842
func (c *chanConn) Read(b []byte) (n int, err error) {
@@ -38,17 +52,11 @@ func (c *chanConn) Close() error {
3852
}
3953

4054
func (c *chanConn) LocalAddr() net.Addr {
41-
return &net.TCPAddr{
42-
IP: net.ParseIP(c.drtMsg.Laddr),
43-
Port: int(c.drtMsg.Lport),
44-
}
55+
return &c.localAddr
4556
}
4657

4758
func (c *chanConn) RemoteAddr() net.Addr {
48-
return &net.TCPAddr{
49-
IP: net.ParseIP(c.drtMsg.Raddr),
50-
Port: int(c.drtMsg.Rport),
51-
}
59+
return &c.remoteAddr
5260
}
5361

5462
func (c *chanConn) SetDeadline(t time.Time) error {
@@ -67,7 +75,17 @@ func (c *chanConn) SetWriteDeadline(t time.Time) error {
6775

6876
func channelToConn(channel ssh.Channel, drtMsg internal.ChannelOpenDirectMsg) net.Conn {
6977

70-
return &chanConn{channel, drtMsg}
78+
return &chanConn{
79+
channel: channel,
80+
localAddr: chanAddress{
81+
Port: drtMsg.Lport,
82+
IP: drtMsg.Raddr,
83+
},
84+
remoteAddr: chanAddress{
85+
Port: drtMsg.Rport,
86+
IP: drtMsg.Raddr,
87+
},
88+
}
7189
}
7290

7391
func ServerPortForward(clientId string) func(_ string, _ *users.User, newChannel ssh.NewChannel, log logger.Logger) {

internal/server/sshd.go

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -253,37 +253,51 @@ func StartSSHServer(sshListener net.Listener, privateKey ssh.Signer, insecure, o
253253
PublicKeyCallback: func(conn ssh.ConnMetadata, key ssh.PublicKey) (*ssh.Permissions, error) {
254254

255255
remoteIp := getIP(conn.RemoteAddr().String())
256+
// from forwradserverport.go, effectively when pivoting and exposing the server port we have to just trust whatever structure the client gives us for our remote/local addresses,
257+
// we dont want someone being able to bypass ip allow lists, so mark it as untrusted
258+
isUntrustWorthy := conn.RemoteAddr().Network() == "remote_forward_tcp"
256259

257260
if remoteIp == nil {
258261
return nil, fmt.Errorf("not authorized %q, could not parse IP address %s", conn.User(), conn.RemoteAddr())
259262
}
260263

261264
// Check administrator keys first, they can impersonate users
262265
perm, err := CheckAuth(adminAuthorizedKeysPath, key, remoteIp, false)
263-
if err == nil {
266+
if err == nil && !isUntrustWorthy {
264267
perm.Extensions["type"] = "user"
265268
perm.Extensions["privilege"] = "5"
266269

267270
return perm, err
268271
}
269272
if err != ErrKeyNotInList {
270-
return nil, fmt.Errorf("admin with supplied username (%s) denied login: %s", strconv.QuoteToGraphic(conn.User()), err)
273+
err = fmt.Errorf("admin with supplied username (%s) denied login: %s", strconv.QuoteToGraphic(conn.User()), err)
274+
if isUntrustWorthy {
275+
err = fmt.Errorf("admin (%s) denied login: %s: cannot connect admins via pivoted server port (may result in allow list bypass)", strconv.QuoteToGraphic(conn.User()), err)
276+
}
277+
return nil, err
271278
}
272279

273280
// Stop path traversal
274281
authorisedKeysPath := filepath.Join(usersKeysDir, filepath.Join("/", filepath.Clean(conn.User())))
275282
perm, err = CheckAuth(authorisedKeysPath, key, remoteIp, false)
276-
if err == nil {
283+
if err == nil && !isUntrustWorthy {
277284
perm.Extensions["type"] = "user"
278285
perm.Extensions["privilege"] = "0"
279286

280287
return perm, err
281288
}
282289

283290
if err != ErrKeyNotInList {
284-
return nil, fmt.Errorf("user (%s) denied login: %s", strconv.QuoteToGraphic(conn.User()), err)
291+
err = fmt.Errorf("user (%s) denied login: %s", strconv.QuoteToGraphic(conn.User()), err)
292+
if isUntrustWorthy {
293+
err = fmt.Errorf("user (%s) denied login: %s: cannot connect users via pivoted server port (may result in allow list bypass)", strconv.QuoteToGraphic(conn.User()), err)
294+
}
295+
296+
return nil, err
285297
}
286298

299+
// not going to check isUntrustWorthy down here as these are often the reason we're pivoting into a place anyway
300+
287301
//If insecure mode, then any unknown client will be connected as a controllable client.
288302
//The server effectively ignores channel requests from controllable clients.
289303
perms, err := CheckAuth(authorizedControlleeKeysPath, key, remoteIp, insecure)

internal/server/webserver/buildmanager.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ type BuildConfig struct {
4545
Garble bool
4646
DisableLibC bool
4747
RawDownload bool
48+
UseHostHeader bool
4849

4950
WorkingDirectory string
5051

@@ -87,6 +88,7 @@ func Build(config BuildConfig) (string, error) {
8788
var f data.Download
8889
f.WorkingDirectory = config.WorkingDirectory
8990
f.CallbackAddress = config.ConnectBackAdress
91+
f.UseHostHeader = config.UseHostHeader
9092

9193
filename, err := internal.RandomString(16)
9294
if err != nil {

internal/server/webserver/webserver.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ func buildAndServe(autogeneratedConnectBack bool) http.HandlerFunc {
8686
if linkExtension != "" {
8787

8888
host := DefaultConnectBack
89-
if autogeneratedConnectBack {
89+
if autogeneratedConnectBack || f.UseHostHeader {
9090
host = req.Host
9191
}
9292

0 commit comments

Comments
 (0)