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

Jump to bottom

libpython should not be linked with libcrypt #89596

Closed
floppym mannequin opened this issue Oct 11, 2021 · 3 comments
Closed

libpython should not be linked with libcrypt #89596

floppym mannequin opened this issue Oct 11, 2021 · 3 comments
Labels
3.11 only security fixes build The build process and cross-build

Comments

@floppym
Copy link
Mannequin

floppym mannequin commented Oct 11, 2021

BPO 45433
Nosy @vstinner, @floppym, @thesamesam
PRs
  • bpo-45433: Do not link libpython against libcrypt #28881
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
closed_at = <Date 2021-10-11.23:24:50.373>
created_at = <Date 2021-10-11.18:11:14.569>
labels = ['build', '3.11']
title = 'libpython should not be linked with libcrypt'
updated_at = <Date 2021-10-12.02:58:00.239>
user = 'https://github.com/floppym'

    bugs.python.org fields:

    activity = <Date 2021-10-12.02:58:00.239>
actor = 'thesamesam'
assignee = 'none'
closed = True
closed_date = <Date 2021-10-11.23:24:50.373>
closer = 'vstinner'
components = ['Build']
creation = <Date 2021-10-11.18:11:14.569>
creator = 'floppymaster'
dependencies = []
files = []
hgrepos = []
issue_num = 45433
keywords = ['patch']
message_count = 3.0
messages = ['403663', '403701', '403702']
nosy_count = 3.0
nosy_names = ['vstinner', 'floppymaster', 'thesamesam']
pr_nums = ['28881']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'compile error'
url = 'https://bugs.python.org/issue45433'
versions = ['Python 3.11']
    @floppym
    Copy link
    Mannequin Author

    floppym mannequin commented Oct 11, 2021

    In https://bugs.python.org/issue44751, crypt.h was removed from Python.h. This would imply that libpython is not meant to expose any crypt-related symbols.

    In fact, it looks like libpython does not use crypt() or crypt_r() at all. These are only used by cryptmodule.

    In configure.ac, we have this this logic to determine if crypt_r() is available:

    # We search for both crypt and crypt_r as one or the other may be defined
# This gets us our -lcrypt in LIBS when required on the target platform.
AC_SEARCH_LIBS(crypt, crypt)
AC_SEARCH_LIBS(crypt_r, crypt)

AC_CHECK_FUNC(crypt_r,
  AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
#define _GNU_SOURCE  /* Required for crypt_r()'s prototype in glibc. */
#include <crypt.h>
]], [[
struct crypt_data d;
char *r = crypt_r("", "", &d);
]])],
    [AC_DEFINE(HAVE_CRYPT_R, 1, [Define if you have the crypt_r() function.])],
    [])
)

    The AC_SEARCH_LIBS macro adds "-lcrypt" to LIBS, and this gets passed down to the link command for libpython. This is probably not intentional.

    The HAVE_CRYPT_R value is used in _cryptmodule.c. setup.py performs its own check for libcrypt when building the module.

    I think the value of LIBS should be saved before the AC_SEARCH_LIBS calls and restored after the AC_CHECK_FUNC call. I have tested this locally on Gentoo Linux, and it seems to work fine.

    I will work on a pull request.

    @floppym floppym mannequin added topic-C-API 3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes 3.10 only security fixes 3.11 only security fixes build The build process and cross-build labels Oct 11, 2021
    @vstinner
    Copy link
    Member

    New changeset be21706 by Mike Gilbert in branch 'main':
    bpo-45433: Do not link libpython against libcrypt (GH-28881)
    be21706

    @vstinner
    Copy link
    Member

    Nicely spotted, thanks for the fix!

    I prefer to not backport to avoid any risk of regression. In my experience, the build system is fragile.

    @vstinner vstinner added build The build process and cross-build and removed topic-C-API 3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes 3.10 only security fixes labels Oct 11, 2021
    @vstinner vstinner removed 3.9 only security fixes 3.10 only security fixes labels Oct 11, 2021
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @rcoup rcoup mentioned this issue Jan 12, 2023
    Merged
    rcoup added a commit to rcoup/vcpkg that referenced this issue Jan 12, 2023
    @rcoup
    python3: libcrypt doesn't need to be a dependency of libpython
    a89b54e 
    Fixed upstream in Python3.11 via python/cpython#89596

Backport here to Python3.10 so libpython doesn't have a needless system
dependency on libcrypt (which is different on Linux between RH-based and
Debian-based distros too).
    BillyONeal pushed a commit to microsoft/vcpkg that referenced this issue Jan 18, 2023
    @rcoup
    python3: libcrypt doesn't need to be a dependency of libpython (#28887)
    d2b8d93 
    Fixed upstream in Python3.11 via python/cpython#89596

Backport here to Python3.10 so libpython doesn't have a needless system
dependency on libcrypt (which is different on Linux between RH-based and
Debian-based distros too).
    @JorjMcKie JorjMcKie mentioned this issue Aug 24, 2023
    Closed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    3.11 only security fixes build The build process and cross-build
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    1 participant
    @vstinner