added libutempter #3583
added libutempter #3583
Conversation
mosh now builds against libutempter, this is a requirement for adding support for displaying detached sessions.
|
@sn00pster Yes, this looks great! Just one observation and one question. The observation: I think the latest package is 1.1.6. It might better to use that. The question: Does this actually solve the original problem you investigated? |
|
Humn, I just downloaded the archive and that is 1.1.5, the release list of archives only showed this as the latest. Looking at the CVS there is a later version but it looks like there were no real changes, just changes to documentation. I’ll see if they have a later archive version, I just assumed the archive on the official page would be the latest..... And no, it doesn’t fix the issue, I need to investigate why utmp doesn’t work with it, but this was a prerequisite for that support. |
|
OK. Does that mean, your problem has been solved, then? |
|
Sorry, no the problem has not been solved. I was hopeful that just adding this library would, but unfortunately it doesn’t. However, this library is required to enable the feature I’m after, so having it build and be used is a requirement for getting the feature working. I need to debug the actual mosh server itself to see why it’s not adding the records to utmp, once I’ve done that hopefully it will work. This work that I’ve done so far would also allow somebody else to debug the issue. |
|
Just as an aside, I checked the FTP site and they have 1.1.6 on there (they just haven't updated the link), so modified it for 1.1.6, still no change in that it doesn't work, but as least it's now the latest version. Now I need to figure out the best way of debugging the issue at hand! |
|
Right. I'm afraid I won't be able to help you much more then. I only have one additional recommendation: It might be worthwhile to ensure that Otherwise, I'd probably look for |
|
It’s definitely being used, I’ve checked the binary and also the configure shows it being detected.
I’ve already put a post on the mosh development list asking for information on how I go about debugging this seeing as it’s a client server program with ssh involved for part of it! Might be my old faithful printf debugging! Lol
Thanks for all your help.
Adrian
…On 18 Jan 2019, 15:48 +0000, m4tt075 ***@***.***>, wrote:
Right. I'm afraid I won't be able to help you much more then. I only have one additional recommendation: It might be worthwhile to ensure that libutempter is REALLY automatically detected and correctly linked into mosh. As you basically had to "hardcode" the library and include pathes in the libutempter cross-package, automatic detection in mosh might(!) fail. I've epxerienced that with other packages. If so, you might have to add configuration arguments in the cross/mosh/Makefile to fix it.
Otherwise, I'd probably look for mosh experts on other forums to help you debug.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
Any easy way of setting the build environment to an architecture?
Just so I can compile a c program with gcc -o test test.c
And then copy to the Synology and run it, I figure the easiest way to figure out what is happening is to write a little bit of simple code to manually modify the utmp file, if that works then I can take it from there.
Utmp is pretty simple.
…On 18 Jan 2019, 15:48 +0000, m4tt075 ***@***.***>, wrote:
Right. I'm afraid I won't be able to help you much more then. I only have one additional recommendation: It might be worthwhile to ensure that libutempter is REALLY automatically detected and correctly linked into mosh. As you basically had to "hardcode" the library and include pathes in the libutempter cross-package, automatic detection in mosh might(!) fail. I've epxerienced that with other packages. If so, you might have to add configuration arguments in the cross/mosh/Makefile to fix it.
Otherwise, I'd probably look for mosh experts on other forums to help you debug.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
|
||
| .PHONY: libutempter_post_patch | ||
| libutempter_post_patch: | ||
| $(RUN) sed -e 's#@INSTALL_PREFIX@#$(STAGING_INSTALL_PREFIX)#' -i Makefile |
There was a problem hiding this comment.
I am interested in a more "elegant" way to apply STAGING_INSTALL_PREFIX at "install" step... maybe overriding "DESTDIR" as make install command option may do the trick
There was a problem hiding this comment.
I’m really not familiar at all with spksrc, with matts help I’ve managed to get this far with libutempter but that’s the extent of my knowledge, I don’t know where to start with doing this another way!
There was a problem hiding this comment.
I need a bit of a “you need to do these changes” help for this, I’m new to spksrc and generally Makefiles (30 years of commercial development and I’ve avoided Makefiles!).
| -libdir = /usr/lib | ||
| -libexecdir = /usr/lib | ||
| -includedir = /usr/include | ||
| -mandir = /usr/share/man |
There was a problem hiding this comment.
mandir has been discarded, so man3dir will be mangled. OK it does not matter in our case, but I like when things are clean.
There was a problem hiding this comment.
I’ll take a look tomorrow at this, I might have just got over zeleous
|
This lib may be useful to tmux either: https://unix.stackexchange.com/questions/181824/compile-static-tmux-with-libutempter-support |
|
Right, the good news is that writing a small c program to manually fudge an entry into utmp works, so whatever the issue is it should be resolvable with a bit of debugging. |
Good to know this work might be usable for something else in synocommunity! |
|
Ok, I think I know what the issue is....I didn’t realise the library was quite so trivial and that it was an interface to a separate binary, so although I’ve compiled and linked to the library, I haven’t bundled the helper application that it builds and that actually does the work! This should work fine when I’ve made that change. It will have to wait a few days as I’m currently at hospital having a liver transplant assessment done! |
|
I have this working now, but I am not at a computer and have done the work remotely on my iPad, when I'm back at the computer tomorrow I will finish up the work and submit a new pull request. |
|
Yay! Congrats! Looking forward to the new pull request! |
|
Any idea how we deal with permissions and groups? the permissions and group need to be modified on the /var/run/utmp file, as per normal Synology behaviour a reboot of the NAS resets any changes made. The group and mode need to be changed on the file to allow it to run with setgid. Furthermore every user will need to be a member of the group, so how do we deal with existing and future users? |
|
Yes, can be done. You are entering the beautiful world of Synology (=ACL) permission management. I'd recommend reading the corresponding Wiki sections:
Then, you can look at the
Just get back to us if you get stuck... |
|
Thanks, I’m shattered from my assessment so this is going to be a job for tomorrow. |
|
@sn00pster Whenever you are ready. Keeping my fingers crossed for you! |
|
I’ve taken a quick look at that, I don’t think the way that package does it will work for this on, the issue is that the permissions and group on a Synology system file (var/run/utmp) get reset after a boot of the NAS. While using postinst would work straight away, it wouldn’t survive a reboot. I will take a look at the official plex spk as that has to do the same to the GPU driver for hardware transcoding. |
|
Hmh, not sure I understand correctly. Doesn't that file have read permissions already? Does |
|
Yeah, it needs write permissions so that it can tell the system that there’s a tty session opened by mosh.
The default permissions on the file mean that it can’t be written to.
…On 22 Jan 2019, 21:53 +0000, m4tt075 ***@***.***>, wrote:
Hmh, not sure I understand correctly. Doesn't that file have read permissions already? Does libutempter really have to modify it?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
Ugh, that's ugly. Packages can run as root. They did in the past. But all the premission changes, those by Synology, and subsequently here, had the main objective to avoid that... |
|
Quick question, In the past I hand crafted an spk for a much newer version than what Synology provided, I now no longer use docker on the Synology so have not bothered maintaining that any longer. From memory there’s a script which gets called on start or stop of the service and I believe that it runs as root, so any objections if inside there I do th relevant chmod and chggrp commands so that th file becomes available to members of a utmp group? Unless there’s a cleaner way? I’ve verified that the group and permission changes work perfectly, so we’re nearly there with support for this. |
|
Thinking about this, the best way of sorting the group issue is to set the group of the file to users, any objections to this? |
|
Give it a try, but I don't think this works, as I think(!) the group still only has read permissions on that file, hasn'T it? I believe the only "clean" solution is to use the |
|
It still requires the chmod to users group, but I figured using the users group is a better bet than creating a new group which would require all existing and future users to be part of the group, using users alleviates this problem.
It can’t be run as a service, it doesn’t work that way unfortunately.
I’m hoping that my memory is correct and that start-stop-status runs as root. The recreating of system files on reboot is a pain on DSM.
…On 23 Jan 2019, 18:57 +0000, m4tt075 ***@***.***>, wrote:
Give it a try, but I don't think this works, as I think(!) the group still only has read permissions on that file, hasn'T it? I believe the only "clean" solution is to use the SERVICE functionalities and run the package as root. A viable approach is outlined in our Wiki (scroll down): https://github.com/SynoCommunity/spksrc/wiki/Service-Support. Never had to use this option myself so far though...
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
I'm pretty sure the Just to be clear, I don't think that you should run |
|
Ok, so I looked at this briefly earlier, the synocli-net package does not run as a service, that is to say the only option available in the package manager is “uninstall”, there is no start stop option. This makes sense for mosh as the initial connection is made over ssh and the mosh server is run and then ssh disconnects. I’m going to have to do a bit more delving I think. Apologies for seeming a bit dumb about this, I greatly appreciate all the help you’ve given me, I wouldn’t be at this point without you! |
|
@sn00pster You have been coming a long way with only very little! And To be honest, I forgot that |
|
I’m pretty stuck :( I’m out of my depth here with the whole spksrc architecture. My only other thought (which does work, but it’s not nice) is to create a task in the scheduler which runs at boot and as root and sets the permissions and group for the /var/run/utmp to 664 and users respectively. It’s not a pretty solution. I don’t know whether this can be automated with spksrc and even if it could whether it’s anywhere near an appropriate thing to be doing, it seems a very hacky way of doing it. |
|
I have found that old "cross/openssh" has disabled "utmp" support probably because of same permission troubles... Probably mosh and tmux binaries has to have "setuid" flag set to operate properly. Will do some testing... |
|
Yeah, it requires setuid on the helper binary. Sorry, I forgot to sort out the rest of the changes I made, I will try to do that today and push them, but to get it to work I had to create a task to set the permissions so that the utmp file can be written. It’s actually quite important that mosh is built with a working libutempter otherwise you could potentially end up with a lot of stale sessions running and be unaware of this. |
|
Ok, I seem to have lost the rest of the changes. :( The fix I just pushed is required because the helper application doesn't seem to like checking for the std io and fails under DSM, so I just removed them so that the helper works. The rest of the changes were to: 1.) Modify the make file so that the libexec points to the installed target These are minor changes that would take an experienced dev here a minute to perform, I sadly don't have the time at the moment to recreate these as it will require me digging once again through the spksrc documentation. Once these are done, you should have a working libutempter and helper binary, but due to the permissions problem it still will not work, which is why I use a startup task to set them. I'm hopeful somebody else can run with this, as I mentioned earlier this is an important requirement for mosh, I'd even say that it shouldn't be an optional thing in the mosh code because without it you can end up with a lot of stale sessions. |
|
Have you done that changes manually on DSM or in a package itself? If so, you can upload in comment your binary package and I will reverse-engineer your "changes". |
|
The changes were done in the work folder, I checked this morning but they were gone (I just have cleaned it by accident), however, along with the modification I mentioned earlier they were simply changes to the make file so that the library picks the correct place for the helper binary (lib exec), and also makes sure the binary is included in the installation and the patch I provided earlier today to the helper binary.
As long as the library can call the helper binary you’re in the same position as me, with a problem with permissions, I worked around it with a startup task but that’s obviously not ideal.
I have a working binary package (once again that requires the permissions be set correctly on the utmp file) but I’m not sure how useful it will be as the changes are mostly source level.
I know very little about how DSM handles permissions or how/why it insists on resetting everything upon reboot, but it definitely caused issues here.
…On 9 Feb 2019, 10:21 +0000, Yves Martin ***@***.***>, wrote:
Have you done that changes manually on DSM or in a package itself? If so, you can upload in comment your binary package and I will reverse-engineer your "changes".
For permissions on that utmp file, is it possible to use ACL? By the way, I guess DSM update flushes them too.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
I no longer have a synology as I have replaced it with a Unraid server, hopefully somebody else will pick up where I left off. |
|
@sn00pster I agree. You have come a long way. Many thanks for your contribution and all the best for your Unraid experience... |
|
I have worked on this, including utempter into tmux (which is easier/faster to test) but cannot confirm as "last" does not exist on DSM, I tried to confirm with "w" from "synogear" package... but I am not sure yet tmux properly enlists user login/logout thanks to utempter... |
|
Hello @hgy59 |
| - if (fstat(i, &sb) < 0) | ||
| - /* At this stage, we shouldn't even report error. */ | ||
| - exit(EXIT_FAILURE); | ||
| - } |
There was a problem hiding this comment.
any idea what this code is for?
what it the reason to remove it?
There was a problem hiding this comment.
It doesn't work on synology, I removed it because it caused a failure and it doesn’t actually do anything than check the std channels.
There was a problem hiding this comment.
for info I’m sn00pster, I created a new github account.
mosh now builds against libutempter, this is a requirement for adding support for displaying detached sessions.
Motivation: Trying to add support to show detached sessions upon connection.
Linked issues: #3582
Checklist
all-supportedcompleted successfully