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

Fix misplaced library location and update to samba 4.21.0 #30389

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

gdonval
Copy link

@gdonval gdonval commented Oct 9, 2024

Fixes misplaced libraries (mistakenly put origin's /usr/lib/samba into /usr/lib).

@gdonval gdonval force-pushed the samba_fix_n_update branch from ed85a16 to 39c2152 Compare October 9, 2024 10:47
@gdonval
Copy link
Author

gdonval commented Oct 9, 2024

New samba requires lmdb-dev to build and updated patch... Do we still need that patch? There was no documentation attached.

@gdonval gdonval force-pushed the samba_fix_n_update branch from 39c2152 to e8e8f31 Compare October 9, 2024 12:11
@gdonval gdonval force-pushed the samba_fix_n_update branch 3 times, most recently from 096bca3 to 6610fa2 Compare October 9, 2024 16:24
@hbh7
Copy link
Member

hbh7 commented Oct 9, 2024

Thanks for the additional fixes. The build failure is the same one I've been looking at in the other PR.
2024-10-08T22:21:27.6985538Z 2024/10/08 22:21:27 WARN mv: can't rename '/home/build/melange-out/samba/usr/lib/samba/libtrusts-util-private-samba.so': No such file or directory subpackage=samba-libs. I'm hoping to find out how the list of lib mvs was generated.

@gdonval
Copy link
Author

gdonval commented Oct 10, 2024

Alright, I cleared the cache and I get the same error... Let me try a couple things.

@gdonval gdonval force-pushed the samba_fix_n_update branch 3 times, most recently from d775272 to 53e50ec Compare October 10, 2024 13:17
@gdonval
Copy link
Author

gdonval commented Oct 10, 2024

I don't know what to say other that "this compiles at home":

image

The elastic-build is frustrating as we can't access it to see what is wrong.

@hbh7
Copy link
Member

hbh7 commented Oct 10, 2024

Can you explain the rationale behind the additional build flags you added? Thanks!

@gdonval
Copy link
Author

gdonval commented Oct 10, 2024

Can you explain the rationale behind the additional build flags you added?

All but --without-smb1-server are actually default values (just like existing --with-ads or --with-pam). It's to make them explicit so that, if they change in the future, we don't get weird breakage in userspace. Here is a proof:

$ ./configure --help
...
  --with-ads
            Build with ads support (default=yes)
...
  --with-pam
            Build with pam support (default=yes)
...
  --with-acl-support
            Build with acl-support support (default=yes)
...
  --with-ldap
            Build with ldap support (default=yes)
...
  --with-smb1-server
            Build smbd with SMB1 support (default=yes).
...
  --with-pammodulesdir=PAMMODULESDIR
          Which directory to use for PAM modules
          [STD-Default: ${LIBDIR}/security]
          [FHS-Default: ${LIBDIR}/security]
...

Again, the point is to make sure that a future update won't pull the rug under us. This could go in another commit but as I've alluded to, I'm going to rework subpackages anyway.

As for LIBDIR

...
    --libdir=LIBDIR
            object code libraries [EXEC_PREFIX/lib]
...
    --exec-prefix=EXEC_PREFIX
            installation prefix for binaries [PREFIX]
...
    --prefix=PREFIX
            installation prefix [default: '/usr/local/samba']
...

Melange's default PREFIX is /usr. EXEC_PREFIX defaults to /usr and LIBDIR default to /usr/lib. Again, the point is to avoid a rug pull. pam is a bit picky about where it looks for modules so melange ever decided to change its defaults, we won't get this part breaking.

The last part, the only one that differs from defaults, sort-of, is --without-smb1-server. I thought this was disabled by default but it's only been deprecated for years, with the last OS supporting only that version having been out-of-support for a decade. Samba does not build SMB1 client support anymore (and I'm not sure it still can). There is literally no reason to build SMB1 in smbd for it not to be enabled ever (it really should not be enabled). So I removed it.


Once again, I'll resplit the package cleanly to make it all work.

@hbh7
Copy link
Member

hbh7 commented Oct 10, 2024

Much appreciated!

@gdonval gdonval marked this pull request as draft October 14, 2024 16:18
@gdonval gdonval force-pushed the samba_fix_n_update branch from 53e50ec to 6b86721 Compare October 14, 2024 16:19
@gdonval
Copy link
Author

gdonval commented Oct 14, 2024

Status is that I've mostly split things up into small pieces (so that the whole 50 MB of samba are not pulled every time someone "just" needs smbclient).

However so-dep detection in melange is flawed: they don't detect the use of rpaths.

Opened a bug report here and identified where shit hits the fan. I'll try to submit code tomorrow to get detection running. It's a matter of scanning the elf object for RPATHS and try that first (that's how it works under Linux). I suggest keeping the slashes in the handles like so:samba/libfoo.so when it's non-standard. Anyway, I'll fix that there then this can go forward.

@hbh7
Copy link
Member

hbh7 commented Oct 15, 2024

@gdonval have you seen #30765? Looks like some similar changes have been made.

@gdonval
Copy link
Author

gdonval commented Oct 15, 2024

Yeah... It does not solve my original problem, which is that private libs are copied in the wrong place. Those packages are also huge and contain a lot of attack surface for a piece of software that constantly have CVEs to its name...

I understand why those libraries were copied in a wrong place though: melange does not detect them when they are not in normal system folders.

I'll submit my changes to melange tomorrow for it to detect libraries in binary's rpath/runpath. Once that's done, I'll submit an updated finer-grained melange file.

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.

2 participants