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

explore hashlib use of the Linux Kernel CryptoAPI #91258

Closed
gpshead opened this issue Mar 23, 2022 · 9 comments
Closed

explore hashlib use of the Linux Kernel CryptoAPI #91258

gpshead opened this issue Mar 23, 2022 · 9 comments
Labels
type-feature A feature request or enhancement

Comments

@gpshead
Copy link
Member

gpshead commented Mar 23, 2022

BPO 47102
Nosy @gpshead, @tiran
PRs
  • gh-91258: Linux Kernel CryptoAPI bindings #32173
  • bpo-46864: Suppress even more ob_shash deprecation warnings (GH-32176) #32176
  • 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 = None
    created_at = <Date 2022-03-23.19:30:13.093>
    labels = ['type-feature']
    title = 'explore hashlib use of the Linux Kernel CryptoAPI'
    updated_at = <Date 2022-03-29.15:34:13.058>
    user = 'https://github.com/gpshead'

    bugs.python.org fields:

    activity = <Date 2022-03-29.15:34:13.058>
    actor = 'christian.heimes'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = []
    creation = <Date 2022-03-23.19:30:13.093>
    creator = 'gregory.p.smith'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 47102
    keywords = ['patch']
    message_count = 6.0
    messages = ['415896', '415899', '415901', '415903', '415904', '416251']
    nosy_count = 2.0
    nosy_names = ['gregory.p.smith', 'christian.heimes']
    pr_nums = ['32173', '32176']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue47102'
    versions = []

    @gpshead
    Copy link
    Member Author

    gpshead commented Mar 23, 2022

    Linux kernels provide a CryptoAPI. This is a common place for platform specific hardware accelerated hash algorithms to be exposed to the user (especially on SoCs which often have non-standard hardware).

    https://www.kernel.org/doc/html/v4.10/crypto/userspace-if.html
    https://www.kernel.org/doc/html/v5.17/crypto/userspace-if.html
    https://www.chronox.de/libkcapi.html

    hashlib currently uses OpenSSL when possible for performance. We could also look at querying the kernel API. How to decide between the two implementations when both are present is something TBD.

    This would probably be best done via a configure time check for libkcapi?

    @gpshead gpshead added type-feature A feature request or enhancement labels Mar 23, 2022
    @tiran
    Copy link
    Member

    tiran commented Mar 23, 2022

    We don't need libkcapi. I added AF_ALG support a while ago:

    import binascii
    import os
    import socket
    
    with socket.socket(socket.AF_ALG, socket.SOCK_SEQPACKET, 0) as cfgsock:
        cfgsock.bind(("hash", "sha256"))
        opsock, _ = cfgsock.accept()
        with opsock:
            with open("/etc/os-release") as f:
                st = os.fstat(f.fileno())
                # blindly assumes that sendfile() exhausts the fd.
                os.sendfile(
                    opsock.fileno(), f.fileno(), offset=0, count=st.st_size
                )
                res = opsock.recv(512)
            print(binascii.hexlify(res))

    @gpshead
    Copy link
    Member Author

    gpshead commented Mar 23, 2022

    Neat. I've never used the API, just filing a breadcrumb suggesting we see if it makes sense. Being I/O based, that even takes care of GIL releasing from Python. :)

    @tiran
    Copy link
    Member

    tiran commented Mar 23, 2022

    test_socket has examples for HMAC, AES-CBC, and AES-GCM.

    with self.create_alg('hash', 'hmac(sha1)') as algo:
        algo.setsockopt(socket.SOL_ALG, socket.ALG_SET_KEY, b"Jefe")
        op, _ = algo.accept()
        with op:
            op.sendall(b"what do ya want for nothing?")
            self.assertEqual(op.recv(512), expected)

    @tiran
    Copy link
    Member

    tiran commented Mar 23, 2022

    And sendfile() is zero-copy. Data does not have to leave Kernel space.

    @tiran
    Copy link
    Member

    tiran commented Mar 29, 2022

    I figured out how to implement copy(). dup() does not work as expected, but accept() on an AF_ALG client socket creates an independent copy.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @gpshead
    Copy link
    Member Author

    gpshead commented Feb 6, 2023

    Playing with your draft PR branch and testing things out... the performance of the Linux kernel crypto API for hash algorithms is several times slower on a modern AMD Zen, regardless of data size.

    I realize it probably gets a tad better in limited situations where sendfile or splice can used, but it doesn't seem like a useful API on modern mainstream hardware. It could still make sense on low performance things that have non-userspace accelerator hardware. But this doesn't leave much motivation to implement code using it.

    This one sounds like it could be a bit faster https://github.com/cryptodev-linux/cryptodev-linux (and is what would be needed for this on FreeBSD and OpenBSD), but is not in mainline in Linux.

    @arhadthedev
    Copy link
    Member

    Playing with your draft PR branch and testing things out... the performance of the Linux kernel crypto API for hash algorithms is several times slower on a modern AMD Zen, regardless of data size.

    Probably, this should be added into the documentation if this issue gets rejected. Otherwise, there will be another attempts to introduce CryptoAPI, and one of them can even accidentally slip through.

    @gpshead
    Copy link
    Member Author

    gpshead commented May 20, 2023

    Probably, this should be added into the documentation if this issue gets rejected. Otherwise, there will be another attempts to introduce CryptoAPI, and one of them can even accidentally slip through.

    I'm not worried about that. This issue existing in our issue tracker is sufficient documentation.

    @gpshead gpshead closed this as not planned Won't fix, can't repro, duplicate, stale May 20, 2023
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants