Bug 856860

Created attachment 612311 [details]
replacement systemd service

Created attachment 612312 [details]
replacement systconfig file

Thanks for the info and examples. I thought I was the only consumer for shellinabox on RHEL/Fedora :)

I've started applying the changes you listed plus something more:

1- Install provided CSS files and relevant directory.
2- Expanded sysconfig file with additional example.
3- Expanded sysconfig file with my local ssh setup as example (we embed the Shellinabox window into some cgi scripts without ssl etc.).

There are a few things to consider before proceeding:

1) What about "styles.css" "and print-styles.css"?

According to the man page:

 --css=filename
 Sometimes, it is not necessary to replace the entire style sheet using the
 --static-file option. But instead a small incremental  change  should  be
 made to the visual appearance of the terminal. The --css option provides a
 means to append additional style rules to the end of the default styles.css
 sheet. More than one --css option can be given on the same command line.

When using the original "make install" commands they are not installed, though.

2) The "WorkingDirectory" in the systemd file is fine, but how can we make it available also to the init file for RHEL/CentOS? I've looked at the man page but there's no parameter to set it at runtime.

The Makefile installs the css files by default in $docdir.

3) The makefile install and the manpages are really debian-centric. Should we patch the man page to reflect the path of the installed files?

4) The list of options is now very long. Would setting user, group, cert directory etc. be worth splitting into multiple variables in the sysconfig file like it is in the default debian files? i.e.:

PORT=4200
DATADIR=/var/run/shellinabox
USER=shellinabox
GROUP=shellinabox

And then in the systemd file:

ExecStart=/usr/sbin/shellinaboxd -u $USER -g $GROUP --cert=$DATADIR --port=$PORT

(In reply to comment #3)
> 3) The makefile install and the manpages are really debian-centric. Should
> we patch the man page to reflect the path of the installed files?

Forget this, the file configuration section is added only if DPKGBUILD is set.
 
> 4) The list of options is now very long. Would setting user, group, cert
> directory etc. be worth splitting into multiple variables in the sysconfig
> file like it is in the default debian files? i.e.:
> 
> PORT=4200
> DATADIR=/var/run/shellinabox
> USER=shellinabox
> GROUP=shellinabox
> 
> And then in the systemd file:
> 
> ExecStart=/usr/sbin/shellinaboxd -u $USER -g $GROUP --cert=$DATADIR
> --port=$PORT

This definitely makes sense, here are the udpates:

http://pkgs.fedoraproject.org/cgit/shellinabox.git/log/

Seems like a good idea.  

For the extended options in shellinaboxd.sysconfig

Should the lines include the baseline for example something like this:

# OPTS="--user-css Normal:+black-on-white.css,Reverse:-white-on-black.css ${BASEOPTS}"

with BASEOPTS defined as currently set?

Regarding RHEL/CentOS, one could have a script in /usr/share/shellinabox that CDed to the correct directory before starting the daemon.  Or one could create an init script ;-)

Hello,

I'm currently on holiday, I'll be back next Monday and take care of the bug.
The RHEL/CentOS init scripts are already in place.

Regards,
--Simone

Hope you are having a wonderful holiday!  Thanks for your support.

Back from holiday.

I checked some other init scripts, entering a directory before launching a command is ok in SysV init scripts.

So I've set up WorkingDirectory in service files as your patch and added the cd command to the init scripts.

Regarding the split of multiple options and one long $BASEOPTS I think is better to keep them separate. Other packages follow the same approach.

Putting any additional variable to split the lines can be done by the user in the sysconfig file, if there's any need.

Binaries are here: http://koji.fedoraproject.org/koji/packageinfo?packageID=13996
I will push an update when the other pending updates I have in Bodhi reach stable.

Thank you very much for all your input!

--Simone

Thanks!

Did you forget the 
WorkingDirectory=/usr/share/shellinabox

line in /lib/systemd/system/shellinaboxd.service ?

Just looked in fc16 rpm and that seems to be missing.

Also, did you decide not to include the print-styles.css file?

Joel

(In reply to comment #10)
> Did you forget the 
> WorkingDirectory=/usr/share/shellinabox
> 
> line in /lib/systemd/system/shellinaboxd.service ?
> 
> Just looked in fc16 rpm and that seems to be missing.

Yes, my fault. Too many things at the same time.
 
> Also, did you decide not to include the print-styles.css file?

Well, it is not installed by default and not required for the basic operation of the daemon, unless you customize the interface and expose those files somewhere and reference them in the config file.

But since they're referenced by the man page I've added them to the %doc directory, much like the "examples" directory in other packages.

Are you already a Fedora packager? Would you like to have commit access to the various branches?

Builds available:

http://koji.fedoraproject.org/koji/packageinfo?packageID=13996

I'm not a fedora packager, although I've been making rpms since the 90's (was in RHCN).  Some day I should become one...

Simone, You are going to hate me but there is another bug:

In /lib/systemd/system/shellinaboxd.service

The ExecStart line needs to have curly braces around CERTDIR and PORT after the = sign to get them to expand.  Don't put the curly braces around the others as OPTS will be a single argument rather than a set of arguments.

New Corrected Line:

ExecStart=/usr/sbin/shellinaboxd -u $USER -g $GROUP --cert=${CERTDIR} --port=${PORT} $OPTS


Joel

Strange, it works on my RHEL boxes, but not on Fedora 17.

Builds on the way, here is the commit:

http://pkgs.fedoraproject.org/cgit/shellinabox.git/commit/?id=deba4cbc4f87ccf488f47d374d08cfacd5850a9c

I don't hate anybody, so anything that doesn't work please tell. :)

Thank you very much.

Hello,

I've found a thing that doesn't work. On Fedora (17, at least) the /var/run folder is a symlink to the /run mounted tmpfs filesystem.

tmpfs on /run type tmpfs (rw,nosuid,nodev,seclabel,mode=755)

Upon a reboot, the folders in it gets deleted and so shellinaboxd refuses to start because he dose not create his cert dir automatically. Any idea on how to solve this? systemd flags/directives maybe?

On RHEL is of course fine as /var/run is not on tmpfs.

--Simone

You are right.  Stupid me to suggest /var/run.  That is listed as a place for temporaries.  We should do /var/shellinabox.  

Joel

/var/lib seems to be a better choice [1]; an excerpt from the FHS:

"This hierarchy holds state information pertaining to an application or the system. State information is data that programs modify while they run, and that pertains to one specific host. Users must never need to modify files in /var/lib to configure a package's operation.

State information is generally used to preserve the condition of an application (or a group of inter-related applications) between invocations and between different instances of the same application. State information should generally remain valid after a reboot, should not be logging output, and should not be spooled data.

An application (or a group of inter-related applications) must use a subdirectory of /var/lib for its data. There is one required subdirectory, /var/lib/misc, which is intended for state files that don't need a subdirectory; the other subdirectories should only be present if the application in question is included in the distribution. [38]

/var/lib/<name> is the location that must be used for all distribution packaging support. Different distributions may use different names, of course."

I think these builds are fine and I would like to push them. Please note that you have to remove the old shellinabox user or change its path as its home directory has changed.

[1] http://www.pathname.com/fhs/pub/fhs-2.3.html#VARLIBVARIABLESTATEINFORMATION

shellinabox-2.14-14.el5 has been submitted as an update for Fedora EPEL 5.
https://admin.fedoraproject.org/updates/shellinabox-2.14-14.el5

shellinabox-2.14-14.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/shellinabox-2.14-14.el6

shellinabox-2.14-14.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/shellinabox-2.14-14.fc16

shellinabox-2.14-14.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/shellinabox-2.14-14.fc17

shellinabox-2.14-14.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/shellinabox-2.14-14.fc18

Package shellinabox-2.14-14.el6:
* should fix your issue,
* was pushed to the Fedora EPEL 6 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=epel-testing shellinabox-2.14-14.el6'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-EPEL-2012-13070/shellinabox-2.14-14.el6
then log in and leave karma (feedback).

https://fedoraproject.org/wiki/BugZappers/BugStatusWorkFlow#CLOSED