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

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 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 expert-C-API 3.7 3.8 3.9 3.10 3.11 build The build process and cross-build labels Oct 11, 2021
    @vstinner
    Copy link
    Member

    vstinner commented Oct 11, 2021

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

    @vstinner
    Copy link
    Member

    vstinner commented Oct 11, 2021

    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 expert-C-API 3.7 3.8 3.9 3.10 labels Oct 11, 2021
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.11 build The build process and cross-build
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant