Satisfy opensvc requirements #97
Conversation
To limit the client acceptance period duration. If tty-share is executed on-demand to serve a console to a remote user, we want to limit the duration the tty server will wait for the client to connect. If the requester doesn't use the tty he requested, leaving the server running is both a security liability (brute force url forging attack) and a undue resource usage (port, memory, cpu).
Set a maximum server sessions length. Refuse new sessions
when the limit is reached, and log this warning:
WARN[0822] All seats used (1)
With this patch, when the greet period expires, the number of
seats is set to -1, and no new clients are accepted. In this
case, the log show:
WARN[0822] No longer accepting clients
Allocate a random free port.
Example:
$ ./tty-share -command /bin/sh -public -tty-proxy console:3456 -listen localhost:0 -no-tls
public session: https://console.opensvc.com:443/s/kzWFYPNj7VT2zc8Oe4XcqOJ1oFFJsRSNPPAxd1KZCOaNG7TvISxZ5898KDl0xCmM3SE/ │
local session: http://127.0.0.1:34057/s/local/ │
To workaround self-signed certs validation errors. Takes effect both for client and proxy connections.
So the TUI or webapp that executed the tty-share client can reliably decide to prompt for insecure mode activation (-k).
|
Hi, I'd appreciate your feedback about this PR. You seemed supportive of our effort so far ;) |
|
Hey @cvaroqui ! Thank you for your PR. Unfortunately I've been very busy these days, but I hope to find some time in the coming period to have a closer look. However, at a first look it seems ok. Is it blocking you with anything? |
|
Thank you for the update. No hurry, our early testers can use the build from cvaroqui/tty-share until upstream gets the feature set. I just needed confirmation we have no roadblock so far. |
server/server.go
Outdated
|
|
||
| // When a connection ends, close the server if | ||
| // there are no more connections and we can't accept new ones. | ||
| if server.config.Seats < 0 && server.session.ttyProtoConnections.Len() == 0 { |
There was a problem hiding this comment.
If think the ttyProtoConnections linked list is not thread safe, so reading from it here (and in the other places) might not be safe while the list can be modified (connections added or removed from the list).
There was a problem hiding this comment.
Good catch. I pushed a proposal.
The session already has a RWMutex, use it to protect the List.Len() call.
3 participants
Security:
With
-greet-timeout 5s -seats 1tty-share will accept clients for a limited time and accept only one client.For backward compat,
-greet-timeout 0smeans unrestricted acceptance period-seats 0means unrestricted client numberWith
-ktty-share will accept using a proxy with an invalid certificate, both in server and client mode. If not set, make tty-share exit with a dedicated code (2), so callers can take decision on that exit code without having to parse stderr.Feature:
With
-listen :0the tty-share server port is auto-allocated. Which make it easier for our API to execute many tty-share concurrently.With
-greet-timeoutset, the tty-share process will terminate automatically when the last client disconnects.Note:
I noticed firefox closes the connection after 170s of idling in the console.
Chrome does not, and the connection dies after a much longer timeout set in the haproxy or nginx.
Did you oberve that too ? If so, did you investigate further ?
Thank you for sharing this excellent tool.