X2Go Bug report logs - #1136
Multiple issues with x2gousbmount

Full log

🔗 View this message in rfc822 format

MIME-Version: 1.0
X-Mailer: MIME-tools 5.507 (Entity 5.507)
X-Loop: owner@bugs.x2go.org
From: owner@bugs.x2go.org (X2Go Bug Tracking System)
Subject: Bug#1136 closed by X2Go Release Manager X2Go Release Manager
 <git-admin@x2go.org> (X2Go issue (in src:x2gothinclient) has been marked
 as closed)
Message-ID: <handler.1136.c.15437207968512.notifdone@bugs.x2go.org>
References: <20181202031917.D69F75DAF7@ymir.das-netzwerkteam.de>
X-X2go-PR-Keywords: pending
X-X2go-PR-Message: they-closed 1136
X-X2go-PR-Package: x2gousbmount
X-X2go-PR-Source: x2gothinclient
Date: Sun, 02 Dec 2018 03:20:10 +0000
Content-Type: multipart/mixed; boundary="----------=_1543720810-8715-0"

[Message part 1 (text/plain, inline)]

This is an automatic notification regarding your Bug report
which was filed against the x2gousbmount package:

#1136: Multiple issues with x2gousbmount

It has been closed by X2Go Release Manager X2Go Release Manager <git-admin@x2go.org>.

Their explanation is attached below along with your original report.
If this explanation is unsatisfactory and you have not received a
better one in a separate message then please contact X2Go Release Manager X2Go Release Manager <git-admin@x2go.org> by
replying to this email.


-- 
X2Go Bug Tracking System
Contact owner@bugs.x2go.org with problems

[Message part 2 (message/rfc822, inline)]

From: X2Go Release Manager X2Go Release Manager <git-admin@x2go.org>

To: 1136-submitter@bugs.x2go.org

Cc: control@bugs.x2go.org, 1136@bugs.x2go.org

Subject: X2Go issue (in src:x2gothinclient) has been marked as closed

Date: Sun, 2 Dec 2018 04:19:17 +0100 (CET)

close #1136
thanks

Hello,

we are very hopeful that X2Go issue #1136 reported by you
has been resolved in the new release (1.5.0.0) of the
X2Go source project »src:x2gothinclient«.

You can view the complete changelog entry of src:x2gothinclient (1.5.0.0)
below, and you can use the following link to view all the code changes
between this and the last release of src:x2gothinclient.

    http://code.x2go.org/gitweb?p=x2gothinclient.git;a=commitdiff;h=448f3812eca5180f48b57d7988732eecd14b47dd;hp=d1a83c17ca9568f8ef5ec2bb89e754db5e8b7df3

If you feel that the issue has not been resolved satisfyingly, feel
free to reopen this bug report or submit a follow-up report with
further observations described based on the new released version
of src:x2gothinclient.

Thanks a lot for contributing to X2Go!!!

light+love
X2Go Git Admin (on behalf of the sender of this mail)

---
X2Go Component: src:x2gothinclient
Version: 1.5.0.0-0x2go1
Status: RELEASE
Date: Sun, 02 Dec 2018 04:10:28 +0100
Fixes: 419 718 719 726 840 997 1092 1136 1147
Changes:
 x2gothinclient (1.5.0.0-0x2go1) RELEASED; urgency=low
 .
   [ Mike Gabriel ]
   * New upstream release (1.5.0.0):
     - Stop advertising GNOME in default session profile. Switching to MATE.
     - Use type=auto for automatic keyboard detection as default in example
       session profiles.
     - Avoid error message in x2gothinclient_shell if the chroot does not have
       a file-or-symlink /etc/resolv.conf.
     - Use resolv.conf file from host system when running x2gothinclient_upgrade.
     - Add COPYING file.
     - Run x2gothinclient_shell and x2gothinclient_upgrade with LANG=C.
     - Don't run x2gothinclient_init inside TCE chroot operations
       (x2gothinclient_shell or x2gothinclient_upgrade).
     - Avoid x2gothinclient_init failures, use if..then..fi instead of
       the test command.
     - Init scripts: Provide status option.
     - Use lazy unmounts for unmounting bind-mounted /proc, /sys, /dev/pts
       after exiting the TCE chroot (x2gothinclient_create, x2gothinclient_shell,
       x2gothinclient_upgrade, x2gothinclient_cleanup).
     - Assure that a tmpfs under <chroot>/root gets mounted and cleanly
       unmounted (x2gothinclient_shell, x2gothinclient_upgrade).
     - Use run-parts to initialize the TCE during runtime. (Fixes: #419).
     - New TCE component: x2gothinclient-minidesktop. Instead of using
       X2Go Client as login manager, we launch a minimal MATE desktop and
       connect to X2Go sessions from there.
       . Add WindowList applet to MATE panel.
       . On $HOME creation, copy x2gothinclient-minidesktop.desktop into
         ~/Desktop.
       . Provide init script for x2gothinclient-minidesktop to handle maintenance
         tasks if TCE is installed with in minidesktop flavour.
       . Tweak MATE minidesktop via GScheme overrides.
       . Launch MATE minidesktop via LightDM autologin feature.
       . Add XDG autostart .desktop file for the TCE's X2Go Client.
       . Add VERSION.<package> file for x2gothinclient-minidesktop.
       . Make X2Go TCE layout / appearance overridable by installing
         debian-edu-artwork package.
       . Use default X2Go Client background for x2gothinclient-minidesktop.
         Provide configurable desktop background instead. Install desktop
         background as desktop-base alternative.
       . On minidesktop TCE creation based on wheezy, pull in packages from
         wheezy-backports (i.e. MATE desktop environment).
       . MATE Application menu: Move the default X2Go Client .desktop file out
         of the way (dpkg-divert) and provide the TCE's .desktop file instead.
       . Make sure that launching x2goclient via a terminal or the run command
         box in MATE results in the restricted TCE X2Go Client.
       . Make contrib and non-free software available on minidesktop TCE
         creation.
       . Disallow Caja to handle storage media (automounting, opening, etc.).
       . Enable usbmounts and smartcard rules for minidestop TCE flavour.
       . Enforce TCE session restart after logout.
       . Provide Web browser shortcut in the MATE panel.
       . Enable native-protocol-tcp module in pulseaudio on every X2Go Client
         launch. Allow access from local host only, require auth cookie.
     - Adapt management package to TCE flavouring. Add new configuration
       option in x2gothinclient_settings: TC_FLAVOUR (can be: minidesktop or
       displaymanager).
     - Use proper quotations in several X2Go TCE scripts.
     - Don't choke if /etc/resolv.conf in the TCE chroot is a symbolic link
       (in x2gothinclient_shell, x2gothinclient_upgrade).
     - README.i18n: Add file that explains the translation workflow for
       this package. Thanks to Mark Pedersen-Cook for drafting this file.
     - For X2Go TCE in display manager mode, set login backgrounds of all
       screens (if more than one is used) to a blue'ish background color.
       (Fixes: #718).
     - TCE in display manager mode: Don't align multiple heads next to
       one another if a (Wacom) touchscreen is detected in the list of heads.
       (Fixes: #719).
     - management/sbin/x2gothinclient_update: Create
       $TC_CHROOT/etc/x2go/x2gothinclient_init.d/ before copying files into it.
     - management/sbin/x2gothinclient_update: Consider known_hosts2 FreeRDP host
       key files, known_hosts was used for FreeRDP v1.x and are in fact
       deprecated nowadays.
     - management/sbin/x2gothinclient_create: Stop using deprecated --force-yes.
     - 010_x2gothinclient-fresh-home: Install FreeRDPv2's known_hosts2 file into
       ~x2gothinclient user's home dir when creating a fresh home.
     - x2gothinclient_sessions: Provide RDP-Direct session profile example that
       utilizes FreeRDPv2 rather than deprecated FreeRDPv1.x. (On Debian stretch,
       it is recommended to use FreeRDPv2 from Debian stretch-backports).
     - x2gothinclient-<TC-FLAVOUR>_start: Drop requirement of having symlink
       /etc/x2go/x2gothinclient_start. Now all code passages that use the _start
       wrapper, use the x2gothinclient-<TC-FLAVOUR>_start file directly.
   * debian/control:
     + Rename bin:package: x2gothinclient -> x2gothinclient-daemon.
     + Make sure x2gothinclient-minidesktop pulls in X11 and X2Go Client.
     + New bin:package: x2gothinclient-common. Commonly shared by
       x2gothinclient-minidesktop and x2gothinclient-displaymanager
     + Add D (displaymanager flavour, minidesktop flavour): pinentry-x2go.
     + Add D (x2gothinclient-minidesktop): lightdm.
     + Add R (x2gothinclient-minidesktop): flashplugin-nonfree.
     + Add R (x2gothinclient-chroot): ntp.
     + Move to Pre-D (x2gothinclient-minidesktop): x2goclient, lightdm. Make
       sure those packages are installed already avoiding dpkg-divert failures.
     + Allow co-installation of displaymanager and minidesktop bin:pkgs. They
       have no conflicting files and we should rather make the startup mode
       configurable through debconf.
   * debian/copyright:
     + Use copyright-format 1.0.
 .
   [ Stefan Baur ]
   * New upstream release (1.5.0.0):
     - Allow Xinerama / XRandR setup via /proc/cmdline. (Fixes: #726).
   * usbmount/x2gousbmount:
     - Make user pseudo-selectible via a variable and fix usage across file.
     - Try to create file-sharing facilities harder.
     - Fix desktop/thinclient mode detection. Fixes: #1136.
     - Set default username to x2gothinclient for MATE minidesktop builds, even
       for X2GO TCE Live builds.
     - Add support for (V)FAT, NTFS, HFS, HPFS and EXT* (via fuse-ext2) file
       systems.
 .
   [ Mark Pedersen-Cook ]
   * debian/po:
     + Add DebConf translation to Danish.
 .
   [ Kaan Ozdincer ]
   * debian/po:
     + Add DebConf translation to Turkish.
 .
   [ Mihai Moldovan ]
   * New upstream release (1.5.0.0):
     - Change string "X2go" to "X2Go" where appropriate.
     - displaymanager/init/x2gothinclient-displaymanager.init: fix unchanged
       "gdm" string in log message. Fixes: #997.
     - management/share/etc/x2gothinclient-minidesktop_start: correctly
       continue commented out command on the next line. Fixes: #1147.
     - displaymanager/sbin/x2gothinclientd: correctly break long strings over
       several lines and concatenate them.
     - management/share/etc/x2gothinclient_init.d/
       950_x2gothinclient-minidesktop: call Xsession in mate-minidesktop builds
       of X2Go TCE Live.
     - misc: update copyright notices, but for Mike Gabriel.
     - README.howto: fix typos.
     - misc: update http:// to https:// URLs where appropriate.
     - misc: drop whitespace from hashbangs.
     - debian/x2gothinclient-common.postrm: newline at EOF change.
     - misc: update Oleksandr's mail address.
     - misc manpages: whitespace only.
     - misc manpages: pre-release version and date updates.
     - cdmanager/powerej/mb.{cpp,h}: whitespace only.
     - cdmanager/powerej/powerej.pro: regenerate with Qt5's qmake and merge.
     - cdmanager/powerej/eject.cpp: use PlastiqueStyle with Qt4 and Fusion with
       Qt5.
     - cdmanager/powerej/eject.cpp: include QStyleFactory.
     - misc manpages: pre-release date update.
   * usbmount/x2gousbmount:
     - Fix typo int -> print. Fixes: #840.
     - Update copyright notice.
     - Fix left-over check_x2gothinclientd definition (should have been
       check_x2gothinclientmode).
   * debian/control:
     - Whitespace only.
     - x2gothinclient-cdmanager: drop libqt4-gui Depends. Already handled by
       libqt4-dev Build-Depends and shlibs and misc Depends.
     - Maintainer change in package: X2Go Developers <x2go-dev@lists.x2go.org>.
     - Uploaders: add myself. Also, force a rebuild due to the changed
       versioning.
     - Resurrect x2go{thinclient-management,usbmount,cdmanager,smartcardrules}
       as dummy packages in the spirit of x2gothinclient for a smooth upgrade
       path.
     - mate-media-pulse and mate-settings-daemon-pulse are gone in Stretch.
       Depend on them lightly.
     - Replace iceweasel dependency with firefox-esr. All suites provide the
       newer name. Avoids an ugly outdated menu entry.
     - Build-depend upon Qt5 instead of Qt4.
     - Bump DH compat level to 9.
   * management/share/etc/x2gothinclient-minidesktop_start:
     - Correctly quote pactl command line arguments, avoids the shell
       interpreting the embedded semicolon as a command separator.
   * debian/po:
     - Minor fixup on author name.
     - Regenerate templates and files.
     - Update template with metadata.
     - fr.po: update metadata.
     - Update bugs address in template.
     - Update translation metadata.
     - da.po: fixup.
     - de.po: fixup.
   * debian/copyright:
     - Remove obsolete and wrong obviouslynice.de address.
   * debian/rules:
     - Whitespace only.
     - Switch to building cdmanager/powerej via Qt5.
     - Modernize respective build flags.
     - Add all clause.
     - Make sure to select Qt5 variant, even though we probably won't use
       debhelper to build Qt projects.
     - Call qmake instead of qmake-qt5.
   * debian/compat:
     - Bump DH compat level to 9.
 .
   [ Oleksandr Shneyder ]
   * x2gothinclientd: load config file when starting PulseAudio. Fixes: 1092.
   * upgrade to debian stretch.
   * set oldsatble to jessie in x2gothinclient_settings.
   * install linux-image-4.9.0-4-686 instead of dummy-package
   * add pxelinux in recommends for x2gothinclient-management
   * fix links in x2gothinclient_preptftpboot
   * fix link in x2gothinclient_create
   * install chroot for amd64
   * fix x2gothinclient_update to create files needed by x2gothinclientd
   * let user choose architecture by setting TC_ARCH variable. Default amd64.
   * improve support for gnupg and dbus.
   * modify x2gothinclientd to start scripts needed by gpg agent.
   * install meta-package for linux-image to get most recent version. Thanks to Mike for the hint.
   * start openbox to make xinerama work. Thanks to Walid Moghrabi for the hint.
   * fix Xorg checking in x2gothinclientd.
   * remove nomedeset option from x2go-tce.cfg. This option prevent loading of drm and disabling advanced graphic functions.
   * add openbox dependency.
   * do not start openbox any more. Xinerama issue should be fixed in x2goagent.
 .
   [ Martti Pitkänen ]
   * debian/po:
     + Add DebConf translation to Finnish.
 .
   [ Thierry Kauffmann ]
   * New upstream release (1.5.0.0):
     - debian/po: add French translation file.

[Message part 3 (message/rfc822, inline)]

From: Stefan Baur <X2Go-ML-1@baur-itcs.de>

To: submit@bugs.x2go.org

Cc: Oleksandr Shneyder <o.shneyder@phoca-gmbh.de>, Mike Gabriel <mike.gabriel@das-netzwerkteam.de>

Subject: Multiple issues with x2gousbmount

Date: Sat, 14 Jan 2017 13:54:16 +0100

[Message part 4 (text/plain, inline)]

package: x2gousbmount

Hi $LIST, hi Alex, hi Mike#1,

due to a recent bug report on X2Go-User, I needed to take a closer look
at the x2gousbmount package, and the script
/usr/lib/x2go/tce/x2gousbmount in it.

I'll be referencing
http://code.x2go.org/gitweb?p=x2gothinclient.git;a=blob;f=usbmount/x2gousbmount;h=4da1fbd90b14dda1beb230ea4b08441a77d9e4eb;hb=75211b6e724bd896d85d915c754703cabcf37187

for line numbers below, even though that version is more advanced as the
one in the stable x2gousbmount package.

I'm hoping that Alex or someone else that has a clue wrt/ Perl and this
script can shed some light on these issues.

Mike#1, you're in CC due to issue #3, which seems to have been
introduced in your commit.

Feel free to suggest I should turn every issue into a separate bug
report - I'm not filing them separately at the moment because the issues
seem all more or less interconnected, so I figured I should start with
one bug report for "the big picture".

Issue #1:
Lines 88 and 138 contain a call ... expand_filename("~x2gothinclient ...
Now, I understand that this is an attempt to determine the path to the
home directory of the user named "x2gothinclient".
Which, in the classic X2Go-TCE, is "/var/lib/x2gothinclient". So far, so
good.

But: Lines 107 and 148 contain what looks to me like a subtle typo:
 ... expand_filename("~/x2gothinclient ...
Which means, instead of selecting the home directory of the user
x2gothinclient, it would go to the home directory of the user under
which the script is being executed, and a subdirectory "x2gothinclient"
there, so most likely
/root/x2gothinclient (which doesn't exist)
So the script will probably fail whenever it encounters encrypted volumes.
(I can't test this myself, I don't have a setup with encrypted USB media.)


Issue #2:
The user name "x2gothinclient" is hardcoded in several places inside the
script.

This calls at least for a "my $user='x2gothinclient';" at the beginning
of the script, and the replacement of every occurrence of
"x2gothinclient" with "$user".

The reason being that TCE-Live uses a different user name, namely "user"
(and a different home directory, /home/user), so we need to be able to
swap out the name in one central location.

There are two ways of determining that we're running in TCE-Live, I am
not sure which one would be the preferable one:
1) Checking /proc/cmdline for the appearance of (in regex syntax)
"\W*live\W*" or "\W*boot=live\W*" (in case your mail software turns
asterisks into bold type, there's an asterisk after each capital W).
2) Checking if the directory /lib/live or /lib/live/config exists.
If any of these match, then $user should be set to 'user' instead of
'x2gothinclient'.


Issue #3:
Comparing the master branch in git with the version in stable, line 37
is different.

There now is an additional check for the existence of a directory
"/usr/share/doc/x2gothinclient-minidesktop".

Which script/package creates this directory and why is it equivalent to
detecting a running x2gothinclientd?

Looks like
http://code.x2go.org/gitweb?p=x2gothinclient.git;a=commit;h=a458a1ed666eb12b5d856812396ecd31fba01585
introduced that change, see
http://code.x2go.org/gitweb?p=x2gothinclient.git;a=commit;h=a458a1ed666eb12b5d856812396ecd31fba01585
but there's no meaningful commit message. Mike#1?


Issue #4:
I am unhappy with the subroutine check_x2gothinclientd.  Indeed,
grepping for substrings so you don't trigger on the parameters of your
own grep command in the output of ps is a neat hack, but a hack remains
a hack.
The clean way of handling this, IMO, would be to change the subroutine
as follows:

sub check_x2gothinclientmode {
	# check if X2GoClient is running in thinclient mode
	# old code would check if x2gothinclientd was running,
	# which fails on X2Go-TCE-live
	my $x=`ps u -C x2goclient`;
	if ( $x=~m/\W*--thinclient\W*/ )
	{
		return 1;
	}
	return 0;
}

This will no longer detect if x2gothinclientd is running, but if
x2goclient has been called with parameter --thinclient.  Which should be
the case in both TCE-Classic and TCE-Live.

Due to the name change of the subroutine, line 37 needs to be changed to
use check_x2gothinclientmode instead of check_x2gothinclientd.
Maybe the changed check means the || - part in line 37 is no longer
needed as well?


Issue #5:
Lines 88 and 138 silently assume that there is a subdirectory "export".
I can't see it being created anywhere, though.
Same goes for lines 107 and 148 and the subdirectory "logins".


Issue #6:
Why do we need two separate subdirectories "export" and "logins", anyways?


Issue #7:
Somewhere around line 48, I'm missing a comment that explains what this
part of the code is for.
I would suggest adding:
# mntdir is not the directory where the mountpoint will be rooted,
# but where tracking of mount states takes place


Issue #8:
Why do we have to track these mountpoints manually, anyways?  Is the
information in /proc/mounts insufficient?

-Stefan

-- 
BAUR-ITCS UG (haftungsbeschränkt)
Geschäftsführer: Stefan Baur
Eichenäckerweg 10, 89081 Ulm | Registergericht Ulm, HRB 724364
Fon/Fax 0731 40 34 66-36/-35 | USt-IdNr.: DE268653243

[signature.asc (application/pgp-signature, attachment)]

Send a report that this bug log contains spam.