Bug 5141 - recommending thinlinc-login for preventing shell access is confusing
Summary: recommending thinlinc-login for preventing shell access is confusing
Status: REOPENED
Alias: None
Product: ThinLinc
Classification: Unclassified
Component: Other (show other bugs)
Version: trunk
Hardware: PC Unknown
: P2 Normal
Target Milestone: LowPrio
Assignee: Pierre Ossman
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-05-13 14:24 CEST by Pierre Ossman
Modified: 2016-09-06 10:27 CEST (History)
0 users

See Also:
Acceptance Criteria:

Attachments
Combined patch of this work (6.32 KB, patch)
2016-08-24 17:03 CEST, Peter Åstrand 		Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Description Pierre Ossman cendio 2014-05-13 14:24:46 CEST 
Normally you set a user's shell to /bin/false when you want to prevent them from getting a shell. This however also prevents graphical logins, locally as well as in ThinLinc.

Some customers however want to just prevent shell access, but not ThinLinc. So to solve this we hacked thinlinc-login so that it could be set as a user's shell, and we documented this in the TAG.

This is not a very clean solution though. First of all it means extra magical argument parsing in thinlinc-login. It is also not very future proof. What if something else in ThinLinc requires a working shell? Bug 5099 is one such example. We'd have to keep adding cruft to thinlinc-login for such things.

We should create something better for this use case that is more explicit, and can more easily be extended in the future. E.g. a "noshell" command that would be for this specific use case, and nothing else. It would only allow the things needed for ThinLinc, and return false for any other invocations.

We could also let it integrate with OpenSSH's ForceCommand, for when users want to allow local shells, but not remote ones.
Comment 6 Pierre Ossman cendio 2016-07-12 13:28:41 CEST 
All done.

Tester should verify:

 - documentation
 - normal shell
 - noshell as user's shell
 - thinlinc-login as user's shell
 - all three above with ForceCommand set
Comment 8 Peter Åstrand cendio 2016-08-09 10:00:06 CEST 
Reopening for discussion about alternate solution.
Comment 9 Peter Åstrand cendio 2016-08-24 17:03:07 CEST 
Created attachment 733 [details]
Combined patch of this work

The attached patch is the combined work on this bug. Generated with:

/home/astrand/bin/svn-total-diff r31552 r31553 r31555 r31556 r31557 r31558
Comment 10 Pierre Ossman cendio 2016-08-26 09:08:52 CEST 
noshell vs thinlinc-login:

Pro:

  - Keeps this somewhat odd functionality contained in a separate place

  - A more clear and separate interface for customers that doesn't tangle it up with our IPC mechanism (which might change in the future)

Con:

  - Another script

  - thinlinc-login will still have to have some code related to this during a migration period
Comment 11 Peter Åstrand cendio 2016-08-29 11:11:27 CEST 
(In reply to comment #10)
> noshell vs thinlinc-login:

>Pro:
>
>  - Keeps this somewhat odd functionality contained in a separate place
>
>  - A more clear and separate interface for customers that doesn't tangle it up
>with our IPC mechanism (which might change in the future)

Is this an advantage for us or our customers? IMHO, I believe customers will just find it more confusing with two scripts. After all, "you need to be able to execute this program in order to run ThinLinc" is very close to "If you can only execute this program, you can only run ThinLinc"...


> Con:
> 
>   - Another script
> 
>   - thinlinc-login will still have to have some code related to this during a
> migration period

- noshell is a poor name for something that allows executing a full desktop environment, plus xstartup in login mode, which executes personal RC files etc. 

- Existing customers which uses this functionality needs to migrate from thinlinc-login to noshell

- Bash is a poor language for these types of programs; worse than Python


But the biggest problem, as I see it, is:

- noshell will hardcode configuration files names which can be changed by the customer

So if we should keep noshell, perhaps it should be marked as a config file as well?
Note You need to log in before you can comment on or make changes to this bug.