Bug 37 - Shadowing confirmation
Summary: Shadowing confirmation
Status: CLOSED FIXED
Alias: None
Product: ThinLinc
Classification: Unclassified
Component: Other (show other bugs)
Version: 1.0
Hardware: PC Linux
: P2 Enhancement
Target Milestone: 4.9.0
Assignee: Henrik Andersson
URL:
Keywords: derfian_tester, focus_shadowing, ossman_tester, relnotes, samuel_tester
Depends on: 1034
Blocks: 6120
  Show dependency treegraph
 
Reported: 2002-06-12 16:06 CEST by Anders Subotic
Modified: 2018-05-31 10:29 CEST (History)
5 users (show)

See Also:
Acceptance Criteria:


Attachments
Video showing the gap where nothing is shown (287.61 KB, video/webm)
2017-10-24 12:31 CEST, Samuel Mannehed
Details

Description Anders Subotic cendio 2002-06-12 16:06:50 CEST
Hantering av skuggning (shadow) för att hjälpa användare: Stöd i VSM. Ev.
möjligheter för användaren att godkänna, låsa ute, bryta förbindelse etc. Se
även punkten om Webmin.

Develop support for shadowing in VSM so that users can get help. Possibly, the
user should be able to accept, deny and break a connection to the client
machine. This is related to webmin issues.
Comment 1 Peter Åstrand cendio 2002-08-06 14:35:27 CEST
Partially fixed. The new Webmin GUI supports shadowing. What's left is:

* Investigate if it's smarter to use the applet option "Open new Window",
instead of using a new browser window. 

* Add support for "View only" mode. Add a checkbox next to the button in the GUI
for this. 

Maybe we also want to give the user some way of feedback of shadowing. Maybe he
should be able to deny shadowing. 
Comment 2 Peter Åstrand cendio 2002-09-10 10:57:18 CEST
The two "maybe":s in the last sentence should be removed. The user *should* get
some feedback, and there *should* be some way for the user to confirm the
shadowing (configurable). 
Comment 3 Peter Åstrand cendio 2004-01-08 15:01:15 CET
VNC 3.3.7 includes some of this stuff:

"VNC 3.3.7 includes the QueryConnect option, which allows the user to be
prompted to accept connections."
Comment 4 Peter Åstrand cendio 2004-01-08 15:01:43 CET
"http://www.realvnc.com/winvnc.html#11

Look at QuerySetting and AuthHosts specifically"
Comment 5 Peter Åstrand cendio 2005-01-31 16:35:52 CET
>"VNC 3.3.7 includes the QueryConnect option, which allows the user to be
>prompted to accept connections."

The QueryConnect stuff is only for WinVNC; Xvnc doesn't support it, as far as I
can tell. It seems like xf4vnc (http://xf4vnc.sourceforge.net/) is the only
implementation that actually supports VNC connection confirmation. The source
for vncevent.c even says:

"Sample application to demonstrate the features of the 'useraccept' option in
conjunction with xf4vnc's unique VNC extension to deliver events when someone is
connecting or disconnecting"

We could probably re-use some of this work. Our current shadowing architecture
has additional problems, though: vsmserver is currently delivering the real
session password to the client, and this happens before any VNC communications
has occured. This is not good. A shadow user should not have knowledge of the
users session password. We could solve this problem by altering things slightly:

* The vsmserver returns (instead of the real session password) some kind of
magic string, perhaps crypt(user+sesskey, sesskey), that is, the username of the
shadower concatenated with the sessionkey, and both these two encrypted with the
sessionkey. 

* Xvnc can decrypt the magic string. If the sessionkey matches, Xvnc knows that
vsmserver has authenticated this shadowing. Xvnc can call an external
application that shows a yes/no confirmation, including the username of the
person that is trying to shadow. 

We could add a comment field as well. 

Since tlclient never actually uses the session key, this change should be fairly
easy. As far as I can tell, the only main problem is that the sessionkey is
limited to 8 bytes currently. 
Comment 6 Peter Åstrand cendio 2005-03-09 13:42:50 CET
RealVNC 4.1 supports QueryConnect with Xvnc. 
Comment 7 Peter Åstrand cendio 2007-02-06 11:20:18 CET
I've been thinking some more about this. Here's my idea:

* We are porting the QueryConnect stuff to our Xvnc, but it should only be used
if Xvnc determines that the connection is a shadow connection. 

* When we are shadowing, tlclient should *not* get the real VNC password.
Instead, vsmserver returns a hash of the password. 

* When Xvnc authenticates the vncviewer, it first tries the "real" password. If
this fails, it tries the hash of the password. If this succeeds, the connection
is a shadowing. 

A very rough time estimation would be:

* Port QueryConnect to our Xvnc: 16h

* Select and implement a hash function, both in vsmserver and all our vncviewers: 24

* Deal with backward compatibility: 8h
Comment 8 Peter Åstrand cendio 2007-02-06 16:04:23 CET
If we implement the idea in comment #7, we probably won't need to change the VNC
protocol. One problem might be that we cannot transfer information about which
user is trying to shadow, nor any messages. One strange thing is that RealVNCs
vncconfig actually says "(anonymous)" or something like that when using
QueryConnect, so perhaps they have some extension or preparation for
transmitting the user name. 
Comment 10 Pierre Ossman cendio 2013-04-08 09:31:07 CEST
The QueryConnect code seems to be present in Xvnc as far as I can tell. Were the previous comments before the RealVNC 4 upgrade?
Comment 11 Luciano 2015-10-02 16:37:32 CEST
Hi
I am new about Thinlinc (4.4)
The shadow without user explicit confirmation, is not compliant with privacy and with company rules about proper management.
It is important to introduce the confirm upon the notification.
Thanks
Comment 12 Luciano 2015-10-08 19:28:05 CEST
(In reply to comment #11)
> Hi
> I am new about Thinlinc (4.4)
> The shadow without user explicit confirmation, is not compliant with privacy
> and with company rules about proper management.
> It is important to introduce the confirm upon the notification.
> Thanks

I SOLVED WITH BYPASS SOLUTION:
1) user A can show own session password using:
x11vnc -showrfbauth /var/opt/thinlinc/sessions/$U/last/sessionkey
and, can show own session number using:
cat /var/opt/thinlinc/sessions/$U/last/sessnum

So user A can deliver session number and password to 	
authorize user B to share current desktop session A

2) user B to get desktop A, may use:
vncviewer 127.0.0.1:<n.session> -Shared &
and typing password on vncviewer request

That is all
Comment 15 Henrik Andersson cendio 2017-10-10 10:08:38 CEST
The introduction of using QueryConnect will push strings from Xvnc to the client which is shown in a message box. These messages are not translated nor any infrastructure is in place for it yet.

There are three strings that needs translation:

 - When user interactively rejects the connection

 - When a timeout appears waiting for answer to shadowing request

 - When there is no vnc-shadow-notify service listening on event from vncExt
Comment 17 Henrik Andersson cendio 2017-10-10 10:12:59 CEST
To get an insight of this change, read the documentation which describes the new shadowing modes and configuration parameters.
Comment 23 Samuel Mannehed cendio 2017-10-24 12:31:41 CEST
Created attachment 828 [details]
Video showing the gap where nothing is shown

In ask mode, if the shadowee doesn't accept or reject the shadowing request, there is a gap for the shadower of 10 seconds where no spinner or anything is shown. The tlclient (login window) disappears and vncviewer isn't shown yet.

This should be fixed in my opinion.
Comment 24 Samuel Mannehed cendio 2017-10-24 16:12:37 CEST
(In reply to comment #23)
> Created an attachment (id=828) [details]
> Video showing the gap where nothing is shown
> 
> In ask mode, if the shadowee doesn't accept or reject the shadowing request,
> there is a gap for the shadower of 10 seconds where no spinner or anything is
> shown. The tlclient (login window) disappears and vncviewer isn't shown yet.
> 
> This should be fixed in my opinion.

Bug 7070.
Comment 25 Samuel Mannehed cendio 2017-10-24 16:25:42 CEST
A few comments regarding the documentation changes:
---------------------------------------------------


> +    All shadowing requests are rejected. You should set this if
> +    you want to disabled shadowing feature.
                          ^^
Shouldn't be a 'd' there, and a 'the' is also missing. There are a couple of places where "if you want to disabled" is written.

> +      <term>reject</term>
> +
> +      <listitem><para>
> +          All shadowing requests are rejected. You should set this if
> +          you want to disabled shadowing feature.
> +      </para></listitem>

In general I believe we avoid using 'you' and 'your' etc. and instead say for example:

"This should be set in order to disable the shadowing feature."

> +     The above command should be run on all ThinLinc servers in your
> +     cluster.

We have said "the cluster" in other places.

> +      <para>
> +	Shadowing feature is enabled by default and is configured
> +	to ask the user to accept or reject a shadowing request.
> +      </para>
> +

"The shadowing feature"




Release notes:
--------------


> +  The default behaviour of shadowing for an installation has changed
> +  from silent shadowing of a user (no notification), to were the user
> +  is interactivly asked to accept or reject the shadowing request.

"interactively"
Comment 28 Pierre Ossman cendio 2017-11-01 11:14:19 CET
Found a crash:

 - Configure for "ask" mode
 - Close dialog instead of clicking any button

It seems to be related to the timer as we see this output:

> vnc-shadow-notify: shadow request is rejected
> vnc-shadow-notify: shadow request is rejected
> vnc-shadow-notify: request timed out and is automatically rejected.
Comment 29 Henrik Andersson cendio 2017-11-02 09:37:08 CET
(In reply to comment #28)
> Found a crash:
> 
>  - Configure for "ask" mode
>  - Close dialog instead of clicking any button
> 
> It seems to be related to the timer as we see this output:
> 
> > vnc-shadow-notify: shadow request is rejected
> > vnc-shadow-notify: shadow request is rejected
> > vnc-shadow-notify: request timed out and is automatically rejected.

I couldn't reproduce this using nightly build. Maybe it is all related
to the timer which is not removed in the dialog exit callback.
Comment 31 Samuel Mannehed cendio 2017-11-02 17:37:15 CET
(In reply to comment #29)
> (In reply to comment #28)
> > Found a crash:
> > 
> >  - Configure for "ask" mode
> >  - Close dialog instead of clicking any button
> > 
> > It seems to be related to the timer as we see this output:
> > 
> > > vnc-shadow-notify: shadow request is rejected
> > > vnc-shadow-notify: shadow request is rejected
> > > vnc-shadow-notify: request timed out and is automatically rejected.
> 
> I couldn't reproduce this using nightly build. Maybe it is all related
> to the timer which is not removed in the dialog exit callback.

One easily reproduceable effect was to try another shadowing connection after the shadowee closed the dialog for the first request. I can verify that at least this is fixed now after r32855.
Comment 32 Pierre Ossman cendio 2017-11-06 13:59:43 CET
There is some differences between how we used to inform the user:

 - The previous message was much more verbose in telling the user the risks of shadowing.

 - We previously had a notification informing the user when the shadowing ended

The release notes are also out of date. They mention "shadowing_enabled", and also has the wrong default value for "shadowing_mode".
Comment 33 Pierre Ossman cendio 2017-11-06 14:23:05 CET
ERR_SHADOW_DISABLED is still present in some parts of the code.
Comment 34 Pierre Ossman cendio 2017-11-06 14:30:41 CET
Things tested:

 - allowed_shadowers:
   ✓ Empty
   ✓ Other users
   ✓ Current user
 - shadowing_mode:
   - ask
     ✓ yes
     ✓ no
     ✓ close
     ✓ timeout
   - notify
     ✓ ok
     ✓ close
   ✓ silent
   - reject
     ✓ by vsmserver
     ✓ by tl-shadow-notify
 ✓ upgrade from 4.8.0 with notify
 ✓ shadow disconnected session
 ✓ shadowing with 4.4.0 client
Comment 38 Thomas Nilefalk cendio 2017-11-07 10:44:02 CET
(In reply to comment #32)
> There is some differences between how we used to inform the user:
> 
>  - The previous message was much more verbose in telling the user the risks of
> shadowing.

Fixed in r32867.

>  - We previously had a notification informing the user when the shadowing ended

Yes. This has been moved to bug #7076, see bug #7076 comment #1.

> The release notes are also out of date. They mention "shadowing_enabled", and
> also has the wrong default value for "shadowing_mode".

Fixed in r32866.

(In reply to comment #33)
> ERR_SHADOW_DISABLED is still present in some parts of the code.

Fixed in r32865.
Comment 39 Pierre Ossman cendio 2017-11-07 12:52:18 CET
Let's try to get the end notification back before we close this bug.
Comment 43 Samuel Mannehed cendio 2017-11-13 14:28:41 CET
I have verified that the shadowing disconnect notification works well:

* The shadowee gets notified when the shadower disconnects when in 'ask' mode.
* The shadowee gets notified when the shadower disconnects when in 'notify' mode.
* The shadowee does not get notified when in 'silent' mode.
* The shadower does not get notified when the shadowee disconnects.

However, I think we should mention in the documentation that in 'ask' and 'notify' mode the shadowee will be notified on shadower disconnection as well.
Comment 45 Pierre Ossman cendio 2017-11-14 15:27:16 CET
Updated the documentation.
Comment 47 Samuel Mannehed cendio 2017-11-15 12:36:32 CET
Looks good. That means everything is verified for this bug. Closing
Comment 48 Pierre Ossman cendio 2017-11-29 14:32:58 CET
Found a bug: Whenever a shadowing is rejected you get the "end of shadowing" notification.
Comment 49 Pierre Ossman cendio 2017-11-29 14:41:02 CET
We're also not getting any translations.
Comment 53 Samuel Mannehed cendio 2017-12-04 14:11:59 CET
(In reply to comment #48)
> Found a bug: Whenever a shadowing is rejected you get the "end of shadowing"
> notification.

Fixed now.
Comment 54 Samuel Mannehed cendio 2017-12-04 14:29:23 CET
The fix for comment #48 was not sufficient, found and verified a problem. Steps to reproduce a problem with the current code:

 1. configure your shadowing_mode to ask
 2. start a session with user1
 3. connect with shadower1 to shadow user1
 4. accept the shadowing request for shadower1
 5. connect with shadower2 to shadow user1
 6. reject the shadowing request for shadower2
 7. disconnect shadower1
 8. you will now not get a notification for the shadowing disconnection

We probably want to stop Xvnc from sending the event in the cases of rejected shadowing requests instead of just preventing the dialogue from being shown.
Comment 57 Samuel Mannehed cendio 2017-12-05 15:53:55 CET
r32907 fixed comment #49.
r32921 fixes comment #48 and comment #54.
Comment 59 Karl Mikaelsson cendio 2017-12-19 16:07:56 CET
(In reply to comment #57)
> r32907 fixed comment #49.

✓ - got a translated dialog about shadowing.


(In reply to comment #57)
> r32921 fixes comment #48 and comment #54.

✓ - no "shadowing ended" dialog noticed when rejecting shadowing
✓ - got a "shadowing ended" dialog when following steps in #54.

Note You need to log in before you can comment on or make changes to this bug.