Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: Use -module for sudo_noexec.la #102

Closed
wants to merge 1 commit into from
Closed

Conversation

orbea
Copy link

@orbea orbea commented May 13, 2021

When building sudo with slibtool-0.5.34 or newer (https://dev.midipix.org/cross/slibtool) it will fail.

rdlibtool --tag=disable-static --mode=install /bin/sh ../scripts/install-sh -c -o 0 -g 0 -m 0644 sudo_noexec.la /tmp/delme/usr/local/libexec/sudo

rdlibtool: lconf: {.name="libtool"}.
rdlibtool: fdcwd: {.fdcwd=AT_FDCWD, .realpath="/tmp/sudo/src"}.
rdlibtool: lconf: fstatat(AT_FDCWD,".",...) = 0 {.st_dev = 45, .st_ino = 31051}.
rdlibtool: lconf: openat(AT_FDCWD,"libtool",O_RDONLY,0) = -1 [ENOENT].
rdlibtool: lconf: openat(AT_FDCWD,"../",O_DIRECTORY,0) = 3.
rdlibtool: lconf: fstat(3,...) = 0 {.st_dev = 45, .st_ino = 29805}.
rdlibtool: lconf: openat(3,"libtool",O_RDONLY,0) = 4.
rdlibtool: lconf: found "/tmp/sudo/libtool".
rdlibtool: error logged in slbt_exec_install_entry(), line 516: path not found: .libs/sudo_noexec.so.def.host.
rdlibtool: < returned to > slbt_exec_install(), line 875.
make[1]: *** [Makefile:243: install-noexec] Error 2
make[1]: Leaving directory '/tmp/sudo/src'
make: *** [Makefile:190: install] Error 2

This is because of sudo contains a hack for darwin which builds libsudo_noexec.la and then manually edits and renames the file to sudo_noexec.la instead of just compiling it with -module. Additionally due to unrelated bug fixes slibtool exposed this issue in the recent 0.5.34 release which now requires host information during install mode which will not be present with such a hack. Not to mention that manually editing and renaming .la files is entirely non-portable and only works with GNU libtool because it is an inactive project which is far less permissive than slibtool.

Unfortunately I do not have a darwin system to test, but if this is still a problem there I strongly suggest that a better solution is found on that platform.

@orbea
Copy link
Author

orbea commented May 13, 2021

Also please see this downstream issue:

https://bugs.gentoo.org/789711

@millert
Copy link
Collaborator

millert commented May 13, 2021

The problem is that modules and shared libraries are not the same on all platforms. In the case of macOS (darwin), you cannot use a module with the LD_PRELOAD equivalent, it has to be a shared library. This hack is really to work around a GNU libtool restriction that shared libraries begin with "lib". Do you know if slibtool has this restriction? My inclination is to just remove this restriction from the libtool bundled with sudo and use -module on non-macOS platforms.

@orbea
Copy link
Author

orbea commented May 13, 2021

This hack is really to work around a GNU libtool restriction that shared libraries begin with "lib". Do you know if slibtool has this restriction?

Yes, libraries must start with lib unless they use -module as described in the automake documentation.

https://www.gnu.org/software/automake/manual/html_node/Libtool-Modules.html

Perhaps on darwin it can be build as libsudo_noexec.la without -module and as sudo_noexec.la with -module elsewhere?

millert added a commit that referenced this pull request May 13, 2021
On Darwin, shared modules and shared libraries are not interchangable
and since we preload sudo_noexec.so via DYLD_INSERT_LIBRARIES it
must be a library, not a module.  We must relax the requirement
that libraries begin with a "lib" prefix to work around this
difference.  This does mean you must use sudo's libtool on Darwin
(macOS) but that is already a requirement on other systems (notably
HP-UX and SCO) due to a number of libtool patches we require that
haven't be accepted upstream.  This is a different fix for PR #102.
@millert
Copy link
Collaborator

millert commented May 13, 2021

I'd like to keep the file name the same on both Darwin and other systems. I've committed changes to use -module on systems other than Darwin and relaxed the requirement that shared libraries begin with a "lib" prefix on sudo's bundled version of libtool. This does mean you must use sudo's version of libtool on Darwin but there are already patches present in sudo's libtool that haven't been committed upstream so this is not a new situation.

@millert millert closed this May 13, 2021
@orbea
Copy link
Author

orbea commented May 13, 2021

With slibtool building sudo_noexec.la without -module results in:

rdlibtool --tag=disable-static --mode=link gcc -Wl,--enable-new-dtags -Wl,--allow-multiple-definition -Wl,-z,relro -Wc,-fstack-protector-strong -ldl -o sudo_noexec.la sudo_noexec.lo -avoid-version -rpath /usr/local/libexec/sudo -shrext .so

rdlibtool: lconf: {.name="libtool"}.
rdlibtool: fdcwd: {.fdcwd=AT_FDCWD, .realpath="/tmp/sudo/src"}.
rdlibtool: lconf: fstatat(AT_FDCWD,".",...) = 0 {.st_dev = 45, .st_ino = 31051}.
rdlibtool: lconf: openat(AT_FDCWD,"libtool",O_RDONLY,0) = -1 [ENOENT].
rdlibtool: lconf: openat(AT_FDCWD,"../",O_DIRECTORY,0) = 3.
rdlibtool: lconf: fstat(3,...) = 0 {.st_dev = 45, .st_ino = 29805}.
rdlibtool: lconf: openat(3,"libtool",O_RDONLY,0) = 4.
rdlibtool: lconf: found "/tmp/sudo/libtool".
rdlibtool: error: output file prefix does not match its (libtool wrapper) suffix; the expected prefix was 'lib'
make[1]: *** [Makefile:180: sudo_noexec.la] Error 2
make[1]: Leaving directory '/tmp/sudo/src'
make: *** [Makefile:190: install] Error 2

@millert
Copy link
Collaborator

millert commented May 13, 2021

Since -module is now used to link sudo_noexec.la on all platforms except on Darwin I don't see why this is an issue for you.

@orbea
Copy link
Author

orbea commented May 13, 2021

Sorry I was not clear, I added that mostly for reference. I think this would actually be a problem for anyone that is using darwin + slibtool, but you're right I would not be personally affected.

Edit: For reference the relevant commit is: 2e49226

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants