16 Jul, 2020

1 commit

  • The flag CRYPTO_ALG_ASYNC is "inherited" in the sense that when a
    template is instantiated, the template will have CRYPTO_ALG_ASYNC set if
    any of the algorithms it uses has CRYPTO_ALG_ASYNC set.

    We'd like to add a second flag (CRYPTO_ALG_ALLOCATES_MEMORY) that gets
    "inherited" in the same way. This is difficult because the handling of
    CRYPTO_ALG_ASYNC is hardcoded everywhere. Address this by:

    - Add CRYPTO_ALG_INHERITED_FLAGS, which contains the set of flags that
    have these inheritance semantics.

    - Add crypto_algt_inherited_mask(), for use by template ->create()
    methods. It returns any of these flags that the user asked to be
    unset and thus must be passed in the 'mask' to crypto_grab_*().

    - Also modify crypto_check_attr_type() to handle computing the 'mask'
    so that most templates can just use this.

    - Make crypto_grab_*() propagate these flags to the template instance
    being created so that templates don't have to do this themselves.

    Make crypto/simd.c propagate these flags too, since it "wraps" another
    algorithm, similar to a template.

    Based on a patch by Mikulas Patocka
    (https://lore.kernel.org/r/alpine.LRH.2.02.2006301414580.30526@file01.intranet.prod.int.rdu2.redhat.com).

    Signed-off-by: Eric Biggers
    Signed-off-by: Herbert Xu

    Eric Biggers
     

09 Jan, 2020

4 commits

  • Convert shash_free_instance() and its users to the new way of freeing
    instances, where a ->free() method is installed to the instance struct
    itself. This replaces the weakly-typed method crypto_template::free().

    This will allow removing support for the old way of freeing instances.

    Also give shash_free_instance() a more descriptive name to reflect that
    it's only for instances with a single spawn, not for any instance.

    Signed-off-by: Eric Biggers
    Signed-off-by: Herbert Xu

    Eric Biggers
     
  • Now that all users of single-block cipher spawns have been converted to
    use 'struct crypto_cipher_spawn' rather than the less specifically typed
    'struct crypto_spawn', make crypto_spawn_cipher() take a pointer to a
    'struct crypto_cipher_spawn' rather than a 'struct crypto_spawn'.

    Signed-off-by: Eric Biggers
    Signed-off-by: Herbert Xu

    Eric Biggers
     
  • Make the vmac64 template use the new function crypto_grab_cipher() to
    initialize its cipher spawn.

    This is needed to make all spawns be initialized in a consistent way.

    This required making vmac_create() allocate the instance directly rather
    than use shash_alloc_instance().

    Also simplify the error handling by taking advantage of crypto_drop_*()
    now accepting (as a no-op) spawns that haven't been initialized yet, and
    by taking advantage of crypto_grab_*() now handling ERR_PTR() names.

    Signed-off-by: Eric Biggers
    Signed-off-by: Herbert Xu

    Eric Biggers
     
  • The CRYPTO_TFM_RES_BAD_KEY_LEN flag was apparently meant as a way to
    make the ->setkey() functions provide more information about errors.

    However, no one actually checks for this flag, which makes it pointless.

    Also, many algorithms fail to set this flag when given a bad length key.
    Reviewing just the generic implementations, this is the case for
    aes-fixed-time, cbcmac, echainiv, nhpoly1305, pcrypt, rfc3686, rfc4309,
    rfc7539, rfc7539esp, salsa20, seqiv, and xcbc. But there are probably
    many more in arch/*/crypto/ and drivers/crypto/.

    Some algorithms can even set this flag when the key is the correct
    length. For example, authenc and authencesn set it when the key payload
    is malformed in any way (not just a bad length), the atmel-sha and ccree
    drivers can set it if a memory allocation fails, and the chelsio driver
    sets it for bad auth tag lengths, not just bad key lengths.

    So even if someone actually wanted to start checking this flag (which
    seems unlikely, since it's been unused for a long time), there would be
    a lot of work needed to get it working correctly. But it would probably
    be much better to go back to the drawing board and just define different
    return values, like -EINVAL if the key is invalid for the algorithm vs.
    -EKEYREJECTED if the key was rejected by a policy like "no weak keys".
    That would be much simpler, less error-prone, and easier to test.

    So just remove this flag.

    Signed-off-by: Eric Biggers
    Reviewed-by: Horia Geantă
    Signed-off-by: Herbert Xu

    Eric Biggers
     

18 Apr, 2019

1 commit

  • Use subsys_initcall for registration of all templates and generic
    algorithm implementations, rather than module_init. Then change
    cryptomgr to use arch_initcall, to place it before the subsys_initcalls.

    This is needed so that when both a generic and optimized implementation
    of an algorithm are built into the kernel (not loadable modules), the
    generic implementation is registered before the optimized one.
    Otherwise, the self-tests for the optimized implementation are unable to
    allocate the generic implementation for the new comparison fuzz tests.

    Note that on arm, a side effect of this change is that self-tests for
    generic implementations may run before the unaligned access handler has
    been installed. So, unaligned accesses will crash the kernel. This is
    arguably a good thing as it makes it easier to detect that type of bug.

    Signed-off-by: Eric Biggers
    Signed-off-by: Herbert Xu

    Eric Biggers
     

01 Jul, 2018

4 commits

  • Remove the original version of the VMAC template that had the nonce
    hardcoded to 0 and produced a digest with the wrong endianness. I'm
    unsure whether this had users or not (there are no explicit in-kernel
    references to it), but given that the hardcoded nonce made it wildly
    insecure unless a unique key was used for each message, let's try
    removing it and see if anyone complains.

    Leave the new "vmac64" template that requires the nonce to be explicitly
    specified as the first 16 bytes of data and uses the correct endianness
    for the digest.

    Signed-off-by: Eric Biggers
    Signed-off-by: Herbert Xu

    Eric Biggers
     
  • Currently the VMAC template uses a "nonce" hardcoded to 0, which makes
    it insecure unless a unique key is set for every message. Also, the
    endianness of the final digest is wrong: the implementation uses little
    endian, but the VMAC specification has it as big endian, as do other
    VMAC implementations such as the one in Crypto++.

    Add a new VMAC template where the nonce is passed as the first 16 bytes
    of data (similar to what is done for Poly1305's nonce), and the digest
    is big endian. Call it "vmac64", since the old name of simply "vmac"
    didn't clarify whether the implementation is of VMAC-64 or of VMAC-128
    (which produce 64-bit and 128-bit digests respectively); so we fix the
    naming ambiguity too.

    Signed-off-by: Eric Biggers
    Signed-off-by: Herbert Xu

    Eric Biggers
     
  • syzbot reported a crash in vmac_final() when multiple threads
    concurrently use the same "vmac(aes)" transform through AF_ALG. The bug
    is pretty fundamental: the VMAC template doesn't separate per-request
    state from per-tfm (per-key) state like the other hash algorithms do,
    but rather stores it all in the tfm context. That's wrong.

    Also, vmac_final() incorrectly zeroes most of the state including the
    derived keys and cached pseudorandom pad. Therefore, only the first
    VMAC invocation with a given key calculates the correct digest.

    Fix these bugs by splitting the per-tfm state from the per-request state
    and using the proper init/update/final sequencing for requests.

    Reproducer for the crash:

    #include
    #include
    #include

    int main()
    {
    int fd;
    struct sockaddr_alg addr = {
    .salg_type = "hash",
    .salg_name = "vmac(aes)",
    };
    char buf[256] = { 0 };

    fd = socket(AF_ALG, SOCK_SEQPACKET, 0);
    bind(fd, (void *)&addr, sizeof(addr));
    setsockopt(fd, SOL_ALG, ALG_SET_KEY, buf, 16);
    fork();
    fd = accept(fd, NULL, NULL);
    for (;;)
    write(fd, buf, 256);
    }

    The immediate cause of the crash is that vmac_ctx_t.partial_size exceeds
    VMAC_NHBYTES, causing vmac_final() to memset() a negative length.

    Reported-by: syzbot+264bca3a6e8d645550d3@syzkaller.appspotmail.com
    Fixes: f1939f7c5645 ("crypto: vmac - New hash algorithm for intel_txt support")
    Cc: # v2.6.32+
    Signed-off-by: Eric Biggers
    Signed-off-by: Herbert Xu

    Eric Biggers
     
  • The VMAC template assumes the block cipher has a 128-bit block size, but
    it failed to check for that. Thus it was possible to instantiate it
    using a 64-bit block size cipher, e.g. "vmac(cast5)", causing
    uninitialized memory to be used.

    Add the needed check when instantiating the template.

    Fixes: f1939f7c5645 ("crypto: vmac - New hash algorithm for intel_txt support")
    Cc: # v2.6.32+
    Signed-off-by: Eric Biggers
    Signed-off-by: Herbert Xu

    Eric Biggers
     

26 Nov, 2014

1 commit

  • This adds the module loading prefix "crypto-" to the template lookup
    as well.

    For example, attempting to load 'vfat(blowfish)' via AF_ALG now correctly
    includes the "crypto-" prefix at every level, correctly rejecting "vfat":

    net-pf-38
    algif-hash
    crypto-vfat(blowfish)
    crypto-vfat(blowfish)-all
    crypto-vfat

    Reported-by: Mathias Krause
    Signed-off-by: Kees Cook
    Acked-by: Mathias Krause
    Signed-off-by: Herbert Xu

    Kees Cook
     

17 Oct, 2014

1 commit

  • Recently, in commit 13aa93c70e71 ("random: add and use memzero_explicit()
    for clearing data"), we have found that GCC may optimize some memset()
    cases away when it detects a stack variable is not being used anymore
    and going out of scope. This can happen, for example, in cases when we
    are clearing out sensitive information such as keying material or any
    e.g. intermediate results from crypto computations, etc.

    With the help of Coccinelle, we can figure out and fix such occurences
    in the crypto subsytem as well. Julia Lawall provided the following
    Coccinelle program:

    @@
    type T;
    identifier x;
    @@

    T x;
    ... when exists
    when any
    -memset
    +memzero_explicit
    (&x,
    -0,
    ...)
    ... when != x
    when strict

    @@
    type T;
    identifier x;
    @@

    T x[...];
    ... when exists
    when any
    -memset
    +memzero_explicit
    (x,
    -0,
    ...)
    ... when != x
    when strict

    Therefore, make use of the drop-in replacement memzero_explicit() for
    exactly such cases instead of using memset().

    Signed-off-by: Daniel Borkmann
    Cc: Julia Lawall
    Cc: Herbert Xu
    Cc: Theodore Ts'o
    Cc: Hannes Frederic Sowa
    Acked-by: Hannes Frederic Sowa
    Acked-by: Herbert Xu
    Signed-off-by: Theodore Ts'o

    Daniel Borkmann
     

15 Oct, 2012

1 commit

  • VMAC implementation, as it is, does not work with blocks that
    are not multiples of 128-bytes. Furthermore, this is a problem
    when using the implementation on scatterlists, even
    when the complete plain text is 128-byte multiple, as the pieces
    that get passed to vmac_update can be pretty much any size.

    I also added test cases for unaligned blocks.

    Signed-off-by: Salman Qazi
    Signed-off-by: Herbert Xu

    Salman Qazi
     

07 Sep, 2012

1 commit


01 Nov, 2011

1 commit


31 Mar, 2011

1 commit


18 Mar, 2010

1 commit


02 Sep, 2009

1 commit