Meta: DSM6 compatibility of packages with user privilege issues

[m4tt075]

In #2661 @Dr-Bean provided a complete overview on issues with current packages. There is one class of issues related to how DSM6 handles user priviliges (see #2216 for the related discussion and information on how to fix the issue).

Edit: In the meantime, @ymartin59 has developed and merged generic service support for DSM 5+6 packages (PR #2949). As such, the objective is now to convert all the affected packages to use the newly created generic service support.

Here is an overview of the status of those packages.

I We will try to tackle them successively, but will be slow. If you want to contribute, please just comment which packages you would like to tackle so that we avoid double work. I will update the table regularly. Please report any errors / updates you find.

PACKAGE	STATUS	ISSUE	COMMENT
bitlbee	Retired	#2552	Last update: Dec 2016, Retired with #4208
couchpotatoserver	Published
couchpotatoserver-custom	Published
deluge	Published
domoticz			Last update: Mar 2015
ejabberd		#2260
flexget		#3035
full-text-rss
gateone	Published
git-server	Published
haproxy		#2712
headphones	Published
headphones-custom	Published
homeassistant	Published	#2728
htpcmanager
icecast
jackett	Published
lazylibrarian	Published
maraschino
memcached
monit
mosquitto	Published
museek-plus	Retired
mylar	Published
nzbget	Published
nzbhydra	Published
nzbmegasearch
plexivity
plexpy-custom	Published
pyload		#2259
python	Published
radarr	Published
redis	Published	#2832
sabnzbd	Published
sabnzbd-testing	Retired	#2309
saltpad
shairport-sync		#1877
sickbeard-custom	Published
sickrage	Abandoned. Use sickbeard-custom		@Safihre PR #3093 , Link to 6.1 test builds
sonarr	Published
squidguard	Planned		@ymartin59 #2883
syncthing	Published		@Safihre PR #3094 , Link to 6.1 test builds
syncthing-inotify	Obsolete - now included in syncthing
transmission	Published
tvheadend	Published
umurmur	Published
znc	Published	#2294

[BenjV]

I am busy changing the NZBGet package in such a manner that it will be completely compatible with DSM 5 and DSM 6 and in according to Synology standards with as much possible use of DSM tools.

The current version in of that package in the DSM 6 branch is not able to do that correctly in my opinion.
It is important to use as much as possible the tools as provided by Synology, this to prevent that in the future problems arise from incompatibility, like the DSM 5 to DSM 6 transition.

I almost finished the work and will begin testing tomorrow.
If testing is successful, I will make it available to the SynoCommunity.
If agreed upon the changes I will be happy to help with other packages.

[ymartin59]

May it be possible to document a set of "recipes" describing how to create a package "the right way", typically with a "installer.sh" template script when a technical user is required for service process, and how to port existing spksrc package ? Information is spread in lots of issues and comments... it would be perfect as a wiki page ! Thank you in advance

[BenjV]

I agree that the information is spread all over and the "3-rd-party Package Developer Guide" in not a very easy readable document.
Also there are a lot of "syno..." commands that aren't documented at all but are very needed to make a good package.
I will create a document with all my findings and specifications in a (hopefull) readable format and all the .sh (and other) files needed to build a package.

Not sure if I want to spend the time learning how to use wiki for that.

I hope someone can test the package I create on a DSM 5 machine, because I don't have one, both my machines are on DSM 6.

[ymartin59]

I run a XPEnology 5.2 as vmware virtual machine. I should document how to build it too !
Please write your notes "raw", I will do wiki formatting after.

[m4tt075]

Table above updated with WIP notes for nzbget and squidguard.

@BenjV: Great, you are in for it, too! I'm aware of your discussion with @Dr-Bean some months ago but do not have any strong feelings on the approach. I will just wait for your POC PR. Maybe @ymartin59 and @cytec can judge which route is best and then I will help implementing whatever you guys settle on.

[Safihre]

@m4tt075 sabnzbd and sabnzbd-testing are listed as fixed, but if I read the issues you link it seems that it's not fixed?
I also had some users reporting that after every update they have to manually fix permissions on DSM6 by changing the user to root in the boot script. I assumed this was because of it not being fixed yet due to this being still open: #2216. If it should work for them, I will instruct them to create issues here.

[m4tt075]

@Safihre : The 5.2 package on the master branch, which is published via the Synocommunity platform, is not compatible with DSM6. If people install them on DSM6 systems, they are bound to run into the kind of trouble you describe. @Dr-Bean's fixed version is only available on the DSM6 branch (therefore the comment in the above table). You should be able to cross-compile those packages and run them (correctly) on 6.1+ firmwares ("should", because I did not test it). The problem with those DSM6 compatible packages is though, that they can currently not be published via Synocommunity. This is the catch 22 with all affected packages.

[m4tt075]

Table update:
Fix for transmission is already available on DSM6 branch (implemented by @Dr-Bean)

[m4tt075]

@ymartin59 : Help! I just went through the DSM6 branch and realized that @Dr-Bean has already "fixed" almost all of them half a year ago. I simply checked which packages already possess the necessary privilege files and the list below showed up. It is unfortunate @Dr-Bean has suddenly disappeared but I believe he has actually done most of the DSM6 compatibility work already and was "just" lacking the publication platform to initiate broader testing. Might be wrong though. I started offering test builds initially (like you have done in the past), but given the breadth of packages we are looking at here, I don't think that is a viable option. What is your take? How should we proceed?

Here the additional packages @Dr-Bean already addressed (all untested of course):
bitlbee
boxbackup-client
couchpotatoserver
couchpotatoserver-custom,
domoticz
deluge
ejabberd
ffsync
flexget
gateone
git-server
haproxy
headphones
headphones-custom
homeassistant
htpcmanager
icecast
jackett
lazylibrarian
maraschino
memcached
mosquitto
mylar
nzbget
nzbget-testing
nzbhydra
nzbmegasearch
octoprint
plexivity
plexpy-custom
pyload
radarr
redis
rutorrent
saltpad
sickbeard-custom
sickrage
sonarr
squidguard
subliminal
syncthing
znc

[BenjV]

I am sorry to say that the already changes packages on the DSM 6 branch are having some flaws.

I have finished my testing of the NZBGet package.
I used the work of Dr. Bean as a basis to created a package that will work on both DSM 5 and DSM 6, with minimal impact.
I am now busy writing the concepts of such packages that can be applied to all packages..
Probably tomorrow I wil post it here, so someone can test it on a DSM 5 environment.

I still have to do some thinking on how to handle the following scenario.
Packages installed on DSM 5.
NAS was upgraded to DSM 6.
Upgrade the package to a new version after the NAS upgrade.

[ymartin59]

@BenjV If same Makefile can produce both DSM 5 and 6 packages, it would be perfect.

For "upgrade path", it is really difficult to test without real hardware, as virtual machines we may find are not designed to support DSM update/upgrade.

Would it be reasonable to state that "we try to" support in place upgrade of "old" installed package version (which may be really old, some builds in repository have been produced for DSM 4 !) when already running on DSM 6. In case our "tentative" is not successful (and there is high chance it is not without proper testing), we would have to do support to help users to clean up their system, as uninstall scripts often fails on DSM 6. Worst option remains: re-install fresh DSM 6, reapply config backup and reinstall all packages....

What do you think about it ? World cannot be perfect... I wonder if there are many users having upgraded from DSM 5 to 6 with "complex" package installed (typically with database schema)

[BenjV]

The MakeFile can fore sure be used for DSM 5 and DSM 6, but it have to changed a little.
The placing of the firewall file .sc is wrong it should be located inside the package.tgz.
I suggest to put it in the /app folder along with the /image folder and the config file.
A resource file should be located in the /config folder in the root of the package in which a reference to that .sc is made like this.
{
"port-config":{
"protocol-file": "app/.sc"
}
}
I know it is very difficult to test this upgrade stuff, so I am trying the theoretical approach and see what issues comes up.
Later this day (I hope) I will post more details.

A cleanup script would be nice, I myself have had some trouble removing packages while testing.
I will give it some thoughs, when I have finished the current work.

[BenjV]

I have finished the package for NZBGet.
It can be found here:
https://github.com/BenjV/SYNO-packages/raw/master/nzbget_avoton-6.1_16.4-22.spk

I have used the avoton version but the important things are in the script files which can be reused.
Just get the INFO file, the scripts, WIZARD_UI, the config folder and the /app/config out of the package en put it in the spksrc framework.

The main difference is that this package will run as Synology has layed out in their documentation and that the package wil run with user privileges and not with root.
I made as much as can be done use of all the standard variables supplied by Synology.
There is no need for two users like "nzbget" and "sc-nzbget"
The "system internal user" which is used has limited visibility in DSM and priviliges can be granted via a group.
I changed the WIZARD_UIFILES so that the user can choose the name of that group and also the name of the Share which will be created.

If someone can test it under DSM 5 I would be very interested in the line of the /etc/passwd file of the nzbget user. This will show if the package is completely compatible with DSM 5 or not.
If not I may have to make some changes to use a "normal" user and not a "system internal" user if running on DSM 5.

I am working on a more elaborate document but my time is limited at the moment.
Feel free to post questions if you have.

[m4tt075]

Great. Thanks, @BenjV. Could you please PR your changes (I'd propose against the DSM6 branch until it has been tested more broadly) to make sure we are all on the same page? I have set up a VM with XPEnology 5.2. Happy to test for you...

[BenjV]

I am sorry but I am not familiar with the spksrc structure.
I just changed an existing package so it will work and that's not exactly the same as the structure I see on GitHub, so you have to make the translation and create the pr yourselfs.
You have scripts to create those files and I don't no how that functions and I don't have the time to figure it out at the moment.

I can list the files which I changed.
The following files where changed:
INFO
/conf/privilege
/scripts/installer
/scripts/start-stop-status
/WIZARD_UIFILES/install_uifile
/WIZARD_UIFILES/install_uifile_fre

I removed the following file because the were are not needed.
/WIZARD_UIFILES/install_uifile_enu
/WIZARD_UIFILES/upgrade_uifile
/WIZARD_UIFILES/upgrade_uifile_enu

By the way, there were structural errors in the Wizard-UI files so the input validation did not work at all and the other scripts were full of potential risc regarding filenames. There were missing a lot of "" around variables.
That's risky because of potential globbing or word spiltting, for example if someone uses a space in a filename.

[ymartin59]

Many thanks for your work @BenjV. I will report your change on package into spksrc as a PR.

[m4tt075]

Perfect. Thanks, both.

[drpau]

Hi, i am new to the synology community and note that even after enabling betas in DSM package manager, i still am not able to see all packages listed. I presume particular packages have been pulled due to incompatibilities or is there something else i am seeing?

For example - Headphones is not an available package however listed on the synocommunity page.

thank you

[m4tt075]

@drpau: Your problem is related to a lack of DSM6 compatibility of publishable packages. This issue is part of a more general approach to sort these things out. Please open a separate issue on the package you are looking for, if it does not already exist.

@ymartin59 : I have seen you closed #2912 (your first DSM6 merge branch). No prob, but I think we need to step back and sort some things before we can advance here. I have also seen that your are currently reorganizing the issues as projects, which is great. Here my perspective on what we still need to clarify for your consideration and potential reflection in your project overview:

1. I have seen @BenjV's proposal, understand some (not all) of the changes and believe some concerns @Dr-Bean raised in PR DSM6: adduser command not working [$100] #2216 are not addressed. Please correct me if I'm wrong, but my understanding is that you wanted to review and PR @BenjV's changes (with or without adaptations?), which should provide the right place for those discussions. Until learning otherwise, I will watch out for that PR.
2. Once it is clear which way to go and we deviate from the @Dr-Bean approach @maxrogers and I applied, I'd suggest broader testing of sample packages before we scale up. Happy to contribute TVH as guinea pig again. Others (e.g. @Safihre?) might be willing to step in, too. But I would be hesitant to scale up the approach to all the packages above before we have some reassurance that the new approach works reliably.
3. Once we are reassured, we need to coordinate package updates and DSM6 compatibility updates or we will overwrite updated non-compatible packages with old compatible packages or vice versa.

Ok for you?

[ymartin59]

@m4tt075 I really agree. I have not reviewed BenjV's proposal yet but I also guessed it deviates from first approach.

About point 3 and my master-to-dsm6 merge, my aim was to be able to locally merge package branch with dsm6 to produce packages with DSM 6 toolchains without generating conflicts.

From here, I think any package compatible with DSM 6 should be sent to dsm6 branch, even if it is retrocompatible with DSM 5 too.

Probably one of our issue is the copy-paste pattern applied to installer.sh. A proposal may be to provide base features globally in mk/ as functions and invoke them from installer.sh.

[m4tt075]

@ymartin59 : Yes, makes sense. We have got a plan then. Let me know if I can help...

[BenjV]

I also red @Dr-Bean's concerns and I will comment here on how I have dealt with them.

-About the privileges file:
I agree that the user must be created in the privilege file and that no group should be created there.
I disagree with the "sc" adding to the username, that only confuse thing and has no purpose.
Don't use the "wonky" busybox commands but use (as I did) the synouser and the synogroup command, I am busy creating some text explaining how to use them.
Just leave the username equal to the package name.
The comment that the existing users may not be removed because there are not our users is simply not true, they were created in the past by installing the SynoCommunity package.
We have to remove it, because if it still exitst the installer of Synology will create a user with a different name and we no longer have access to that name to be able to add it to a group.

My original idea was to also use the group in the installer, but there is a bug in the installer from Synology that would not allow the same group for different packages.
I contacted Synology about that and they agreed with me that it was a bug and that they will look into it to fix it in "some" future release. So we cannot use that at the moment.

Also we have to add the "status" to run-as root.
To check if a package is running we use the kill -0 and that commands needs root privs.

About the installer:
Again don't use the wonky busybox commands.
I would suggest to leave out the installation of busybox completely.

-Preugrade
Here I remove the legacy user, we don't want them anymore because they are member of the group "users" and so anybody who is a user has access to any file a package creates.
That is from a security standpoint undesirable.

-Postupgrade
Here I create the group and add the user to it.
I disagree to make existing users a member of the group "users" again as I explained.
Adding the user to the group sc-media or sc-download gives sufficient control.
Maybe we should think about some more different "sc" groups, because some packages would be hard to classify to one om those two groups.
I also think it is a very good idea to make the user name equal to the package name, this way there is no need to add to the installer and we do not have to change that for all packages.
This is needed if we want to make script files that are common for all packages.

About compatibility we have to wait the result of the testing, I think even under DSM 5 and with the privilege file in place, DSM 5 would create a hidden user, if not we have to create a legacy user ourselves, my package already takes care of that .

Start-stop-status
Don't use the su command to run the user the package, start the package, Synology will take care that it "run-as" the user from the privilege file, don't duplicate things.

I would suggest to move the standard stuff that is in the installer to the preinst and postinst scripts and just keep the package specifics in the installer.
That's why it is a good idea to keep the package-name and the user-name the same, we could use the environment variable supplied by Synology for both and those script could be the same for everything.

I don't know if the spksrc scripts create the firewall (package.sc) file which I found in the script folder.
If we put a resource file in the "conf" folder and the firewall (.sc) file in the /app folder we don't need the "servicetool" command to install/de-install the firewall file as I did now.
Both solutions are valid.

The main difference for the end-user will be that no longer all users (via the users group) have read/write privs on everything, but has to grant it themselves via the "sc-media" or "sc-download" groups.
But that's the price they have to pay for more security.

I will work on moving the general part of the installer to the standard scripts and finishing the documentation.
If somebody still has concerns that my solution does not address some points from @Dr-Bean please be specific so I can react.
I put a lot of thoughts in this solution and I think I covered all the angels but I am always open for other ideas.

We have to test that all packages will run with user privileges and not need root or system privileges.
If all scripting has been testen and found OK we should start releasing all packages as beta DSM 6 so end-user can test it.
We also have to tell them how granting privs via the sc-media and sc-download groups work, because a lot of end users have no idea how privs work.
I already added some of that info to the WIZARD_UI files, but mayby some text on the synocommunity.com site can help.

[m4tt075]

@BenjV : Wow, you have obviously thought a lot about this. The concerns I was referring to where the ones about using different user names, but I assumed (and did not verify) that what @Dr-Bean wrote, was correct. If it wasn't and this is no longer an issue, that's great. You have also already answered most of the things I did not understand. The only additional questions I' have right now are
a) How we are going to deal with DSM upgrades
b) Whether differentiating btw major firmware versions (rather than the detailed one like in the old approach) is sufficient (if I remember correctly the breaking point was 6.0.2 or something
c) Why the comment in postinst refers to DSM5 or lower, but you test for -lt 7
But it is all academic without running tests, so please don't worry, all of this can wait. I'm keen on testing your new approach...

[BenjV]

The removing of the old name is in my opinion not a problem, those users were created by the SynoCommunity package and if somebody used that name for other purposes he (or she) has to create a new name. I don't think many users have done that, and the few that did it have to take some action.
The name clearly state that it is part of the packages because it is always the same a the package name.

Also if we leave the old username in place it will create enormous confusion, because it doesn't function anymore in settings privs.

We need to keep the username the same as the package name otherwise we don't have a way to find out this name within the scripts and we have to add a new name for every package we have.
Now we can use universal scripts for every package and I have already started to create them.

About your other concerns.
a) I have thought al lot about that but I cannot see a specific reason why it should not functionate.
Old installation will use an old user which keeps functioning and is still part of the "users" group which is very unsecure, but then it always was.
When a package update occurs (or a new installation) that user will be removed and change to a new "hidden" user with added security,

b) No, the breaking point was the change from DSM 5 to DSM 6, and to be honest I believe even DSM 4.2 already has all the tools in place to make this more secure working possible.
This was accomplished by adding the "support_conf_folder = yes" setting to the INFO file, this setting is as of DSM 6 deprecated, because as of that version it is mandatory.
Whether I am right we will see as soon as someone test my package on DSM 5, if so it will not create a normal user but a hidden user which we can see in the /etc/passwd file on the UID it got.
If someone has a DSM 4.2 environment available we can also test it on DSM 4.2

c) Very good spotting, I saw it yesterday myself, it is a leftover from some testing I did and should be "-lt 6" and not "-lt 7",

[BenjV]

Any news from the testing of my package on DSM 5?

[ymartin59]

@BenjV package.tgz in avoton package I downloaded from your URL is in fact a non gziped tar and it is almost empty... May you please check and package binaries with it ?

[BenjV]

@ymartin59
Ok I added the binaries, although they are not necessary to install the package.
I removed it to save time during my own testing.

I am especially interested to see how the /etc/passwd file will look like on DSM 5

[ymartin59]

Why not, I expect set_syno_permissions can be extended with group and path parameters

[Safihre]

I was going to convert sonarr/sickrage/etc packages today but noticed that they are set to GROUP=sc-media'.
Why are they sc-media? Wouldn't it be much more useful if they were also sc-download so they can work in harmony with the download packages without any user intervention?

[m4tt075]

@Safihre PRed quicker than I could update our tracking table... 😏
Just updated again. About time...

[ymartin59]

About start-stop-daemon, I was not aware most services have no "daemon" option. So I will extend generic support to start SERVICE_COMMAND as background process with PID_FILE generation included.

[Safihre]

@ymartin59, I don't think this is sufficient as it doesn't cover the case when the process restarts itself and the PID changes (resulting in the Stopped status in DSM).
The start-stop-deamon allows to locate a process based on executable name and user, without PID. This is how it's used now in some of these packages like Syncthing that really don't generate a PID. Is it really that terrible to use busybox for this? These are compiled packages anyway, and we include it also with the general Python package.

[ymartin59]

OK I propose to create a third generic start-stop-status script to support start-stop-daemon usage

[m4tt075]

+1

[m4tt075]

Welcome back, by the way!
Do you see any light at the end of the spkrepo tunnel in the meantime?

[ymartin59]

Yes, I have identified 502 failure when uploading x64 packages... the architecture list was so long that package filename in storage gets out of postgresql column length. One alter table later, I was able to upload mosquitto x64 package.
For DSM 6.x, firmware version is rejected because of a too restricted regular expression: https://github.com/Diaoul/spkrepo/blob/master/spkrepo/views/api.py#L23
Now firmware version has 5 digits... Should not be long to fix, just have to find proper way to restart uwsgi.

[m4tt075]

@ymartin59 : 👍 Happy New Year, Yves!

[Safihre]

@ymartin59 Great news 🎉👍👍

[ymartin59]

@Safihre I am currently publishing pending packages from master...
About group naming, I wonder if it may be possible to transition with:

• wizard update informing about group name to use
• service_postinst function testing if previous group name exists, and if so adds EFF_USER as member

I do not agree with you - it is never too late... and sooner any is fixed, better it is.

[Safihre]

@ymartin59 That would be great and also what I was thinking, I was working a bit before on this for Sonarr/Radarr but abandoned it when I saw that so many packages had sc-media. I didn't think you would like to make such changes 😉

I imagined:

• If sc-media exists and USER is part of it, make EFF_USER also part of sc-media
• Make this code that adds new users to existing group a function syno_add_to_group
• Or: something a function syno_add_backup_group_membership, that does the check and adds directlt if necessary?
• Simply update the wizard texts on the "DSM6 Permissions page" of all packages involved from sc-media -> sc-download.

[Heptite]

What's the progress on getting ejabberd functional?

[m4tt075]

@Heptite Not aware of any progress on this particular package. Maybe you are lucky and somebody picks it up, but we still have some packages in testing which need to be published first (see WIP in table above). We are getting there, it just takes time...

[Heptite]

@m4tt075 Is there any temporary workaround to getting it working?

Also, I'd like to point out that there's a small bounty (which I added to) on that particular package.

[m4tt075]

@Heptite Nope, the package needs proper adaptation. Seems it hasn't been updated for quite some time. The bounty might raise people's interest though. Good luck.

[m4tt075]

@ymartin59 @Safihre I know that you have been keeping track with the progress, also on the list above. Since most of the packages we had converted have been merged in the meantime, I was wondering whether I should continue with list. Having said that, my feeling is that there is not a lot of demand for the packages that are missing and that it might not be worthwhile to just keep them for the sake of keeping them. On the contrary, there might be a benefit in cleaning up and focusing more on maintainance of those packages we provide. Can you judge (maybe through downloads from spkrepo), which packages should be continued and which ones retired?

[Safihre]

Asked @Diaoul if we can somehow figure this out 👍 Indeed we should convert now the most popular packages.

[m4tt075]

Gateone is WIP: #3431
Table updated...

[markboston34]

Is synocli supported on dsm6?

[th0ma7]

Closing in favor of DSM7 package status.