-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Session allocation policy - Connection setting not useful? #2239
Comments
Thank you for the question. The meaning of was basically to open a new session for each user connection. If you are not lucky with this, you can use . |
Thank you for your answer. What would you then do with the existing session(s) in that model? It seems you could never connect to them at all. Is that right? |
Correct, they're kind of lost but still can run some app's or commands. You might want to set the KillDisconnected, DisconnectedTimeLimit or IdleTimeLimit in sesman.ini |
OK - thanks for the explanation. It seems like an odd kind of use-case to me, but I'm sure a lot of things I do seem odd to other people! It's very easy to support. In fact, it doesn't even need to be combined with the other options, as if it's in any combination. My main reason for asking this question, incidentally, is to tidy up the sesman code in this area. I'd also like to make it easier for other contributors to provide PRs like #2099 which cater for other use-cases the team haven't thought of. How does this look to you as a modified interface for the next major version? ; Policy - session allocation policy
;
; Type: enum [ "Default" | "Separate" | Combination from {UBDIN} ]
; "Default" Same as UB
; "Separate" All sessions are separate. Sessions can never be rejoined,
; and will need to be cleaned up manually, or automatically
; by setting other sesman options.
;
; Combination options:-
; U Sessions are separated per user
; B Sessions are separated by bits-per-pixel
; D Sessions are separated by initial display size
; I Sessions are separated by IP address
; N Sessions are separated by client name sent by the client
;
; The options U and B are always active, and cannot be de-selected.
Policy=Default |
People using 'C' will have to reconfigure Xrdp. Hard to say how many are using option 'C'. Needs at least a release note, pointing to the configuration change. |
Yes - agreed. We've got other config changes in the next major version, including a move to Unix Domain Sockets. The old config files will work, but will generate warnings. I can add a warning that 'C' is replaced with 'Separate'. Would that help do you think? In all the time I've been triaging faults, few people have been changing this option from the default, and no-one has mentioned the 'C' option at all. |
Because it's a background service, it would be very helpful to have a logged warning. Warning will also help you, to reduce the number of support issues. |
I'll add a warning. |
The connected client is currently described in two places in the xrdp_client_info structure:- 1) In the connection_description field. This was introduced as field client_ip by commit d797b2c for xrdp v0.6.0 2) In the client_addr and client_port fields introduced by commit 2536946 for xrdp v0.8.0 This commit unifies these two sets of fields into a single set of fields describing the connection IP and port (for AF_INET/AF_INET6 connections only) and a connection description for all connection types. The code in os_calls to provide client logging has been simplified somewhat which should make it easier to add new connection types (e.g. AF_VSOCK). The old connection_description field used to be passed to sesman to inform sesman of the IP address of the client, and also to provide a string for 'C' field session policy matching. 'C' field session policy matching does not actually need this string (see neutrinolabs#2239), and so now only the IP field is passed to sesman.
The connected client is currently described in two places in the xrdp_client_info structure:- 1) In the connection_description field. This was introduced as field client_ip by commit d797b2c for xrdp v0.6.0 2) In the client_addr and client_port fields introduced by commit 2536946 for xrdp v0.8.0 This commit unifies these two sets of fields into a single set of fields describing the connection IP and port (for AF_INET/AF_INET6 connections only) and a connection description for all connection types. The code in os_calls to provide client logging has been simplified somewhat which should make it easier to add new connection types (e.g. AF_VSOCK). The old connection_description field used to be passed to sesman to inform sesman of the IP address of the client, and also to provide a string for 'C' field session policy matching. 'C' field session policy matching does not actually need this string (see neutrinolabs#2239), and so now only the IP field is passed to sesman.
The connected client is currently described in two places in the xrdp_client_info structure:- 1) In the connection_description field. This was introduced as field client_ip by commit d797b2c for xrdp v0.6.0 2) In the client_addr and client_port fields introduced by commit 2536946 for xrdp v0.8.0 This commit unifies these two sets of fields into a single set of fields describing the connection IP and port (for AF_INET/AF_INET6 connections only) and a connection description for all connection types. The code in os_calls to provide client logging has been simplified somewhat which should make it easier to add new connection types (e.g. AF_VSOCK). The old connection_description field used to be passed to sesman to inform sesman of the IP address of the client, and also to provide a string for 'C' field session policy matching. 'C' field session policy matching does not actually need this string (see neutrinolabs#2239), and so now only the IP field is passed to sesman.
The connected client is currently described in two places in the xrdp_client_info structure:- 1) In the connection_description field. This was introduced as field client_ip by commit d797b2c for xrdp v0.6.0 2) In the client_addr and client_port fields introduced by commit 2536946 for xrdp v0.8.0 This commit unifies these two sets of fields into a single set of fields describing the connection IP and port (for AF_INET/AF_INET6 connections only) and a connection description for all connection types. The code in os_calls to provide client logging has been simplified somewhat which should make it easier to add new connection types (e.g. AF_VSOCK). The old connection_description field used to be passed to sesman to inform sesman of the IP address of the client, and also to provide a string for 'C' field session policy matching. 'C' field session policy matching does not actually need this string (see neutrinolabs#2239), and so now only the IP field is passed to sesman.
The connected client is currently described in two places in the xrdp_client_info structure:- 1) In the connection_description field. This was introduced as field client_ip by commit d797b2c for xrdp v0.6.0 2) In the client_addr and client_port fields introduced by commit 2536946 for xrdp v0.8.0 This commit unifies these two sets of fields into a single set of fields describing the connection IP and port (for AF_INET/AF_INET6 connections only) and a connection description for all connection types. The code in os_calls to provide client logging has been simplified somewhat which should make it easier to add new connection types (e.g. AF_VSOCK). The old connection_description field used to be passed to sesman to inform sesman of the IP address of the client, and also to provide a string for 'C' field session policy matching. 'C' field session policy matching does not actually need this string (see neutrinolabs#2239), and so now only the IP field is passed to sesman.
The connected client is currently described in two places in the xrdp_client_info structure:- 1) In the connection_description field. This was introduced as field client_ip by commit d797b2c for xrdp v0.6.0 2) In the client_addr and client_port fields introduced by commit 2536946 for xrdp v0.8.0 This commit unifies these two sets of fields into a single set of fields describing the connection IP and port (for AF_INET/AF_INET6 connections only) and a connection description for all connection types. The code in os_calls to provide client logging has been simplified somewhat which should make it easier to add new connection types (e.g. AF_VSOCK). The old connection_description field used to be passed to sesman to inform sesman of the IP address of the client, and also to provide a string for 'C' field session policy matching. 'C' field session policy matching does not actually need this string (see neutrinolabs#2239), and so now only the IP field is passed to sesman.
sesman.ini contains the following section for the session allocation policy:-
The above is the latest, but little has changed since xrdp v0.9.1 when this section was introduced as part of commit 1934c9e
The problem I've got is the 'connection' setting. A 'connection' in this context is a string created by common/os_calls.c:g_write_ip_address(). The string is of the form:-
where
<ip_addr>
and<port>
are from the peer connected to xrdp, andsck
is the socket number allocated to the connection by xrdp.If 'C' is in the session allocation policy, the connection string for an existing session has to match the connection string for a reconnection for a reconnect to succeed (code is here). I can't see how this can succeed reliably. Both
<port>
and<sck>
are very likely to be different on a reconnect.So as far as I can tell, the 'Connection' argument to the
Policy
setting is effectively useless. Consequently, I'd like to remove this option for the next major release as it seems to have no use cases, and so no-one can be using it. It can usefully be replaced with the client name - see #2099Am I misunderstanding something here?
@fpaquet - am I correct in thinking you had something to do with 1934c9e? If so, I'd appreciate you telling me what I'm misunderstanding, or what your actual intentions were for this setting.
Thanks for any enlightenment.
The text was updated successfully, but these errors were encountered: