Skip to content
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

Restrict outbound clipboard #1298

Merged

Conversation

jaroslaw-osmanski
Copy link

@jaroslaw-osmanski jaroslaw-osmanski commented Feb 26, 2019

This patch adds configuration option to xrdp-chansrv that turns copying from the session to clipboard.

@metalefty
Copy link
Member

Awesome! Good work!

@metalefty
Copy link
Member

Existing codes use environment variable to pass config parameter. For example, CHANSRV_LOG_PATH.
Of course environment variable is not the best way but consider using environment variable or let's discuss.

@jaroslaw-osmanski
Copy link
Author

jaroslaw-osmanski commented Mar 1, 2019

Should env be the only way of setting restriction on the clipboard or additional method? In my opinion config is less surprising. @metalefty

@metalefty
Copy link
Member

One unacceptable point is chansrv is opening and reading sesman's config file sesman.ini. Please don't do that. Then you'll see how clipboard restriction flag should be passed sesman to chansrv.

@metalefty
Copy link
Member

Adding RestrictOutboundClipboard parameter to sesman.ini is OK. It's not surprising at all. The parameter must be fetched by sesman because it is sesman.ini.

@jaroslaw-osmanski
Copy link
Author

OK. Sesman gathers info from sesman.ini and then passes information about restriction to chansrv, right?

@jaroslaw-osmanski
Copy link
Author

chnsrv is already reading sesman.ini here: https://github.com/neutrinolabs/xrdp/blob/devel/sesman/chansrv/chansrv.c#L1639
So it is config.c that shouldn't be used?

@metalefty
Copy link
Member

Indeed. That should be fixed but not at this time. I'll change it later.

I'm thinking of this. Similar thing is done as CHANSRV_LOG_LEVEL to set chansrv log level. The difference is setting environment variable directly or via config parameter.

sesman

  1. sesman read if RestrictOutboundClipboard is on/off
  2. sesman sets env var something like RESTRICT_OUTBOUND_CLIPBOARD to something like 1, yes, true
  3. start chansrv with that env var

chansrv

  1. read the env var at startup
  2. if set to 1, restrict outbound clipboard

@jaroslaw-osmanski
Copy link
Author

@metalefty Can I do something more to improve the code?

@metalefty
Copy link
Member

@jaroslaw-osmanski Give me some time to test.

sesman/chansrv/chansrv.c Outdated Show resolved Hide resolved
sesman/config.h Outdated Show resolved Hide resolved
sesman/session.c Outdated Show resolved Hide resolved
sesman/config.c Outdated Show resolved Hide resolved
@metalefty
Copy link
Member

I commented on some style & format issues but logic looks OK. After some test, I'll merge. I'd appreciate if you fix style & format before I merge.

@jaroslaw-osmanski
Copy link
Author

@metalefty Fixed style and format

@metalefty
Copy link
Member

I'm testing this today and will be merged shortly. Sorry for the delay and thank you very much for your work!

@metalefty
Copy link
Member

Tested. Clipboard text/image/file LGTM.

@metalefty metalefty merged commit ec05d42 into neutrinolabs:devel Mar 20, 2019
@black1220
Copy link

Can I only pass text copy&paste and limit file transfer ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants