05 Mar, 2020

1 commit

  • Some older version of GAS do not support the ADX instructions, similarly
    to how they also don't support AVX and such. This commit adds the same
    build-time detection mechanisms we use for AVX and others for ADX, and
    then makes sure that the curve25519 library dispatcher calls the right
    functions.

    Reported-by: Willy Tarreau
    Signed-off-by: Jason A. Donenfeld
    Signed-off-by: Herbert Xu

    Jason A. Donenfeld
     

16 Jan, 2020

1 commit

  • These two C implementations from Zinc -- a 32x32 one and a 64x64 one,
    depending on the platform -- come from Andrew Moon's public domain
    poly1305-donna portable code, modified for usage in the kernel. The
    precomputation in the 32-bit version and the use of 64x64 multiplies in
    the 64-bit version make these perform better than the code it replaces.
    Moon's code is also very widespread and has received many eyeballs of
    scrutiny.

    There's a bit of interference between the x86 implementation, which
    relies on internal details of the old scalar implementation. In the next
    commit, the x86 implementation will be replaced with a faster one that
    doesn't rely on this, so none of this matters much. But for now, to keep
    this passing the tests, we inline the bits of the old implementation
    that the x86 implementation relied on. Also, since we now support a
    slightly larger key space, via the union, some offsets had to be fixed
    up.

    Nonce calculation was folded in with the emit function, to take
    advantage of 64x64 arithmetic. However, Adiantum appeared to rely on no
    nonce handling in emit, so this path was conditionalized. We also
    introduced a new struct, poly1305_core_key, to represent the precise
    amount of space that particular implementation uses.

    Testing with kbench9000, depending on the CPU, the update function for
    the 32x32 version has been improved by 4%-7%, and for the 64x64 by
    19%-30%. The 32x32 gains are small, but I think there's great value in
    having a parallel implementation to the 64x64 one so that the two can be
    compared side-by-side as nice stand-alone units.

    Signed-off-by: Jason A. Donenfeld
    Signed-off-by: Herbert Xu

    Jason A. Donenfeld
     

09 Jan, 2020

21 commits

  • Now that all templates provide a ->create() method which creates an
    instance, installs a strongly-typed ->free() method directly to it, and
    registers it, the older ->alloc() and ->free() methods in
    'struct crypto_template' are no longer used. Remove them.

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

    Eric Biggers
     
  • 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
     
  • Convert the "seqiv" template to the new way of freeing instances where a
    ->free() method is installed to the instance struct itself. Also remove
    the unused implementation of the old way of freeing instances from the
    "echainiv" template, since it's already using the new way too.

    In doing this, also simplify the code by making the helper function
    aead_geniv_alloc() install the ->free() method, instead of making seqiv
    and echainiv do this themselves. This is analogous to how
    skcipher_alloc_instance_simple() works.

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

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

    Eric Biggers
     
  • Add support to shash and ahash for the new way of freeing instances
    (already used for skcipher, aead, and akcipher) where a ->free() method
    is installed to the instance struct itself. These methods are more
    strongly-typed than crypto_template::free(), which they replace.

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

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

    Eric Biggers
     
  • Now that crypto_init_spawn() is only called by crypto_grab_spawn(),
    simplify things by moving its functionality into crypto_grab_spawn().

    In the process of doing this, also be more consistent about when the
    spawn and instance are updated, and remove the crypto_spawn::dropref
    flag since now it's always set.

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

    Eric Biggers
     
  • Now that all the templates that need ahash spawns have been converted to
    use crypto_grab_ahash() rather than look up the algorithm directly,
    crypto_ahash_type is no longer used outside of ahash.c. Make it static.

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

    Eric Biggers
     
  • Remove lots of helper functions that were previously used for
    instantiating crypto templates, but are now unused:

    - crypto_get_attr_alg() and similar functions looked up an inner
    algorithm directly from a template parameter. These were replaced
    with getting the algorithm's name, then calling crypto_grab_*().

    - crypto_init_spawn2() and similar functions initialized a spawn, given
    an algorithm. Similarly, these were replaced with crypto_grab_*().

    - crypto_alloc_instance() and similar functions allocated an instance
    with a single spawn, given the inner algorithm. These aren't useful
    anymore since crypto_grab_*() need the instance allocated first.

    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 skcipher_alloc_instance_simple() 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.

    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
     
  • Currently, "cipher" (single-block cipher) spawns are usually initialized
    by using crypto_get_attr_alg() to look up the algorithm, then calling
    crypto_init_spawn(). In one case, crypto_grab_spawn() is used directly.

    The former way is different from how skcipher, aead, and akcipher spawns
    are initialized (they use crypto_grab_*()), and for no good reason.
    This difference introduces unnecessary complexity.

    The crypto_grab_*() functions used to have some problems, like not
    holding a reference to the algorithm and requiring the caller to
    initialize spawn->base.inst. But those problems are fixed now.

    Also, the cipher spawns are not strongly typed; e.g., the API requires
    that the user manually specify the flags CRYPTO_ALG_TYPE_CIPHER and
    CRYPTO_ALG_TYPE_MASK. Though the "cipher" algorithm type itself isn't
    yet strongly typed, we can start by making the spawns strongly typed.

    So, let's introduce a new 'struct crypto_cipher_spawn', and functions
    crypto_grab_cipher() and crypto_drop_cipher() to grab and drop them.

    Later patches will convert all cipher spawns to use these, then make
    crypto_spawn_cipher() take 'struct crypto_cipher_spawn' as well, instead
    of a bare 'struct crypto_spawn' as it currently does.

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

    Eric Biggers
     
  • Currently, ahash spawns are initialized by using ahash_attr_alg() or
    crypto_find_alg() to look up the ahash algorithm, then calling
    crypto_init_ahash_spawn().

    This is different from how skcipher, aead, and akcipher spawns are
    initialized (they use crypto_grab_*()), and for no good reason. This
    difference introduces unnecessary complexity.

    The crypto_grab_*() functions used to have some problems, like not
    holding a reference to the algorithm and requiring the caller to
    initialize spawn->base.inst. But those problems are fixed now.

    So, let's introduce crypto_grab_ahash() so that we can convert all
    templates to the same way of initializing their spawns.

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

    Eric Biggers
     
  • Currently, shash spawns are initialized by using shash_attr_alg() or
    crypto_alg_mod_lookup() to look up the shash algorithm, then calling
    crypto_init_shash_spawn().

    This is different from how skcipher, aead, and akcipher spawns are
    initialized (they use crypto_grab_*()), and for no good reason. This
    difference introduces unnecessary complexity.

    The crypto_grab_*() functions used to have some problems, like not
    holding a reference to the algorithm and requiring the caller to
    initialize spawn->base.inst. But those problems are fixed now.

    So, let's introduce crypto_grab_shash() so that we can convert all
    templates to the same way of initializing their spawns.

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

    Eric Biggers
     
  • Currently, crypto_spawn::inst is first used temporarily to pass the
    instance to crypto_grab_spawn(). Then crypto_init_spawn() overwrites it
    with crypto_spawn::next, which shares the same union. Finally,
    crypto_spawn::inst is set again when the instance is registered.

    Make this less convoluted by just passing the instance as an argument to
    crypto_grab_spawn() instead.

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

    Eric Biggers
     
  • Initializing a crypto_akcipher_spawn currently requires:

    1. Set spawn->base.inst to point to the instance.
    2. Call crypto_grab_akcipher().

    But there's no reason for these steps to be separate, and in fact this
    unneeded complication has caused at least one bug, the one fixed by
    commit 6db43410179b ("crypto: adiantum - initialize crypto_spawn::inst")

    So just make crypto_grab_akcipher() take the instance as an argument.

    To keep the function call from getting too unwieldy due to this extra
    argument, also introduce a 'mask' variable into pkcs1pad_create().

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

    Eric Biggers
     
  • Initializing a crypto_aead_spawn currently requires:

    1. Set spawn->base.inst to point to the instance.
    2. Call crypto_grab_aead().

    But there's no reason for these steps to be separate, and in fact this
    unneeded complication has caused at least one bug, the one fixed by
    commit 6db43410179b ("crypto: adiantum - initialize crypto_spawn::inst")

    So just make crypto_grab_aead() take the instance as an argument.

    To keep the function calls from getting too unwieldy due to this extra
    argument, also introduce a 'mask' variable into the affected places
    which weren't already using one.

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

    Eric Biggers
     
  • Initializing a crypto_skcipher_spawn currently requires:

    1. Set spawn->base.inst to point to the instance.
    2. Call crypto_grab_skcipher().

    But there's no reason for these steps to be separate, and in fact this
    unneeded complication has caused at least one bug, the one fixed by
    commit 6db43410179b ("crypto: adiantum - initialize crypto_spawn::inst")

    So just make crypto_grab_skcipher() take the instance as an argument.

    To keep the function calls from getting too unwieldy due to this extra
    argument, also introduce a 'mask' variable into the affected places
    which weren't already using one.

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

    Eric Biggers
     
  • Define struct ahash_instance in a way analogous to struct
    skcipher_instance, struct aead_instance, and struct akcipher_instance,
    where the struct is defined to include both the algorithm structure at
    the beginning and the additional crypto_instance fields at the end.

    This is needed to allow allocating ahash instances directly using
    kzalloc(sizeof(*inst) + sizeof(*ictx), ...) in the same way as skcipher,
    aead, and akcipher instances. In turn, that's needed to make spawns be
    initialized in a consistent way everywhere.

    Also take advantage of the addition of the base instance to struct
    ahash_instance by simplifying the ahash_crypto_instance() and
    ahash_instance() functions.

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

    Eric Biggers
     
  • Define struct shash_instance in a way analogous to struct
    skcipher_instance, struct aead_instance, and struct akcipher_instance,
    where the struct is defined to include both the algorithm structure at
    the beginning and the additional crypto_instance fields at the end.

    This is needed to allow allocating shash instances directly using
    kzalloc(sizeof(*inst) + sizeof(*ictx), ...) in the same way as skcipher,
    aead, and akcipher instances. In turn, that's needed to make spawns be
    initialized in a consistent way everywhere.

    Also take advantage of the addition of the base instance to struct
    shash_instance by simplifying the shash_crypto_instance() and
    shash_instance() functions.

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

    Eric Biggers
     
  • The CRYPTO_TFM_RES_WEAK_KEY 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.
    There are also no tests that verify that all algorithms actually set (or
    don't set) it correctly.

    This is also the last remaining CRYPTO_TFM_RES_* flag, which means that
    it's the only thing still needing all the boilerplate code which
    propagates these flags around from child => parent tfms.

    And if someone ever needs to distinguish this error in the future (which
    is somewhat unlikely, as it's been unneeded for a long time), it would
    be much better to just define a new return value like -EKEYREJECTED.
    That would be much simpler, less error-prone, and easier to test.

    So just remove this flag.

    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
     
  • skcipher_walk_aead() is unused and is identical to
    skcipher_walk_aead_encrypt(), so remove it.

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

    Eric Biggers
     

27 Dec, 2019

2 commits

  • This patch introduces the skcipher_ialg_simple helper which fetches
    the crypto_alg structure from a simple skcipher instance's spawn.

    This allows us to remove the third argument from the function
    skcipher_alloc_instance_simple.

    In doing so the reference count to the algorithm is now maintained
    by the Crypto API and the caller no longer needs to drop the alg
    refcount.

    Signed-off-by: Herbert Xu

    Herbert Xu
     
  • This patch changes crypto_grab_spawn to retain the reference count
    on the algorithm. This is because the caller needs to access the
    algorithm parameters and without the reference count the algorithm
    can be freed at any time.

    The reference count will be subsequently dropped by the crypto API
    once the instance has been registered. The helper crypto_drop_spawn
    will also conditionally drop the reference count depending on whether
    it has been registered.

    Note that the code is actually added to crypto_init_spawn. However,
    unless the caller activates this by setting spawn->dropref beforehand
    then nothing happens. The only caller that sets dropref is currently
    crypto_grab_spawn.

    Once all legacy users of crypto_init_spawn disappear, then we can
    kill the dropref flag.

    Internally each instance will maintain a list of its spawns prior
    to registration. This memory used by this list is shared with
    other fields that are only used after registration. In order for
    this to work a new flag spawn->registered is added to indicate
    whether spawn->inst can be used.

    Fixes: d6ef2f198d4c ("crypto: api - Add crypto_grab_spawn primitive")
    Signed-off-by: Herbert Xu

    Herbert Xu
     

20 Dec, 2019

1 commit

  • Some of the algorithm unregistration functions return -ENOENT when asked
    to unregister a non-registered algorithm, while others always return 0
    or always return void. But no users check the return value, except for
    two of the bulk unregistration functions which print a message on error
    but still always return 0 to their caller, and crypto_del_alg() which
    calls crypto_unregister_instance() which always returns 0.

    Since unregistering a non-registered algorithm is always a kernel bug
    but there isn't anything callers should do to handle this situation at
    runtime, let's simplify things by making all the unregistration
    functions return void, and moving the error message into
    crypto_unregister_alg() and upgrading it to a WARN().

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

    Eric Biggers
     

11 Dec, 2019

13 commits

  • This patch switches hmac over to the new init_tfm/exit_tfm interface
    as opposed to cra_init/cra_exit. This way the shash API can make
    sure that descsize does not exceed the maximum.

    This patch also adds the API helper shash_alg_instance.

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

    Herbert Xu
     
  • The shash interface supports a dynamic descsize field because of
    the presence of fallbacks (it's just padlock-sha actually, perhaps
    we can remove it one day). As it is the API does not verify the
    setting of descsize at all. It is up to the individual algorithms
    to ensure that descsize does not exceed the specified maximum value
    of HASH_MAX_DESCSIZE (going above would cause stack corruption).

    In order to allow the API to impose this limit directly, this patch
    adds init_tfm/exit_tfm hooks to the shash_alg structure. We can
    then verify the descsize setting in the API directly.

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

    Herbert Xu
     
  • Currently when a spawn is removed we will zap its alg field.
    This is racy because the spawn could belong to an unregistered
    instance which may dereference the spawn->alg field.

    This patch fixes this by keeping spawn->alg constant and instead
    adding a new spawn->dead field to indicate that a spawn is going
    away.

    Signed-off-by: Herbert Xu

    Herbert Xu
     
  • Building with W=1 causes a warning:

    CC [M] arch/x86/crypto/chacha_glue.o
    In file included from arch/x86/crypto/chacha_glue.c:10:
    ./include/crypto/internal/chacha.h:37:1: warning: 'inline' is not at beginning of declaration [-Wold-style-declaration]
    37 | static int inline chacha12_setkey(struct crypto_skcipher *tfm, const u8 *key,
    | ^~~~~~

    Straighten out the order to match the rest of the header file.

    Signed-off-by: Valdis Kletnieks
    Signed-off-by: Herbert Xu

    Valdis Klētnieks
     
  • Add a helper function crypto_skcipher_min_keysize() to mirror
    crypto_skcipher_max_keysize().

    This will be used by the self-tests.

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

    Eric Biggers
     
  • Move crypto_aead_maxauthsize() to so that it's available
    to users of the API, not just AEAD implementations.

    This will be used by the self-tests.

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

    Eric Biggers
     
  • The essiv and hmac templates refuse to use any hash algorithm that has a
    ->setkey() function, which includes not just algorithms that always need
    a key, but also algorithms that optionally take a key.

    Previously the only optionally-keyed hash algorithms in the crypto API
    were non-cryptographic algorithms like crc32, so this didn't really
    matter. But that's changed with BLAKE2 support being added. BLAKE2
    should work with essiv and hmac, just like any other cryptographic hash.

    Fix this by allowing the use of both algorithms without a ->setkey()
    function and algorithms that have the OPTIONAL_KEY flag set.

    Signed-off-by: Eric Biggers
    Acked-by: Ard Biesheuvel
    Signed-off-by: Herbert Xu

    Eric Biggers
     
  • Due to the removal of the blkcipher and ablkcipher algorithm types,
    crypto_skcipher::decrypt is now redundant since it always equals
    crypto_skcipher_alg(tfm)->decrypt.

    Remove it and update crypto_skcipher_decrypt() accordingly.

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

    Eric Biggers
     
  • Due to the removal of the blkcipher and ablkcipher algorithm types,
    crypto_skcipher::encrypt is now redundant since it always equals
    crypto_skcipher_alg(tfm)->encrypt.

    Remove it and update crypto_skcipher_encrypt() accordingly.

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

    Eric Biggers
     
  • Due to the removal of the blkcipher and ablkcipher algorithm types,
    crypto_skcipher::setkey now always points to skcipher_setkey().

    Simplify by removing this function pointer and instead just making
    skcipher_setkey() be crypto_skcipher_setkey() directly.

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

    Eric Biggers
     
  • Due to the removal of the blkcipher and ablkcipher algorithm types,
    crypto_skcipher::keysize is now redundant since it always equals
    crypto_skcipher_alg(tfm)->max_keysize.

    Remove it and update crypto_skcipher_default_keysize() accordingly.

    Also rename crypto_skcipher_default_keysize() to
    crypto_skcipher_max_keysize() to clarify that it specifically returns
    the maximum key size, not some unspecified "default".

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

    Eric Biggers
     
  • Due to the removal of the blkcipher and ablkcipher algorithm types,
    crypto_skcipher::ivsize is now redundant since it always equals
    crypto_skcipher_alg(tfm)->ivsize.

    Remove it and update crypto_skcipher_ivsize() accordingly.

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

    Eric Biggers
     
  • The crypto glue performed function prototype casting via macros to make
    indirect calls to assembly routines. Instead of performing casts at the
    call sites (which trips Control Flow Integrity prototype checking), switch
    each prototype to a common standard set of arguments which allows the
    removal of the existing macros. In order to keep pointer math unchanged,
    internal casting between u128 pointers and u8 pointers is added.

    Co-developed-by: João Moreira
    Signed-off-by: João Moreira
    Signed-off-by: Kees Cook
    Reviewed-by: Eric Biggers
    Signed-off-by: Herbert Xu

    Kees Cook
     

17 Nov, 2019

1 commit