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

OpenSSL 1.1.1 still implements some disable-flags for Blake2, Scrypt #89790

Closed
commodo mannequin opened this issue Oct 27, 2021 · 7 comments
Closed

OpenSSL 1.1.1 still implements some disable-flags for Blake2, Scrypt #89790

commodo mannequin opened this issue Oct 27, 2021 · 7 comments
Assignees
Labels
3.10 only security fixes 3.11 only security fixes build The build process and cross-build extension-modules C modules in the Modules dir topic-SSL

Comments

@commodo
Copy link
Mannequin

commodo mannequin commented Oct 27, 2021

BPO 45627
Nosy @tiran, @commodo
PRs
  • bpo-45627: handle OPENSSL_NO_SCRYPT and OPENSSL_NO_BLAKE defines (GH-29237) #29237
  • 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 = 'https://github.com/tiran'
    closed_at = <Date 2021-10-28.08:14:23.895>
    created_at = <Date 2021-10-27.13:16:46.947>
    labels = ['extension-modules', 'expert-SSL', 'build', '3.10', '3.11']
    title = 'OpenSSL 1.1.1 still implements some disable-flags for Blake2, Scrypt'
    updated_at = <Date 2021-10-28.09:02:25.805>
    user = 'https://github.com/commodo'

    bugs.python.org fields:

    activity = <Date 2021-10-28.09:02:25.805>
    actor = 'Alexandru Ardelean'
    assignee = 'christian.heimes'
    closed = True
    closed_date = <Date 2021-10-28.08:14:23.895>
    closer = 'Alexandru Ardelean'
    components = ['Build', 'Extension Modules', 'SSL']
    creation = <Date 2021-10-27.13:16:46.947>
    creator = 'Alexandru Ardelean'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 45627
    keywords = ['patch']
    message_count = 7.0
    messages = ['405093', '405094', '405098', '405150', '405159', '405162', '405164']
    nosy_count = 2.0
    nosy_names = ['christian.heimes', 'Alexandru Ardelean']
    pr_nums = ['29237']
    priority = 'normal'
    resolution = 'wont fix'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue45627'
    versions = ['Python 3.10', 'Python 3.11']

    @commodo
    Copy link
    Mannequin Author

    commodo mannequin commented Oct 27, 2021

    This follows update https://bugs.python.org/issue43669

    Which is present in Python 3.10

    Some OpenSSL 1.1.1 can be built without Blake2 support or Scrypt.

    SHA3 and SHAKE do not seem to have any enable/disable flags.

    This results in compiler errors where EVP_blake2b512, EVP_blake2s256, EVP_PBE_scrypt and PKCS5_v2_scrypt_keyivgen can be un-defined.

    This is unfortunate behavior on the part of OpenSSL 1.1.1.

    So, for BLAKE2 and SCRYPT, we should still check that the OPENSSL_NO_SCRYPT and OPENSSL_NO_BLAKE2 defines are not-define.

    @commodo commodo mannequin added 3.10 only security fixes 3.11 only security fixes labels Oct 27, 2021
    @commodo commodo mannequin assigned tiran Oct 27, 2021
    @commodo commodo mannequin added build The build process and cross-build extension-modules C modules in the Modules dir topic-SSL 3.10 only security fixes 3.11 only security fixes labels Oct 27, 2021
    @commodo commodo mannequin assigned tiran Oct 27, 2021
    @commodo commodo mannequin added build The build process and cross-build extension-modules C modules in the Modules dir topic-SSL labels Oct 27, 2021
    @commodo
    Copy link
    Mannequin Author

    commodo mannequin commented Oct 27, 2021

    Note: PKCS5_v2_scrypt_keyivgen() will not cause an error, but EVP_PBE_scrypt() will

    @tiran
    Copy link
    Member

    tiran commented Oct 27, 2021

    Python 3.10 and newer require OpenSSL 1.1.1+ with blake2, sha3, scrypt, and similar features enabled. The required feature set is specified in PEP-644, https://www.python.org/dev/peps/pep-0644/#compatibility . Python requires the features, because I want to keep our test matrix small and guarantee the presence of a minimal feature set.

    Why do you want to build OpenSSL without blake2 or scrypt?

    @commodo
    Copy link
    Mannequin Author

    commodo mannequin commented Oct 28, 2021

    Apologies for the slow reply.

    It was the end of work-day when I submitted the bug & patch.
    I know, not a good method, but I do what I can :)

    So, OpenWrt's OpenSSL does not build BLAKE2 by default.
    See: https://github.com/openwrt/openwrt/blob/master/package/libs/openssl/Makefile#L190

    Scrypt is on by default. In the sense that there is no disable flag.

    I only care about BLAKE2, but I was trying not to half-ass the implementation, given that Scrypt is also disable-able.

    Now there are 2 options that I feel could be reasonable (anyone is free to disagree with me here):

    1. this is patch upstream as I'm trying here
    2. keep this patch downstream (i.e. just in our tree for OpenWrt's Python) until OpenSSL (or OpenWrt) enables BLAKE2 by default

    There are several options that feel a bit more difficult (to me):
    3. Enable OpenSSL BLAKE2 on by default in OpenWrt core; this may also work, but requires some discussion with the OpenWrt core-team that handles OpenSSL; though usually OpenWrt tries to be minimal, so I'm feeling there would be some resistance
    4. Not build hashlib (in Python) if BLAKE2 is not enabled in OpenSSL ; this is doable, but maybe a bit too complicated (for what is worth); it can cause support issues (in OpenWrt) like "hey, where did hashlib go?"

    I may have missed a few possible options. But these are the more obvious ones.

    Moving forward, I am fine with either of the first 2 options.
    I am feeling that option 2 is closer to what is desired by upstream Python (which I am fine to do).

    Thank you :)
    Alex

    @commodo
    Copy link
    Mannequin Author

    commodo mannequin commented Oct 28, 2021

    So, I've read through:

    https://www.python.org/dev/peps/pep-0644/#compatibility

    I also stumbled over:
    https://lwn.net/Articles/841664/

    Which also led to:
    https://www.python.org/dev/peps/pep-0644/#libressl-support

    The answer is loud and clear now: will keep this downstream.

    Thank you and apologies for the noise.

    @commodo commodo mannequin closed this as completed Oct 28, 2021
    @commodo commodo mannequin closed this as completed Oct 28, 2021
    @tiran
    Copy link
    Member

    tiran commented Oct 28, 2021

    Ah, OpenWRT. :)

    Thanks for you detailed explanation. I kinda agree that it makes sense for small, embedded systems like OpenWRT to reduce the size of binaries. After all storage and memory are precious on these systems.

    PEP-644 favors usability over flexibility. 3rd party library developers can now rely on the presence of hashlib features. Before 3.10 they could not be sure that a Python build had certain features.

    A downstream patch in OpenWRT makes sense.

    By the way, you might be interested in --with-builtin-hashlib-hashes configure flag. It lets you disable builtin modules like _md5 and _sha3. The shared libraries are large. If you disable the builtin blake2 module, then certain features like MAC and tree hashing are not working.

    @commodo
    Copy link
    Mannequin Author

    commodo mannequin commented Oct 28, 2021

    Thanks for you detailed explanation. I kinda agree that it makes sense for small, embedded systems like OpenWRT to reduce the size of binaries. After all storage and memory are precious on these systems.

    So, OpenWrt is not as tiny as it used to be.
    A large part is due to the Linux kernel increasing in default size.
    But other libs do a small increase as well (with each new revision).
    I think a new micro-OpenWrt (or similar project) would be required in some future to support the devices with less than 4 MB of flash and less than 32 MB of RAM.
    OpenWrt last supported the 4/32 combination in 2018 (with the 18.06 release).
    (But that's another topic I for when I retire maybe :p )

    I think where users still install Python is where they have at least 64/128 MB of persistent storage/flash memory.

    I am still surprised about the desire of users to install Python on such devices (even after doing this for ~7 years of [co-]maintaining it it on OpenWrt).
    The Turris devices/routers (are one example that) use a lot of CPython for some userspace stuff.

    PEP-644 favors usability over flexibility. 3rd party library developers can now rely on the presence of hashlib features. Before 3.10 they could not be sure that a Python build had certain features.

    Ack.

    A downstream patch in OpenWRT makes sense.

    I've started down that path.
    But guess what... I hit LibreSSL (on the host build) :P
    Now I have a bigger job (than I thought) to update to 3.10
    But that's fine.
    These events are rare, and I do enjoy to fix them (when I can).

    By the way, you might be interested in --with-builtin-hashlib-hashes configure flag. It lets you disable builtin modules like _md5 and _sha3. The shared libraries are large. If you disable the builtin blake2 module, then certain features like MAC and tree hashing are not working.

    I would not add too many flags in the Python build (on our side).
    For now we have some ways to "modularize" a Python installation.
    It's basically a hack-job of splitting Core-Python libraries into separate packages.
    That's when someone really wants to do a minimal installation of Python, and add only 2-3 libraries on top.
    Essentially, we build Python with all defaults possible, then some libs (.so & .py files) get packaged together (like ssl.py and _ssl.so from python).
    The more immediate benefits (in this example) are that you can avoid installing OpenSSL libs on your device, but have some Python.

    I know... crazy stuff, but people seem to do it.

    I do admit some Python fans and developers would cringe at this but ¯\(ツ)

    @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.10 only security fixes 3.11 only security fixes build The build process and cross-build extension-modules C modules in the Modules dir topic-SSL
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant