Support mutable outputs for key derivation and ciphers to allow secure memory wiping

[HaZeShade]

Issue: Lack of Mutable Output for Secrets

Most (if not all) of Cryptography's systems that handle and/or generate sensitive material do not support a mutable output. HKDF().derive() for example:

def derive(self, key_material: utils.Buffer) -> bytes:
        utils._check_byteslike("key_material", key_material)
        return self._hkdf_expand.derive(self._extract(key_material))

This returns a bytes object, which is immutable. This is also true for other subsystems such as hazmat.primitives.ciphers.aead.AESGCM.

This entirely destroys any attempt at pure memory hygiene for anyone using the library, as an immutable object can not be effectively zeroed in place, creating a vulnerability, since this memory will persist at least until Python garbage collects, and at most until the program is closed.

Theoretical Solution

I am aware the Cryptography HKDF implementation is in Rust rather than C, and while I am not well versed in Rust, I will provide a proof of concept solution that I have created wrapping OpenSSL. I am not a professional at any of this, so though my implementation may not be perfect, it should work as a proof of concept. Below this wrapper I will also leave a zeroing function, as it is used in my glue code in Python.

#include <openssl/kdf.h>
#include <openssl/evp.h>

int hkdf(
    const uint8_t* ikm,  uint32_t ikmlen,
    const uint8_t* salt, uint32_t saltlen,
    uint8_t* out,        uint32_t outlen,
    uint8_t* info,       uint32_t infolen
) {
    EVP_PKEY_CTX *pctx = EVP_PKEY_CTX_new_id(EVP_PKEY_HKDF, NULL);
    if (!pctx) return -1;

    if (EVP_PKEY_derive_init(pctx) <= 0) goto err;
    if (EVP_PKEY_CTX_set_hkdf_md(pctx, EVP_sha256()) <= 0) goto err;
    if (EVP_PKEY_CTX_set1_hkdf_salt(pctx, salt, saltlen) <= 0) goto err;
    if (EVP_PKEY_CTX_set1_hkdf_key(pctx, ikm, ikmlen) <= 0) goto err;
    if (EVP_PKEY_CTX_add1_hkdf_info(pctx, info, infolen) <= 0) goto err;
    if (EVP_PKEY_derive(pctx, out, (size_t*)&outlen) <= 0) goto err;

    EVP_PKEY_CTX_free(pctx);
    return 0;

err:
    EVP_PKEY_CTX_free(pctx);
    return -1;
}

void secure_zero(void *ptr, size_t len) {
    volatile uint8_t *p = (volatile uint8_t *)ptr;
    while (len--) {
        *p++ = 0;
    }
}

This is essentially a wrapper around OpenSSL's HKDF implementation, which when using ctypes after compiling:

import ctypes
from ctypes import c_uint8, c_uint32, c_int, POINTER, c_void_p, c_size_t

_lib = ctypes.CDLL("./argon2_wrapper.dll")

class HKDFException(Exception):
    pass

_lib.hkdf.argtypes = [
    POINTER(c_uint8), c_uint32,  # ikm, ikmlen
    POINTER(c_uint8), c_uint32,  # salt, saltlen
    POINTER(c_uint8), c_uint32,  # out, outlen
    POINTER(c_uint8), c_uint32,  # info, infolen
]
_lib.hkdf.restype = c_int

def hkdf(ikm: bytearray, salt, dk_len, *, info=b'', preserve_ikm=False):
    if not isinstance(ikm, bytearray):
        raise TypeError("IKM must be a `bytearray`.\nThis allows for zeroing after use.")

    outbuf = (c_uint8 * dk_len)()
    c_ikm = (c_uint8 * len(ikm)).from_buffer_copy(ikm)
    if not preserve_ikm:
        secure_zero(ikm)
    c_salt = (c_uint8 * len(salt)).from_buffer_copy(salt)
    c_info = (c_uint8 * len(info)).from_buffer_copy(info)

    ret = _lib.hkdf(
        c_ikm, len(ikm),
        c_salt, len(salt),
        outbuf, dk_len,
        c_info, len(info),
    )

    if ret != 0:
        raise HKDFException(f"hkdf failed with code {ret}")

    okm = bytearray(outbuf)

    ptr = ctypes.cast(outbuf, ctypes.c_void_p)
    _lib.secure_zero(ptr, len(outbuf))

    return okm

I have put in strict checks for bytearray types in these examples, which I understand would not be compatible with cryptography as it stands due to backwards compatibility issues.

Or if you would rather a more C-style buffer example:

...

def hkdf(ikm: bytearray, okm_buf: bytearray, salt, dk_len, *, info=b'', preserve_ikm=False):
    if not isinstance(ikm, bytearray):
        raise TypeError("IKM must be a `bytearray`.\nThis allows for zeroing after use.")

    outbuf = (c_uint8 * dk_len)()

    ...

    okm = bytearray(outbuf)

    ptr = ctypes.cast(outbuf, ctypes.c_void_p)
    _lib.secure_zero(ptr, len(outbuf))
    
    okm_buf[:] = outbuf

    return 0

...

Some of the above code I wrote in the GitHub issue report window so it may not be syntactically correct, my apologies if it isn't.

This was my solution, and I hope that if you see fit to pursue a fix for this issue that the above snippets will provide some help.

Closing Thoughts

While some may say that anyone writing security critical code in Python, a language which does not support manual memory management, probably does not care about memory hygiene, I feel that this is a very simple issue to fix. If I had to guess, I would say a lot of people doing security critical code in Python may not even know that secrets persisting in memory is an issue, which is all of the more reason to improve upon the memory hygiene of Cryptography.

Thank you for your time, and for maintaining such a valuable library.

[alex]

On ciphers we support update_into, I think we could similarly support a derive_into operation on KDFs.

[HaZeShade]

Oh? I haven't seen the update_into method, I was gonna attach on here that I am aware that Cryptography can't switch over entirely to a new format since I would guess you guys want to keep backwards compatibility, it would still be a good idea to support something like derive_into for KDFs though, yes. I also did a little bit of looking into hazmat.primitives.ciphers, and there is no .encrypt_into/.decrypt_into methods for AEAD, which I feel like that would be a useful addition, and with my limited knowledge of Rust it doesn't seem like that would be too difficult to add. Though yes, I did look at .update_into for ciphers, and if/until something comes out for AEAD like .encrypt_into/.decrypt_into, that will probably be what I use. Thank you for the response!

[HaZeShade]

I also just looked into cryptography.hazmat.primitives.asymmetric, and neither X22519PrivateKey or EllipticCurvePrivateKey have *_into variants. I haven't done as much looking into source for these, but if it's possible I feel like *_into variants for anything that handles or generates secrets couldn't be a bad idea. And for these specifically since it's a more abstracted class it would be adding something like a zero method to the classes, but also making things like serialization have an option to fill a buffer instead of returning an immutable when serializing keys to bytes. Same thing for exchange in X25519PrivateKey and EllipticCurvePrivateKey, both having no *_into variant that I have seen.