If a server has the ability to register on a proxy, the proxy can become aware of servers where discovery via bonjour is not possible.
Furthermore, this allows hole punching in the servers firewall if the server is keeping the connection to the proxy alive. This allows to have client and server behind a NAT and only needs a proxy exposed.
How would a client identify which server it wants to connect to?
The standard way of doing this at the moment is using sqlite auth (#1488), which you can already use today as long as the server is not behind NAT: the servers can add themselves to the sqlite auth database used by the proxy to locate sessions for its users.
To workaround the NAT problem, the servers would need to connect to the proxy instead of the other way around. The connection would have to be authenticated and then kept alive until a client takes over. This is quite similar to #1022.
Add a new command line option so that servers can advertise themselves to remote proxy servers. This would complement wiki/MulticastDNS and make it easier to deploy proxy servers. Can also be used for easy NAT traversal: both server and client can be behind NAT.
Related tickets:
ie:
xpra start --start=xterm \ --register=tcp://proxyusername:proxypassword@proxyip:proxyport/SESSIONID?\ username=optionalclientloginusername&password=optionalclientloginpassword
Issues:
I tried to do changes regarding this ticket, but I got stuck after implementing the command line options.
I suggest using one command line option for the server to register himself at a proxy and one command line option for the proxy to accept registrations.
The option for the server could be like yours above. Multiple --register options could be accepted, each with a list of servers. Connections are made to a random not failing entry in each list. Thus multiple proxies with failover configurations can be configured.
The option for the proxy should include the protocol to listen on.
Should there be a new table containing optionalclientloginusernames and passwords with groups matched to users group?
I tried to do changes regarding this ticket, but I got stuck after implementing the command line options.
Where is your code?
The option for the proxy should include the protocol to listen on.
I think this should be done as per #1160, using chained authentication modules: the last module in the chain would do the registration.
Should there be a new table containing optionalclientloginusernames and passwords with groups matched to users group?
Not sure I understand, I don't think anything should be tied to unix user groups. The solution should be generic.
I attached a patch regarding the command line options. The cmd options are not in the format you mentioned but in the old one.
I also attached a patch what I tried with a new paket handler, but I certainly sure this is not the right way to do it.
Regarding optionalclientloginusernames: I understood that a server willing to register at a proxy can provide credentials (optionalclientloginusernames, optionalclientloginpassword). I wanted to ask if a new table in db/... is needed (e.g. server_users) to store these combinations. Furthermore I thought about the idea, to be able to indicate groups at server_users to be able to present users a list of servers matching by group.
detach
or info
requests are handled)
mod_python
here
add_user
anywhere
tcp://username:password@host:port/?extra_args
), that way authentication will be handled automatically
See also #2261
Here's my plan - feel free to comment:
--proxy-sessions=auth,sys,mdns,register
so the proxy can be told where to get the list of sessions from (and maybe those options can be combined?):
auth
(default) would still use the auth module so we don't break any existing setups
sys
same as what most auth modules currently do: similar to running xpra list
using the same uid supplied by the auth module (usually the user that authenticated)
mdns
without knowing if the server already has clients or not since we can't seem to get TXT record update notifications (see #2187)
register
get the list of sessions from servers that register with the proxy
--bind-XXXX
option), or as part of the authentication module? (ie: access=full,register,info,...
to specify what the authentication gives access to)
--register=tcp://host:port/?options
, it will need options like keepalive and re-connect, we should detect authentication failures and not retry too often in that case
proxy_auth
can be taught about the special "registered" sessions and bypass connect_to
Notes:
(CC was removed by mistake)
Some useful socket functions have been moved:
socket_util
socket_util
to xpra.net
socket_types
mapping, use callback argument instead
accept_connection
code to socket_util
More network related updates that can help here:
Too late for 3.0, will deal with this after #1160 so we can re-use the same bind-XXX syntax and have different capabilities on different ports.
--proxy-sessions=auth,sys,mdns,register
so the proxy can be told where to get the list of sessions from (and maybe those options can be combined?):
Combining seems like a good idea.
- the sessions can be identified using their uuid, and maybe other attributes? the display string is not a good match as multiple servers running on different systems can use the same display number. uids are not usually portable either (not without things like ldap - which we can't require)
The server could send its desired display name, e.g. "Facility 7, Room 5" and forwarded application. Furthermore, the session is tied to a proxy user (from e.g. server_users, see above(and could be shared with a proxy_group)). Clients requesting sessions from the proxy are only getting their accessible sessions, thus the displayname has limited scope.
- do we want to think about load-balancing?
To balance register/list requests? Or to balance connections between client and server in TURN mode?
- how do we select a session easily?
- if more than one is available?
A session has to be selected choosen from prior requested list of accessible sessions.
- only allow registration on some tcp / ws / wss / ssh sockets?
Allow on all specified authenticators?
- support multiple proxies: register with all of them, or the first one, or random, or...
Take a xpraUri[][] as proxies. Server will connect to one of each inner in all outer.
- the proxy needs to keep track of those sessions in memory, return them before or after the ones from the client auth module?
Can they be stored using the Authenticator?
- the proxy servers could also listen for mdns broadcasts and expose those servers
Are mdns broadcast receivable via client packets?
- I would like to re-use all the socket code and authentication configuration options for the server registration (so servers can use ssl / ssh / whatever), but ideally without letting the servers have the same privileges as regular authenticated users (ie: no ability to connect to sessions or start new sessions, just registering) - not sure how to do that yet (comment:4 suggested using a new authentication module, but this would be too difficult to tie with other features: keepalive, etc), should this be configured at the socket level (as part of the
--bind-XXXX
option), or as part of the authentication module? (ie:access=full,register,info,...
to specify what the authentication gives access to)
Is this still a problem?
- the server code will need a new option for telling it to register with the proxy, something like:
--register=tcp://host:port/?options
, it will need options like keepalive and re-connect, we should detect authentication failures and not retry too often in that case
I would consider to specify a list of proxies where keep-alive packet are send to (including a token). Registering using credentials should be a one time (interactive) action to get a token.
Replying to Antoine Martin:
- I would like to re-use all the socket code and authentication configuration options for the server registration (so servers can use ssl / ssh / whatever), but ideally without letting the servers have the same privileges as regular authenticated users (ie: no ability to connect to sessions or start new sessions, just registering) - not sure how to do that yet (comment:4 suggested using a new authentication module, but this would be too difficult to tie with other features: keepalive, etc), should this be configured at the socket level (as part of the
--bind-XXXX
option), or as part of the authentication module? (ie:access=full,register,info,...
to specify what the authentication gives access to)
How much does the patch above hurt your eyes? It seems like an option to divide traffic in the proxy_auth and let the authenticator handle DB/memory specific things. I mocked it up in the proxy_auth function, this could also go in a different class.
Is there any way I can create a branch? Or is uploading patches the way to go?
How much does the patch above hurt your eyes?
The patch is not too bad, but I'm out of time.
I've done some work on generalizing bind
command line options in #2406.
This needs extending.
I rewrote the patch to use a hello request type for register and update requests.
Start a proxy with register/update support:
proxy --tcp-auth=sqlite:filename=userlist.sdb --proxy-sessions=register --bind-tcp=127.0.0.1:10009
Add a session at the proxy which returns a token to update the session:
xpra register tcp://username:password@127.0.0.1:10009/
Start a server and update the session at the proxy:
xpra start --start=xterm --bind-tcp=127.0.0.1:10010 --proxies=tcp://127.0.0.1:10009/update_token=SESSION_TOKEN
I have only tested it with sqlite, it only works with sqlite atm (scripts/server.py:427). It seems verify_auth in the core has to handle the update request and call auth_verified (server_core.py:1670).
Perhaps you can help me with the following:
Can you explain why you need to register
the session first?
Doesn't it make sense to just let the server register with the proxy without needing to register first? What's the benefit?
This registration ability should be an attribute of the authentication modules - see #2424. So you could enable this on a specific port (DMZ), and not on another (WAN), ie:
--bind-tcp=127.0.0.1:10009,auth=file:filename=server-auth,auth=sqlite:filename=userlist.sdb:register=1 \ --bind-tcp=0.0.0.0:10000,auth=sqlite:filename=userlist.sdb:register=0
In this example: the registration requires password authentication using file
auth before you can add entries to the sqlite db, clients connect to port 10000 and only need to authenticate with sqlite
.
The registration would only be implemented in the generic sql backend for now - the other modules would just fail when calling proxy_register
/ proxy_update
.
Not essential, but it would be nice of the server maintained the connection to the proxy:
I've merged the 2 whitespace changes.
I think that most of the changes to sqlauthbase.py
and sqlite_auth.py
would be worth merging without the rest as it would reduce the diff when reviewing. (just without the token
/ session_add
stuff for now)
As for the remainder of your questions, the line numbers don't match trunk?
Replying to Antoine Martin:
Can you explain why you need to
register
the session first?
For authenticated updates (server -> proxy), an authentication mechanism must be present. Since servers are mostly non-interactive (daemon), any credentials need to be stored in the config file or as parameter. Storing a token which can only update one session at the proxy prevents storing usernames/passwords and limits the abilities of stolen tokens.
Doesn't it make sense to just let the server register with the proxy without needing to register first? What's the benefit?
With the DB schema from the patch, one user can have multiple sessions. Only with username/password, no session to update can be identified. If the identifier can be chosen freely by the client, uniqueness is not guaranteed.
Registering is a one-time action per server (upon installing/setting up xpra), updates are sent periodically while the server is running.
This registration ability should be an attribute of the authentication modules - see #2424. So you could enable this on a specific port (DMZ), and not on another (WAN), ie:
--bind-tcp=127.0.0.1:10009,auth=file:filename=server-auth,auth=sqlite:filename=userlist.sdb:register=1 \ --bind-tcp=0.0.0.0:10000,auth=sqlite:filename=userlist.sdb:register=0In this example: the registration requires password authentication using
file
auth before you can add entries to the sqlite db, clients connect to port 10000 and only need to authenticate withsqlite
. The registration would only be implemented in the generic sql backend for now - the other modules would just fail when callingproxy_register
/proxy_update
.
So there is a need for another option which enables registering (and updating) selectively by auth backend.
Not essential, but it would be nice of the server maintained the connection to the proxy:
- the proxy could automatically remove the session when the server disconnects
- the proxy would not need to connect back to the server when a client claims the session, it could just re-use the connection
I agree that the server should keep the connection alive. But the proxy should not delete the session because it is possibly updated later. I thought to fill the "updated" field with a timestamp of the last update, but a flag "currently connected" is also possible.
I reused the sessions table to also store "registered servers".
As for the remainder of your questions, the line numbers don't match trunk?
The line numbers match revision 27551 with 8d6c8c863e1a97162f1c49598ffeb4cfde96e9c5.patch applied.
For authenticated updates (server -> proxy), an authentication mechanism must be present.
Yes, that's why I proposed: --bind-tcp=127.0.0.1:10009,auth=file:filename=server-auth,auth=sqlite:filename=userlist.sdb:register=1
for the socket which is facing the servers. I used file
as an example but you could use sqlite
for that too.
Using tokens makes things more complicated. We have multi-layer authentication support, let's use it.
Storing a token which can only update one session at the proxy prevents storing usernames/passwords and limits the abilities of stolen tokens.
The only gain here is the ability to restrict to one session, not sure how useful that is in practice.
With the DB schema from the patch, one user can have multiple sessions. Only with username/password, no session to update can be identified. If the identifier can be chosen freely by the client, uniqueness is not guaranteed.
OK, or you could just use different username + password combinations for each session.
Registering is a one-time action per server (upon installing/setting up xpra), updates are sent periodically while the server is running.
Hmm. Then if the server drops off, the session will be unreachable. And the proxy has to make a new connection to the server.
The registration would only be implemented in the generic sql backend for now - the other modules would just fail when calling proxy_register / proxy_update.
So there is a need for another option which enables registering (and updating) selectively by auth backend.
Yes. It is much safer that way.
The line numbers match revision 27551 with 8d6c8c863e1a97162f1c49598ffeb4cfde96e9c5.patch applied.
Can you please post easy to parse links, maybe just code extracts.
And please submit things separately: most of the DatabaseBaseQueries
could be merged without the rest. (though I do wonder the point of having QueriesBase
- doubles the size of the code without any visible benefits)
Replying to Antoine Martin:
And please submit things separately: most of the
DatabaseBaseQueries
could be merged without the rest. (though I do wonder the point of havingQueriesBase
- doubles the size of the code without any visible benefits)
QueriesBase
only ties a connection between the sql variants so that I (the developer) do not forget to implement used sql commands in all variants.
Note that I also added a db.commit() since without this, multiple sqlite commands failed on me at the second command.
Replying to Antoine Martin:
Using tokens makes things more complicated. We have multi-layer authentication support, let's use it. The only gain here is the ability to restrict to one session, not sure how useful that is in practice.
IMHO security is another big gain. If credentials are used to update sessions, they are stored in plain text in the servers config (e.g. to be used while starting automatically on boot). These credentials are designed to be used as a login. By introducing a conceptual difference of credentials and tokens, stored values do not need to be credentials. A token is saved and used to update the session while credentials are not saved and used only while interactively registering a server.
Optionally, if the user decides to store credentials, the server could itself register at the proxy atomatically and de-register itself upon shutdown (dynamically). The benefit is a clean sessions table but it makes e.g. multiple user more complex.
OK, or you could just use different username + password combinations for each session.
I like the idea of splitting users and sessions since it enables that a user can access multiple sessions. An existing user is also required to register a session dynamically. Am I overlooking a disadvantage?
Replying to Antoine Martin:
--bind-tcp=12[...],auth=sqlite:filename=userlist.sdb:register=1
This seems to require at least the patch above. I would also add update=1
.
Replying to brief:
Perhaps you can help me with the following:
How do I get options for a client request (line 11) to not reuse the ones from server start? What should they contain except the token? Is this method of creating a client generally correct?
1 def do_run_mode(script_file, error_cb, options, args, mode, defaults): 2 [...] 3 if not (mode=="proxy" and supports_proxy) and options.proxies: 4 def update_proxies(): 5 6 ips = options.bind_tcp 7 8 for proxy in options.proxies: 9 # try to update proxy with token 10 11 update_options = options 12 update_display_desc = pick_display(error_cb, update_options, [proxy]) 13 14 from xpra.client.gobject_client_base import UpdateProxyClient 15 update_app = UpdateProxyClient(update_options, ips) 16 17 try: 18 connect_to_server(update_app, update_display_desc, udate_options) 19 except Exception: 20 update_app.cleanup() 21 raise 22 23 24 do_run_client(update_app) 25 add_when_ready(update_proxies)
How is it possible in verify_auth() in server_core.py to access the display_desc which is added on client requests (main.py:1685: app.display_desc = display_desc)?
I listed three sql variants with support of tokens and multiple user per sessions.
If the table sessions should only contain available sessions, Variant 3 cannot be used.
Furthermore, Variant 2 seems to be simpler and cleaner than Variant 1.
-------------- ----------- ----------- | SESSION_USER | | SESSIONS | | USERS | ______________ ___________ ___________ | session_id | <--> | id | |-> | id | <--> | user_id | | display | | | user | -------------- | | | | password | | | | ----------- | | | | | | --------------- | | | | TOKEN_USER | ----------- | | | _______________ | TOKENS | | | |-> | user_id | ___________ | datetime | | token_id | <--> | id | <--> | token_id | --------------- | value | ----------- -----------
-------------- ----------- ----------- | SESSION_USER | | SESSIONS | | USERS | ______________ ___________ ___________ | session_id | <--> | id | |-> | id | <--> | user_id | | display | | | user | -------------- ----------- | | password | | ----------- ------------------- | | TOKEN_SESSIONS | | --------------- ___________________ | | TOKEN_USER | ----------- | id | | _______________ | TOKENS | | | |-> | user_id | ___________ | datetime | | token_id | <--> | id | <--> | token_id | --------------- | value | ------------------- -----------
-------------- ----------- ----------- | SESSION_USER | | SESSIONS | | USERS | ______________ ___________ ___________ | session_id | <--> | id | | id | <--> | user_id | | display | | user | -------------- | token | | password | | datetime | ----------- -----------
*in normal mode:
if a unique "token" (e.g. UUID) already exists at server, it could be used as token.
Otherwise, a token can be requested to use persistent user assignments.
I added my current state of work.
This patch should be split up in multiple patches but shows the idea.
In an environment with server and client behind different NATs, if the right protocol picks up the packets at the proxy (since there are multiple), a direct connection between server and client is established. In the current code, it stays in this state without activating the 'real' client (screen transmission, ...).
It would help to have a description of the changes and the intent.
Review:
self.sessions = [(uid, gid, displays, env_options, session_options)]
should become [(uid, gid, display, env_options, session_options) for each display]
- this should be a separate change from get_session_by_token
and parse_session_datas
. AFAICT, this could be merged easily enough for 4.1, including the unit test changes, etc
assert len(passwords) in 1
- why?
print("success, found password")
- never call print from server code
if auth_sessions:
with if len(auth_sessions) > 0:
?
# TODO selection session
- this is crucial and should be moved to a method that could eventually do more than just fine the matching display
sessions_info
changes - not sure what that does, not looked at it carefully - should be split up as this is unrelated?
listenSocket
class, though I'm not sure I want to deal with the extra abc
dependency for packaging (minor detail) - this is probably too late to merge into 4.1, or maybe not..
SQLAuthenticator
should be split up: first the move to QueriesBase
(preferably without abc
) without the token bits
hole_punch_authenticator
needs an explanation
opts.local_port = client_proto._conn.local[1]
- what does this do?
if hasattr(socket, 'SO_REUSEPORT'): listener.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEPORT, 1)
proxy_sessions
and hole_punch
in server_core.py
is unlikely to fly
socket_connect(dtype, host, port, reuse=False,local_port=0)
- can you explain why we need to re-use the local_port
and how it's done?
hole_punch
, hp_local_server_address
changes to the proxy instance classes is unlikely to fly
hp-client-status
/ hp-server-status
- what does this do?
All in all, the token stuff adds quite a lot of code and complicates things quite a bit too. Then the proxy still needs to connect back to the server when the client connects, so this doesn't help with NATed servers either..
Replying to Antoine Martin:
It would help to have a description of the changes and the intent.
The intention is to enable server and client behind NATs.
Its implemented like this (also have a look at the attached ods document):
Review:
- if all authentication modules now return a list of sessions (which makes sense) then I think that
self.sessions = [(uid, gid, displays, env_options, session_options)]
should become[(uid, gid, display, env_options, session_options) for each display]
- this should be a separate change fromget_session_by_token
andparse_session_datas
. AFAICT, this could be merged easily enough for 4.1, including the unit test changes, etc
assert len(passwords) in 1
- why?
get_passwords
seems to return multiple passwords so this checks if multiple users are present. perhaps get_passwords
should be replaced by get_password
.
- why replace
if auth_sessions:
withif len(auth_sessions) > 0:
?
get_sessions
returns an empty array on sqlite in this implementation. this could return none or the datatype in all authenticators could be changed to array.
# TODO selection session
- this is crucial and should be moved to a method that could eventually do more than just fine the matchingdisplay
perhaps the authenticator should decide about sessions and display?
sessions_info
changes - not sure what that does, not looked at it carefully - should be split up as this is unrelated?
with this change, multiple session are returned on get_info (related to get_sessions
)
- the
hole_punch_authenticator
needs an explanation
hole_punching can be enabled/disabled at client, server and authentication module (proxy). hole_punching_authenticator saves the value from the authenticator which is used in proxy_instance:run() to decide if hole punching should start. if hole punching is started, the proxy does not start a connection to the server but reuses a previous "update connection" from the server.
opts.local_port = client_proto._conn.local[1]
- what does this do?
this line can be removed :)
I previously used connect_to(disp_desc, opts) instead of preserving the connection. if local_port is set, client requests are send from the local_port. I thought this is useful if no preserved connection from the server is present but the servers firewall still accept packets originating from the proxies ip:port.
- why do we want / need this?
if hasattr(socket, 'SO_REUSEPORT'): listener.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEPORT, 1)
I think this is needed on non-windows to reuse sockets with the same port properly.
socket_connect(dtype, host, port, reuse=False,local_port=0)
- can you explain why we need to re-use thelocal_port
and how it's done?
If reuse is set, socket.SO_REUSEADDR is set. if a port is given, the socket binds to it so that multiple sockets exist on the port. I found no problems using the sockets if only one type of protocol (server/client) is used.
In an environment with server and client behind different NATs, if the right protocol picks up the packets at the proxy (since there are multiple), a direct connection between server and client is established. In the current code, it stays in this state without activating the 'real' client (screen transmission, ...).
With this implementation, sometime an error occurs: unknown or invalid packet type at the proxy server.
hp-client-status
/hp-server-status
- what does this do?
While server and client are hole punching, they send status updates to the proxy.
I tested with this network:
server router proxy router client 172.20.42.1 172.20.42.3 192.168.178.42 192.168.178.25 192.168.178.49 10.0.0.3 10.0.0.1
and this router config based on iptables:
# Delete all sudo iptables -P INPUT ACCEPT sudo iptables -P FORWARD ACCEPT sudo iptables -F sudo iptables -F -t nat # Allow traffic from internal network sudo iptables -A INPUT -i enp0s8 -j ACCEPT # Allow local sudo iptables -A INPUT -i lo -j ACCEPT # Allow ssh traffic from external sudo iptables -A INPUT -i enp0s3 -p tcp -m tcp --dport 22 -j ACCEPT # allow local related packets sudo iptables -A INPUT -i enp0s3 -m state --state RELATED,ESTABLISHED -j ACCEPT # Drop everything by default sudo iptables -P INPUT DROP # NAT sudo iptables -A FORWARD -i enp0s3 -o enp0s8 -m state --state RELATED,ESTABLISHED -j ACCEPT sudo iptables -A FORWARD -i enp0s8 -o enp0s3 -s [10.0.0.0/24 or 172.20.42.0/24] -j ACCEPT sudo iptables -t nat -A POSTROUTING -o enp0s3 -s [10.0.0.0/24 or 172.20.42.0/24] -j MASQUERADE sudo iptables -P FORWARD DROP
hp
patch is way too big and includes some commented out code, needs splitting up - and as I said before, we can't pollute the interface with hope-punching concerns
this ticket has been moved to: https://github.com/Xpra-org/xpra/issues/2125