[FreeBSD] User's login class ignored #572
Comments
|
Looking through XDM, which does not suffer from this problem, I thought that a call to setlogin(2) might be necessary after setgid(). So I patched it into Ly. Didn't help, though. Even if I find the solution, the question is whether this should be merged upstream. The current v0.6.0 FreeBSD port contains a bunch of OS-specific patches ontop of Ly. I don't know whether you intend to upstream all of this. Or whether everything BSD-specific should remain in the FreeBSD ports tree instead. |
|
Yeah, it seems I am also supposed to call setusercontext() instead of setuid(). From XDM: #ifndef HAVE_SETUSERCONTEXT
if (setuid(verify->uid) < 0) {
LogError ("setuid %d (user \"%s\") failed: %s\n",
verify->uid, name, _SysErrorMsg (errno));
return (0);
}
#else /* HAVE_SETUSERCONTEXT */
/*
* Set the user's credentials: uid, gid, groups,
* environment variables, resource limits, and umask.
*/
/* destroy user environment before calling setusercontext */
environ = verify->userEnviron;
pwd = getpwnam(name);
if (pwd) {
if (setusercontext(NULL, pwd, pwd->pw_uid, LOGIN_SETALL) < 0) {
LogError ("setusercontext for \"%s\" failed: %s\n",
name, _SysErrorMsg (errno));
return (0);
}
verify->userEnviron = environ;
endpwent();
} else {
LogError ("getpwnam for \"%s\" failed: %s\n",
name, _SysErrorMsg (errno));
return (0);
}
#endif /* HAVE_SETUSERCONTEXT */Will try this next. |
|
Yes, this worked. So I do have a working patch! Is there anybody feeling responsible for this repository at all? |
|
I officially submitted my patch to the FreeBSD ports tree. |
AnErrupTion
added
bug
|
I noticed that the default PATH from FreeBSD's |
|
@rhaberkorn Ly applies its own PATH which can be found in its configuration file. |
Does not appear to work. I am still trying to figure out where my PATH in X11 is actually coming from, so I can extend it. Is ly supposed to expand |
It's not supposed to do that, since Ly treats PATH as a big blob of characters and doesn't parse it in any way. It is simply set as an environment variable. I'm pretty sure that's the responsibility of the shell to expand the home directory, but I'm not 100% sure.
The problem with this is it would somewhat break portability, since we'd have to add a feature specific to FreeBSD. But then, if Ly does it, then what would the user expect if Ly's being run on FreeBSD? To source PATH from |
Yes, But anyway, the path inheriting from ly does not currently work anyway. The only way I found for extending PATH in X11 is via
I think ideally, the default value of the btw. I am still using the heavily-patched v0.6.0 version of ly from FreeBSD ports. We cannot switch to the Zig remake unless you integrate all of the FreeBSD-specific stuff. |
|
@rhaberkorn So, after quite a bit of thinking, I'm ready to merge as much FreeBSD-specific code as possible. Could you point me to where those patches are, please? That is, if you don't want to make a PR, of course. If you want to instead of me doing it, go ahead!
That's pretty odd. Perhaps
Sure, but in this case adding a comment above the
Eh? I thought you weren't able to do so? |
https://github.com/freebsd/freebsd-ports/tree/main/x11/ly/files You will notice that these patches are not organized by feature, but rather by file. Every patch file contains all the changes to one individual source file. Totally idiotic, but that's what most FreeBSD ports do!
That's right. I was just describing how it would ideally work. |
|
@rhaberkorn I've gone ahead and applied pretty much all of the FreeBSD authentication patches (like switching to utmpx which was done even a bit earlier in the commits) except the VT patch, since I have a few questions. Is there any fundamental difference between VTs and TTYs on FreeBSD? Especially when the default VT value is And, speaking of VTs, PR #623 tries to set the current active VT as the TTY at runtime: // This will set the current active VT as our TTY.
var vtstat: interop.vt.vt_stat = undefined;
_ = std.c.ioctl(std.c.STDIN_FILENO, interop.VT_GETSTATE, &vtstat);
config.tty = @intCast(vtstat.v_active);To do this, it includes the The other patches seem to simply be replacing (a lot of) paths. I wonder if I should make the Zig build script be able to patch some specific strings inside resources like the config file, the X/Wayland setup scripts, etc... to accodomate for this use case. This way we could also modify the default TTY ID that's in the config file. Sorry for the huge wall of text, haha! And it's no problem if you can't answer all the questions of course 😄 |
I have no idea why they introduced an additional
There is no such header or structure in FreeBSD. But I think you could use
Build systems typically allow that - should allow that. Take for instance Autoconf's
That's exactly the
|
|
@rhaberkorn I believe I have implemented pretty much all of the patches, except for the VT one, and one last which I forgot to mention because I'm not quite sure: in the #path = /sbin:/bin:/usr/local/sbin:/usr/local/bin:/usr/bin:/usr/sbin
+path = /sbin:/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/usr/bin/envIt swaps the |
I have no idea. But this is easy to patch in by the FreeBSD maintainer if it indeed serves any purpose. I will give the current version a try soon. |
I believe right now it won't build out of the box due to one Linux-specific (or at least I think it is) function, |
|
Actually there are other problems. Zig v0.12.1 from the FreeBSD ports comes with a C function definition that does not match the system headers (???): Although, I do not see where the problem is with And when I try Zig-devel-0.12.0.d.1879+e19219.f0, I get the following error: |
Ah yes. So what's happening here is that, for FreeBSD at least, Zig doesn't define the
That version is definitely too old. Ly needs the final version of v0.12.0 or newer. |
|
I've pushed the fix to master. If you still encounter such errors after that, you can try to manually modify the source code yourself like how I did it in the latest commit until it builds successfully. |
Why does it do that if it can parse the system headers in the first place? I thought that was a big selling point for Zig? Now it does that: Shouldn't that NOTE: I am still new to Zig, although I wanted to learn it anyway. |
It can do that, but the
That's the function I was talking about earlier. Zig does lazy compilation, which means that if a statement isn't used, it won't get compiled in. However, currently the |
You say that if the vt namespace is not used, it reliably won't even try to parse the header? Interesting. |
Yup, that's exactly what'll happen. As long as the |
|
Sorry, you have to serve as my Zig-mentor, but why the hell it cannot automatically cast Even more strangely, I declare a variable like Is that a Zig bug? |
|
@rhaberkorn What's happening here is that pub const passwd = if (builtin.os.tag == .freebsd) logincap.passwd else pwd.passwd;
pub const endpwent = if (builtin.os.tag == .freebsd) logincap.endpwent else pwd.endpwent;
pub const getpwnam = if (builtin.os.tag == .freebsd) logincap.getpwnam else pwd.getpwnam;I had also forgotten to use the |
I see, but that's very unintuitive. IMHO the compiler should at least make clear that he's talking about 2 different types. I think these problems could also be avoided if you used one big
This workaround could also be avoided if you had only one big
Not quite, but you will get the remaining changes from me once everything works. :-) It's compiling now, but I will still have to test it. |
I probably could do that, but I like having multiple import statements since it makes it clearer which headers provide which functions, unlike in C. |
Sure, but namespaces won't collide anyway as C programmers keep an eye on that. |
|
That does indeed sound like a better solution, which I've now pushed to master. |
|
ly still crashes in src/main.c:60, ie. in a defer block. Unfortunately, GDB does not show where this comes from (what caused returning from main). Will have to debug this further. |
It probably returned from a Zig error, in which case the terminal should give you more information. It'd be interesting to run this in a regular terminal (with no root permissions) to see if it still crashes, and if so, what error it returns. |
|
Okay I managed to login via the new version. Although I still see ly in the process tree. I believe it should have execed to the setup script or something. Does it no longer do that? I also still have a weird behavior with my virtual terminals, which is probably related to the A few remarks:
|
|
And logging in with the new Ly worked only manually from one of the VTs. If I try to start it at as the login process, it fails. Unfortunately, the system does not tell me why it's failing, just that For Ly 0.6, things were as follows: You would have getty with the "Ly" session on ttyv8. getty would exec to Ly according to /etc/gettytab. Ly would then have The missing |
I don't think Ly was ever supposed to do that. Right now, what it does is fork() when starting authentication, then fork() again when actually starting the shell, X11 or Wayland session. With X11 however, it forks a bit more (for xauth, the X server & the actual WM/DE respectively). It needs to do that so it can clean up at the end & handle logouts as well.
I'm assuming that happens when logging inside an X11 session. I noticed that the X server allows you to specify a
Hm, I wasn't aware of this option being available. It looks like you can also add custom install files, so I may end up using that instead of the custom option before releasing Ly v1.1.0. For now though, it works, so I'll leave it as-is.
The problem with this is that most Linux distributions, as far as I'm aware, don't symlink packaged binaries to /usr/local/bin, so unless you override the prefix directory yourself as well when building from source, it most likely won't work.
You mean when using
I'd expect Ly to be executed instead of getty, not for getty to execute Ly. Perhaps you can try to disable getty on ttyv8? If it's even possible, that is. |
I don't get that. Both
I honestly don't know why they are doing that but I already tried with something like that in /etc/ttys: Didn't make a difference.
I am still confused. It seems that Does the Zig build system not foresee a separate build-stage (cf. It's a recurring theme. People - especially language designers - underestimate the complexity of build systems and think they are unnecessarily bloated. They then reimplement them, supposedly simplifying things, but end up with idiosyncratic and half-working systems, that won't integrate into existing infrastructure and expectations, need to be patched to be usable etc. If you need to reimplement proper installation behavior from scratch, you would be better off using Autotools, CMake, Meson(?) or just properly written Makefiles. |
|
Reading through the documentation, I fear that "reinventing the wheel poorly" is exactly what was done here. The "install" target is supposed to create some local directory that includes the complete directory hierarchy.
|
What I mean is the paths in the config file. Some, like
Does the same thing happen if you try to spawn Ly on
It technically does the correct thing- it builds the binary and puts it in an
That's actually a pretty fair change I should probably make.
I'm personally fine with making a Makefile wrapper over the current build system if it makes packaging easier. And, with the proper custom stages in the Zig build system, we shouldn't have to entirely rely on the Makefile for building.
I think that's fair, since with Makefiles you also need to handle the
I more or less agree with idea, but let's not forget that Zig is still pre-v1.0.0, so voicing concerns right now is best before it reaches that milestone, IMO. |
Yes, you are right. And in fact you cannot that easily predict where On Autotools and CMake you could also find these tools wherever they are installed, make them configurable, and use the absolute paths further on.
After thinking about it a bit, I guess they could get away with it. The
Probably true, but it's nothing I want to take care of now. Enough side projects for now. Should I ever start something in Zig from scratch, I will probably get more involved. |
If you mean each one individually, the number of options can get rather bloated with that.
That's a possible feature, using the
I think the less patches needed downstream, the better. |
Hello!
I am on FreeBSD 14. Ly version 0.6.0. I was configuring a special login class for my user. Login classes allow customizing resource limits on FreeBSD. An ordinary VT login respects my settings and sets the correct login class for the login shell. Ly does not unfortunately. It appears that login(1) is responsible on VTs for setting the user's login class.
I am happy to build and test and even create a pull request. Some pointers on where to start looking would be appreciated, though.
Best regards!
The text was updated successfully, but these errors were encountered: