20 Mar, 2018

2 commits


28 Feb, 2018

3 commits

  • commit 4b34968e77ad09628cfb3c4a7daf2adc2cefc6e8 upstream.

    The asymmetric key type allows an X.509 certificate to be added even if
    its signature's hash algorithm is not available in the crypto API. In
    that case 'payload.data[asym_auth]' will be NULL. But the key
    restriction code failed to check for this case before trying to use the
    signature, resulting in a NULL pointer dereference in
    key_or_keyring_common() or in restrict_link_by_signature().

    Fix this by returning -ENOPKG when the signature is unsupported.

    Reproducer when all the CONFIG_CRYPTO_SHA512* options are disabled and
    keyctl has support for the 'restrict_keyring' command:

    keyctl new_session
    keyctl restrict_keyring @s asymmetric builtin_trusted
    openssl req -new -sha512 -x509 -batch -nodes -outform der \
    | keyctl padd asymmetric desc @s

    Fixes: a511e1af8b12 ("KEYS: Move the point of trust determination to __key_link()")
    Cc: # v4.7+
    Signed-off-by: Eric Biggers
    Signed-off-by: David Howells
    Signed-off-by: Greg Kroah-Hartman

    Eric Biggers
     
  • commit 971b42c038dc83e3327872d294fe7131bab152fc upstream.

    When pkcs7_verify_sig_chain() is building the certificate chain for a
    SignerInfo using the certificates in the PKCS#7 message, it is passing
    the wrong arguments to public_key_verify_signature(). Consequently,
    when the next certificate is supposed to be used to verify the previous
    certificate, the next certificate is actually used to verify itself.

    An attacker can use this bug to create a bogus certificate chain that
    has no cryptographic relationship between the beginning and end.

    Fortunately I couldn't quite find a way to use this to bypass the
    overall signature verification, though it comes very close. Here's the
    reasoning: due to the bug, every certificate in the chain beyond the
    first actually has to be self-signed (where "self-signed" here refers to
    the actual key and signature; an attacker might still manipulate the
    certificate fields such that the self_signed flag doesn't actually get
    set, and thus the chain doesn't end immediately). But to pass trust
    validation (pkcs7_validate_trust()), either the SignerInfo or one of the
    certificates has to actually be signed by a trusted key. Since only
    self-signed certificates can be added to the chain, the only way for an
    attacker to introduce a trusted signature is to include a self-signed
    trusted certificate.

    But, when pkcs7_validate_trust_one() reaches that certificate, instead
    of trying to verify the signature on that certificate, it will actually
    look up the corresponding trusted key, which will succeed, and then try
    to verify the *previous* certificate, which will fail. Thus, disaster
    is narrowly averted (as far as I could tell).

    Fixes: 6c2dc5ae4ab7 ("X.509: Extract signature digest and make self-signed cert checks earlier")
    Cc: # v4.7+
    Signed-off-by: Eric Biggers
    Signed-off-by: David Howells
    Signed-off-by: Greg Kroah-Hartman

    Eric Biggers
     
  • commit 437499eea4291ae9621e8763a41df027c110a1ef upstream.

    The X.509 parser mishandles the case where the certificate's signature's
    hash algorithm is not available in the crypto API. In this case,
    x509_get_sig_params() doesn't allocate the cert->sig->digest buffer;
    this part seems to be intentional. However,
    public_key_verify_signature() is still called via
    x509_check_for_self_signed(), which triggers the 'BUG_ON(!sig->digest)'.

    Fix this by making public_key_verify_signature() return -ENOPKG if the
    hash buffer has not been allocated.

    Reproducer when all the CONFIG_CRYPTO_SHA512* options are disabled:

    openssl req -new -sha512 -x509 -batch -nodes -outform der \
    | keyctl padd asymmetric desc @s

    Fixes: 6c2dc5ae4ab7 ("X.509: Extract signature digest and make self-signed cert checks earlier")
    Reported-by: Paolo Valente
    Cc: Paolo Valente
    Cc: # v4.7+
    Signed-off-by: Eric Biggers
    Signed-off-by: David Howells
    Signed-off-by: Greg Kroah-Hartman

    Eric Biggers
     

25 Feb, 2018

2 commits

  • commit 9fa68f620041be04720d0cbfb1bd3ddfc6310b24 upstream.

    Currently, almost none of the keyed hash algorithms check whether a key
    has been set before proceeding. Some algorithms are okay with this and
    will effectively just use a key of all 0's or some other bogus default.
    However, others will severely break, as demonstrated using
    "hmac(sha3-512-generic)", the unkeyed use of which causes a kernel crash
    via a (potentially exploitable) stack buffer overflow.

    A while ago, this problem was solved for AF_ALG by pairing each hash
    transform with a 'has_key' bool. However, there are still other places
    in the kernel where userspace can specify an arbitrary hash algorithm by
    name, and the kernel uses it as unkeyed hash without checking whether it
    is really unkeyed. Examples of this include:

    - KEYCTL_DH_COMPUTE, via the KDF extension
    - dm-verity
    - dm-crypt, via the ESSIV support
    - dm-integrity, via the "internal hash" mode with no key given
    - drbd (Distributed Replicated Block Device)

    This bug is especially bad for KEYCTL_DH_COMPUTE as that requires no
    privileges to call.

    Fix the bug for all users by adding a flag CRYPTO_TFM_NEED_KEY to the
    ->crt_flags of each hash transform that indicates whether the transform
    still needs to be keyed or not. Then, make the hash init, import, and
    digest functions return -ENOKEY if the key is still needed.

    The new flag also replaces the 'has_key' bool which algif_hash was
    previously using, thereby simplifying the algif_hash implementation.

    Reported-by: syzbot
    Cc: stable@vger.kernel.org
    Signed-off-by: Eric Biggers
    Signed-off-by: Herbert Xu
    Signed-off-by: Greg Kroah-Hartman

    Eric Biggers
     
  • commit a208fa8f33031b9e0aba44c7d1b7e68eb0cbd29e upstream.

    We need to consistently enforce that keyed hashes cannot be used without
    setting the key. To do this we need a reliable way to determine whether
    a given hash algorithm is keyed or not. AF_ALG currently does this by
    checking for the presence of a ->setkey() method. However, this is
    actually slightly broken because the CRC-32 algorithms implement
    ->setkey() but can also be used without a key. (The CRC-32 "key" is not
    actually a cryptographic key but rather represents the initial state.
    If not overridden, then a default initial state is used.)

    Prepare to fix this by introducing a flag CRYPTO_ALG_OPTIONAL_KEY which
    indicates that the algorithm has a ->setkey() method, but it is not
    required to be called. Then set it on all the CRC-32 algorithms.

    The same also applies to the Adler-32 implementation in Lustre.

    Also, the cryptd and mcryptd templates have to pass through the flag
    from their underlying algorithm.

    Cc: stable@vger.kernel.org
    Signed-off-by: Eric Biggers
    Signed-off-by: Herbert Xu
    Signed-off-by: Greg Kroah-Hartman

    Eric Biggers
     

17 Feb, 2018

4 commits

  • commit a16e772e664b9a261424107784804cffc8894977 upstream.

    Since Poly1305 requires a nonce per invocation, the Linux kernel
    implementations of Poly1305 don't use the crypto API's keying mechanism
    and instead expect the key and nonce as the first 32 bytes of the data.
    But ->setkey() is still defined as a stub returning an error code. This
    prevents Poly1305 from being used through AF_ALG and will also break it
    completely once we start enforcing that all crypto API users (not just
    AF_ALG) call ->setkey() if present.

    Fix it by removing crypto_poly1305_setkey(), leaving ->setkey as NULL.

    Signed-off-by: Eric Biggers
    Signed-off-by: Herbert Xu
    Signed-off-by: Greg Kroah-Hartman

    Eric Biggers
     
  • commit fa59b92d299f2787e6bae1ff078ee0982e80211f upstream.

    When the mcryptd template is used to wrap an unkeyed hash algorithm,
    don't install a ->setkey() method to the mcryptd instance. This change
    is necessary for mcryptd to keep working with unkeyed hash algorithms
    once we start enforcing that ->setkey() is called when present.

    Signed-off-by: Eric Biggers
    Signed-off-by: Herbert Xu
    Signed-off-by: Greg Kroah-Hartman

    Eric Biggers
     
  • commit 841a3ff329713f796a63356fef6e2f72e4a3f6a3 upstream.

    When the cryptd template is used to wrap an unkeyed hash algorithm,
    don't install a ->setkey() method to the cryptd instance. This change
    is necessary for cryptd to keep working with unkeyed hash algorithms
    once we start enforcing that ->setkey() is called when present.

    Signed-off-by: Eric Biggers
    Signed-off-by: Herbert Xu
    Signed-off-by: Greg Kroah-Hartman

    Eric Biggers
     
  • commit cd6ed77ad5d223dc6299fb58f62e0f5267f7e2ba upstream.

    Templates that use an shash spawn can use crypto_shash_alg_has_setkey()
    to determine whether the underlying algorithm requires a key or not.
    But there was no corresponding function for ahash spawns. Add it.

    Note that the new function actually has to support both shash and ahash
    algorithms, since the ahash API can be used with either.

    Signed-off-by: Eric Biggers
    Signed-off-by: Herbert Xu
    Signed-off-by: Greg Kroah-Hartman

    Eric Biggers
     

13 Feb, 2018

1 commit

  • commit 5c6ac1d4f8fbdbed65dbeb8cf149d736409d16a1 upstream.

    In case buffer length is a multiple of PAGE_SIZE,
    the S/G table is incorrectly generated.
    Fix this by handling buflen = k * PAGE_SIZE separately.

    Signed-off-by: Robert Baronescu
    Signed-off-by: Herbert Xu
    Signed-off-by: Horia Geantă
    Signed-off-by: Greg Kroah-Hartman

    Robert Baronescu
     

04 Feb, 2018

3 commits

  • commit bb30b8848c85e18ca7e371d0a869e94b3e383bdf upstream.

    The user space interface allows specifying the type and mask field used
    to allocate the cipher. Only a subset of the possible flags are intended
    for user space. Therefore, white-list the allowed flags.

    In case the user space caller uses at least one non-allowed flag, EINVAL
    is returned.

    Reported-by: syzbot
    Signed-off-by: Stephan Mueller
    Signed-off-by: Herbert Xu
    Signed-off-by: Greg Kroah-Hartman

    Stephan Mueller
     
  • commit c013cee99d5a18aec8c71fee8f5f41369cd12595 upstream.

    Ensure that the input is byte swabbed before injecting it into the
    SHA3 transform. Use the get_unaligned() accessor for this so that
    we don't perform unaligned access inadvertently on architectures
    that do not support that.

    Fixes: 53964b9ee63b7075 ("crypto: sha3 - Add SHA-3 hash algorithm")
    Signed-off-by: Ard Biesheuvel
    Signed-off-by: Herbert Xu
    Signed-off-by: Greg Kroah-Hartman

    Ard Biesheuvel
     
  • commit b5b9007730ce1d90deaf25d7f678511550744bdc upstream.

    This fixes a typo in the CRYPTO_KPP dependency of CRYPTO_ECDH.

    Fixes: 3c4b23901a0c ("crypto: ecdh - Add ECDH software support")
    Signed-off-by: Hauke Mehrtens
    Signed-off-by: Herbert Xu
    Signed-off-by: Greg Kroah-Hartman

    Hauke Mehrtens
     

17 Jan, 2018

1 commit

  • commit 9a00674213a3f00394f4e3221b88f2d21fc05789 upstream.

    syzkaller triggered a NULL pointer dereference in crypto_remove_spawns()
    via a program that repeatedly and concurrently requests AEADs
    "authenc(cmac(des3_ede-asm),pcbc-aes-aesni)" and hashes "cmac(des3_ede)"
    through AF_ALG, where the hashes are requested as "untested"
    (CRYPTO_ALG_TESTED is set in ->salg_mask but clear in ->salg_feat; this
    causes the template to be instantiated for every request).

    Although AF_ALG users really shouldn't be able to request an "untested"
    algorithm, the NULL pointer dereference is actually caused by a
    longstanding race condition where crypto_remove_spawns() can encounter
    an instance which has had spawn(s) "grabbed" but hasn't yet been
    registered, resulting in ->cra_users still being NULL.

    We probably should properly initialize ->cra_users earlier, but that
    would require updating many templates individually. For now just fix
    the bug in a simple way that can easily be backported: make
    crypto_remove_spawns() treat a NULL ->cra_users list as empty.

    Reported-by: syzbot
    Signed-off-by: Eric Biggers
    Signed-off-by: Herbert Xu
    Signed-off-by: Greg Kroah-Hartman

    Eric Biggers
     

10 Jan, 2018

2 commits

  • commit d76c68109f37cb85b243a1cf0f40313afd2bae68 upstream.

    pcrypt is using the old way of freeing instances, where the ->free()
    method specified in the 'struct crypto_template' is passed a pointer to
    the 'struct crypto_instance'. But the crypto_instance is being
    kfree()'d directly, which is incorrect because the memory was actually
    allocated as an aead_instance, which contains the crypto_instance at a
    nonzero offset. Thus, the wrong pointer was being kfree()'d.

    Fix it by switching to the new way to free aead_instance's where the
    ->free() method is specified in the aead_instance itself.

    Reported-by: syzbot
    Fixes: 0496f56065e0 ("crypto: pcrypt - Add support for new AEAD interface")
    Signed-off-by: Eric Biggers
    Signed-off-by: Herbert Xu
    Signed-off-by: Greg Kroah-Hartman

    Eric Biggers
     
  • commit e57121d08c38dabec15cf3e1e2ad46721af30cae upstream.

    If the rfc7539 template was instantiated with a hash algorithm with
    digest size larger than 16 bytes (POLY1305_DIGEST_SIZE), then the digest
    overran the 'tag' buffer in 'struct chachapoly_req_ctx', corrupting the
    subsequent memory, including 'cryptlen'. This caused a crash during
    crypto_skcipher_decrypt().

    Fix it by, when instantiating the template, requiring that the
    underlying hash algorithm has the digest size expected for Poly1305.

    Reproducer:

    #include
    #include
    #include

    int main()
    {
    int algfd, reqfd;
    struct sockaddr_alg addr = {
    .salg_type = "aead",
    .salg_name = "rfc7539(chacha20,sha256)",
    };
    unsigned char buf[32] = { 0 };

    algfd = socket(AF_ALG, SOCK_SEQPACKET, 0);
    bind(algfd, (void *)&addr, sizeof(addr));
    setsockopt(algfd, SOL_ALG, ALG_SET_KEY, buf, sizeof(buf));
    reqfd = accept(algfd, 0, 0);
    write(reqfd, buf, 16);
    read(reqfd, buf, 16);
    }

    Reported-by: syzbot
    Fixes: 71ebc4d1b27d ("crypto: chacha20poly1305 - Add a ChaCha20-Poly1305 AEAD construction, RFC7539")
    Signed-off-by: Eric Biggers
    Signed-off-by: Herbert Xu
    Signed-off-by: Greg Kroah-Hartman

    Eric Biggers
     

30 Dec, 2017

1 commit

  • commit 9abffc6f2efe46c3564c04312e52e07622d40e51 upstream.

    mcryptd_enqueue_request() grabs the per-CPU queue struct and protects
    access to it with disabled preemption. Then it schedules a worker on the
    same CPU. The worker in mcryptd_queue_worker() guards access to the same
    per-CPU variable with disabled preemption.

    If we take CPU-hotplug into account then it is possible that between
    queue_work_on() and the actual invocation of the worker the CPU goes
    down and the worker will be scheduled on _another_ CPU. And here the
    preempt_disable() protection does not work anymore. The easiest thing is
    to add a spin_lock() to guard access to the list.

    Another detail: mcryptd_queue_worker() is not processing more than
    MCRYPTD_BATCH invocation in a row. If there are still items left, then
    it will invoke queue_work() to proceed with more later. *I* would
    suggest to simply drop that check because it does not use a system
    workqueue and the workqueue is already marked as "CPU_INTENSIVE". And if
    preemption is required then the scheduler should do it.
    However if queue_work() is used then the work item is marked as CPU
    unbound. That means it will try to run on the local CPU but it may run
    on another CPU as well. Especially with CONFIG_DEBUG_WQ_FORCE_RR_CPU=y.
    Again, the preempt_disable() won't work here but lock which was
    introduced will help.
    In order to keep work-item on the local CPU (and avoid RR) I changed it
    to queue_work_on().

    Signed-off-by: Sebastian Andrzej Siewior
    Signed-off-by: Herbert Xu
    Signed-off-by: Greg Kroah-Hartman

    Sebastian Andrzej Siewior
     

20 Dec, 2017

4 commits

  • [ Upstream commit 7aacbfcb331ceff3ac43096d563a1f93ed46e35e ]

    Fix the way the length of the buffers used for
    encryption / decryption are computed.
    For e.g. in case of encryption, input buffer does not contain
    an authentication tag.

    Signed-off-by: Robert Baronescu
    Signed-off-by: Herbert Xu
    Signed-off-by: Sasha Levin
    Signed-off-by: Greg Kroah-Hartman

    Robert Baronescu
     
  • commit ecaaab5649781c5a0effdaf298a925063020500e upstream.

    When asked to encrypt or decrypt 0 bytes, both the generic and x86
    implementations of Salsa20 crash in blkcipher_walk_done(), either when
    doing 'kfree(walk->buffer)' or 'free_page((unsigned long)walk->page)',
    because walk->buffer and walk->page have not been initialized.

    The bug is that Salsa20 is calling blkcipher_walk_done() even when
    nothing is in 'walk.nbytes'. But blkcipher_walk_done() is only meant to
    be called when a nonzero number of bytes have been provided.

    The broken code is part of an optimization that tries to make only one
    call to salsa20_encrypt_bytes() to process inputs that are not evenly
    divisible by 64 bytes. To fix the bug, just remove this "optimization"
    and use the blkcipher_walk API the same way all the other users do.

    Reproducer:

    #include
    #include
    #include

    int main()
    {
    int algfd, reqfd;
    struct sockaddr_alg addr = {
    .salg_type = "skcipher",
    .salg_name = "salsa20",
    };
    char key[16] = { 0 };

    algfd = socket(AF_ALG, SOCK_SEQPACKET, 0);
    bind(algfd, (void *)&addr, sizeof(addr));
    reqfd = accept(algfd, 0, 0);
    setsockopt(algfd, SOL_ALG, ALG_SET_KEY, key, sizeof(key));
    read(reqfd, key, sizeof(key));
    }

    Reported-by: syzbot
    Fixes: eb6f13eb9f81 ("[CRYPTO] salsa20_generic: Fix multi-page processing")
    Signed-off-by: Eric Biggers
    Signed-off-by: Herbert Xu
    Signed-off-by: Greg Kroah-Hartman

    Eric Biggers
     
  • commit af3ff8045bbf3e32f1a448542e73abb4c8ceb6f1 upstream.

    Because the HMAC template didn't check that its underlying hash
    algorithm is unkeyed, trying to use "hmac(hmac(sha3-512-generic))"
    through AF_ALG or through KEYCTL_DH_COMPUTE resulted in the inner HMAC
    being used without having been keyed, resulting in sha3_update() being
    called without sha3_init(), causing a stack buffer overflow.

    This is a very old bug, but it seems to have only started causing real
    problems when SHA-3 support was added (requires CONFIG_CRYPTO_SHA3)
    because the innermost hash's state is ->import()ed from a zeroed buffer,
    and it just so happens that other hash algorithms are fine with that,
    but SHA-3 is not. However, there could be arch or hardware-dependent
    hash algorithms also affected; I couldn't test everything.

    Fix the bug by introducing a function crypto_shash_alg_has_setkey()
    which tests whether a shash algorithm is keyed. Then update the HMAC
    template to require that its underlying hash algorithm is unkeyed.

    Here is a reproducer:

    #include
    #include

    int main()
    {
    int algfd;
    struct sockaddr_alg addr = {
    .salg_type = "hash",
    .salg_name = "hmac(hmac(sha3-512-generic))",
    };
    char key[4096] = { 0 };

    algfd = socket(AF_ALG, SOCK_SEQPACKET, 0);
    bind(algfd, (const struct sockaddr *)&addr, sizeof(addr));
    setsockopt(algfd, SOL_ALG, ALG_SET_KEY, key, sizeof(key));
    }

    Here was the KASAN report from syzbot:

    BUG: KASAN: stack-out-of-bounds in memcpy include/linux/string.h:341 [inline]
    BUG: KASAN: stack-out-of-bounds in sha3_update+0xdf/0x2e0 crypto/sha3_generic.c:161
    Write of size 4096 at addr ffff8801cca07c40 by task syzkaller076574/3044

    CPU: 1 PID: 3044 Comm: syzkaller076574 Not tainted 4.14.0-mm1+ #25
    Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
    Call Trace:
    __dump_stack lib/dump_stack.c:17 [inline]
    dump_stack+0x194/0x257 lib/dump_stack.c:53
    print_address_description+0x73/0x250 mm/kasan/report.c:252
    kasan_report_error mm/kasan/report.c:351 [inline]
    kasan_report+0x25b/0x340 mm/kasan/report.c:409
    check_memory_region_inline mm/kasan/kasan.c:260 [inline]
    check_memory_region+0x137/0x190 mm/kasan/kasan.c:267
    memcpy+0x37/0x50 mm/kasan/kasan.c:303
    memcpy include/linux/string.h:341 [inline]
    sha3_update+0xdf/0x2e0 crypto/sha3_generic.c:161
    crypto_shash_update+0xcb/0x220 crypto/shash.c:109
    shash_finup_unaligned+0x2a/0x60 crypto/shash.c:151
    crypto_shash_finup+0xc4/0x120 crypto/shash.c:165
    hmac_finup+0x182/0x330 crypto/hmac.c:152
    crypto_shash_finup+0xc4/0x120 crypto/shash.c:165
    shash_digest_unaligned+0x9e/0xd0 crypto/shash.c:172
    crypto_shash_digest+0xc4/0x120 crypto/shash.c:186
    hmac_setkey+0x36a/0x690 crypto/hmac.c:66
    crypto_shash_setkey+0xad/0x190 crypto/shash.c:64
    shash_async_setkey+0x47/0x60 crypto/shash.c:207
    crypto_ahash_setkey+0xaf/0x180 crypto/ahash.c:200
    hash_setkey+0x40/0x90 crypto/algif_hash.c:446
    alg_setkey crypto/af_alg.c:221 [inline]
    alg_setsockopt+0x2a1/0x350 crypto/af_alg.c:254
    SYSC_setsockopt net/socket.c:1851 [inline]
    SyS_setsockopt+0x189/0x360 net/socket.c:1830
    entry_SYSCALL_64_fastpath+0x1f/0x96

    Reported-by: syzbot
    Signed-off-by: Eric Biggers
    Signed-off-by: Herbert Xu
    Signed-off-by: Greg Kroah-Hartman

    Eric Biggers
     
  • commit d2890c3778b164fde587bc16583f3a1c87233ec5 upstream.

    In rsa_get_n(), if the buffer contained all 0's and "FIPS mode" is
    enabled, we would read one byte past the end of the buffer while
    scanning the leading zeroes. Fix it by checking 'n_sz' before '!*ptr'.

    This bug was reachable by adding a specially crafted key of type
    "asymmetric" (requires CONFIG_RSA and CONFIG_X509_CERTIFICATE_PARSER).

    KASAN report:

    BUG: KASAN: slab-out-of-bounds in rsa_get_n+0x19e/0x1d0 crypto/rsa_helper.c:33
    Read of size 1 at addr ffff88003501a708 by task keyctl/196

    CPU: 1 PID: 196 Comm: keyctl Not tainted 4.14.0-09238-g1d3b78bbc6e9 #26
    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.0-20171110_100015-anatol 04/01/2014
    Call Trace:
    rsa_get_n+0x19e/0x1d0 crypto/rsa_helper.c:33
    asn1_ber_decoder+0x82a/0x1fd0 lib/asn1_decoder.c:328
    rsa_set_pub_key+0xd3/0x320 crypto/rsa.c:278
    crypto_akcipher_set_pub_key ./include/crypto/akcipher.h:364 [inline]
    pkcs1pad_set_pub_key+0xae/0x200 crypto/rsa-pkcs1pad.c:117
    crypto_akcipher_set_pub_key ./include/crypto/akcipher.h:364 [inline]
    public_key_verify_signature+0x270/0x9d0 crypto/asymmetric_keys/public_key.c:106
    x509_check_for_self_signed+0x2ea/0x480 crypto/asymmetric_keys/x509_public_key.c:141
    x509_cert_parse+0x46a/0x620 crypto/asymmetric_keys/x509_cert_parser.c:129
    x509_key_preparse+0x61/0x750 crypto/asymmetric_keys/x509_public_key.c:174
    asymmetric_key_preparse+0xa4/0x150 crypto/asymmetric_keys/asymmetric_type.c:388
    key_create_or_update+0x4d4/0x10a0 security/keys/key.c:850
    SYSC_add_key security/keys/keyctl.c:122 [inline]
    SyS_add_key+0xe8/0x290 security/keys/keyctl.c:62
    entry_SYSCALL_64_fastpath+0x1f/0x96

    Allocated by task 196:
    __do_kmalloc mm/slab.c:3711 [inline]
    __kmalloc_track_caller+0x118/0x2e0 mm/slab.c:3726
    kmemdup+0x17/0x40 mm/util.c:118
    kmemdup ./include/linux/string.h:414 [inline]
    x509_cert_parse+0x2cb/0x620 crypto/asymmetric_keys/x509_cert_parser.c:106
    x509_key_preparse+0x61/0x750 crypto/asymmetric_keys/x509_public_key.c:174
    asymmetric_key_preparse+0xa4/0x150 crypto/asymmetric_keys/asymmetric_type.c:388
    key_create_or_update+0x4d4/0x10a0 security/keys/key.c:850
    SYSC_add_key security/keys/keyctl.c:122 [inline]
    SyS_add_key+0xe8/0x290 security/keys/keyctl.c:62
    entry_SYSCALL_64_fastpath+0x1f/0x96

    Fixes: 5a7de97309f5 ("crypto: rsa - return raw integers for the ASN.1 parser")
    Cc: Tudor Ambarus
    Signed-off-by: Eric Biggers
    Reviewed-by: James Morris
    Reviewed-by: David Howells
    Signed-off-by: Herbert Xu
    Signed-off-by: Greg Kroah-Hartman

    Eric Biggers
     

14 Dec, 2017

2 commits

  • commit 54c1fb39fe0495f846539ab765925b008f86801c upstream.

    ->pkey_algo used to be an enum, but was changed to a string by commit
    4e8ae72a75aa ("X.509: Make algo identifiers text instead of enum"). But
    two comparisons were not updated. Fix them to use strcmp().

    This bug broke signature verification in certain configurations,
    depending on whether the string constants were deduplicated or not.

    Fixes: 4e8ae72a75aa ("X.509: Make algo identifiers text instead of enum")
    Signed-off-by: Eric Biggers
    Signed-off-by: David Howells
    Signed-off-by: Greg Kroah-Hartman

    Eric Biggers
     
  • commit 0f30cbea005bd3077bd98cd29277d7fc2699c1da upstream.

    Adding a specially crafted X.509 certificate whose subjectPublicKey
    ASN.1 value is zero-length caused x509_extract_key_data() to set the
    public key size to SIZE_MAX, as it subtracted the nonexistent BIT STRING
    metadata byte. Then, x509_cert_parse() called kmemdup() with that bogus
    size, triggering the WARN_ON_ONCE() in kmalloc_slab().

    This appears to be harmless, but it still must be fixed since WARNs are
    never supposed to be user-triggerable.

    Fix it by updating x509_cert_parse() to validate that the value has a
    BIT STRING metadata byte, and that the byte is 0 which indicates that
    the number of bits in the bitstring is a multiple of 8.

    It would be nice to handle the metadata byte in asn1_ber_decoder()
    instead. But that would be tricky because in the general case a BIT
    STRING could be implicitly tagged, and/or could legitimately have a
    length that is not a whole number of bytes.

    Here was the WARN (cleaned up slightly):

    WARNING: CPU: 1 PID: 202 at mm/slab_common.c:971 kmalloc_slab+0x5d/0x70 mm/slab_common.c:971
    Modules linked in:
    CPU: 1 PID: 202 Comm: keyctl Tainted: G B 4.14.0-09238-g1d3b78bbc6e9 #26
    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.0-20171110_100015-anatol 04/01/2014
    task: ffff880033014180 task.stack: ffff8800305c8000
    Call Trace:
    __do_kmalloc mm/slab.c:3706 [inline]
    __kmalloc_track_caller+0x22/0x2e0 mm/slab.c:3726
    kmemdup+0x17/0x40 mm/util.c:118
    kmemdup include/linux/string.h:414 [inline]
    x509_cert_parse+0x2cb/0x620 crypto/asymmetric_keys/x509_cert_parser.c:106
    x509_key_preparse+0x61/0x750 crypto/asymmetric_keys/x509_public_key.c:174
    asymmetric_key_preparse+0xa4/0x150 crypto/asymmetric_keys/asymmetric_type.c:388
    key_create_or_update+0x4d4/0x10a0 security/keys/key.c:850
    SYSC_add_key security/keys/keyctl.c:122 [inline]
    SyS_add_key+0xe8/0x290 security/keys/keyctl.c:62
    entry_SYSCALL_64_fastpath+0x1f/0x96

    Fixes: 42d5ec27f873 ("X.509: Add an ASN.1 decoder")
    Signed-off-by: Eric Biggers
    Signed-off-by: David Howells
    Reviewed-by: James Morris
    Signed-off-by: Greg Kroah-Hartman

    Eric Biggers
     

24 Nov, 2017

2 commits

  • commit 12d41a023efb01b846457ccdbbcbe2b65a87d530 upstream.

    When setting the secret with the software Diffie-Hellman implementation,
    if allocating 'g' failed (e.g. if it was longer than
    MAX_EXTERN_MPI_BITS), then 'p' was freed twice: once immediately, and
    once later when the crypto_kpp tfm was destroyed.

    Fix it by using dh_free_ctx() (renamed to dh_clear_ctx()) in the error
    paths, as that correctly sets the pointers to NULL.

    KASAN report:

    MPI: mpi too large (32760 bits)
    ==================================================================
    BUG: KASAN: use-after-free in mpi_free+0x131/0x170
    Read of size 4 at addr ffff88006c7cdf90 by task reproduce_doubl/367

    CPU: 1 PID: 367 Comm: reproduce_doubl Not tainted 4.14.0-rc7-00040-g05298abde6fe #7
    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
    Call Trace:
    dump_stack+0xb3/0x10b
    ? mpi_free+0x131/0x170
    print_address_description+0x79/0x2a0
    ? mpi_free+0x131/0x170
    kasan_report+0x236/0x340
    ? akcipher_register_instance+0x90/0x90
    __asan_report_load4_noabort+0x14/0x20
    mpi_free+0x131/0x170
    ? akcipher_register_instance+0x90/0x90
    dh_exit_tfm+0x3d/0x140
    crypto_kpp_exit_tfm+0x52/0x70
    crypto_destroy_tfm+0xb3/0x250
    __keyctl_dh_compute+0x640/0xe90
    ? kasan_slab_free+0x12f/0x180
    ? dh_data_from_key+0x240/0x240
    ? key_create_or_update+0x1ee/0xb20
    ? key_instantiate_and_link+0x440/0x440
    ? lock_contended+0xee0/0xee0
    ? kfree+0xcf/0x210
    ? SyS_add_key+0x268/0x340
    keyctl_dh_compute+0xb3/0xf1
    ? __keyctl_dh_compute+0xe90/0xe90
    ? SyS_add_key+0x26d/0x340
    ? entry_SYSCALL_64_fastpath+0x5/0xbe
    ? trace_hardirqs_on_caller+0x3f4/0x560
    SyS_keyctl+0x72/0x2c0
    entry_SYSCALL_64_fastpath+0x1f/0xbe
    RIP: 0033:0x43ccf9
    RSP: 002b:00007ffeeec96158 EFLAGS: 00000246 ORIG_RAX: 00000000000000fa
    RAX: ffffffffffffffda RBX: 000000000248b9b9 RCX: 000000000043ccf9
    RDX: 00007ffeeec96170 RSI: 00007ffeeec96160 RDI: 0000000000000017
    RBP: 0000000000000046 R08: 0000000000000000 R09: 0248b9b9143dc936
    R10: 0000000000001000 R11: 0000000000000246 R12: 0000000000000000
    R13: 0000000000409670 R14: 0000000000409700 R15: 0000000000000000

    Allocated by task 367:
    save_stack_trace+0x16/0x20
    kasan_kmalloc+0xeb/0x180
    kmem_cache_alloc_trace+0x114/0x300
    mpi_alloc+0x4b/0x230
    mpi_read_raw_data+0xbe/0x360
    dh_set_secret+0x1dc/0x460
    __keyctl_dh_compute+0x623/0xe90
    keyctl_dh_compute+0xb3/0xf1
    SyS_keyctl+0x72/0x2c0
    entry_SYSCALL_64_fastpath+0x1f/0xbe

    Freed by task 367:
    save_stack_trace+0x16/0x20
    kasan_slab_free+0xab/0x180
    kfree+0xb5/0x210
    mpi_free+0xcb/0x170
    dh_set_secret+0x2d7/0x460
    __keyctl_dh_compute+0x623/0xe90
    keyctl_dh_compute+0xb3/0xf1
    SyS_keyctl+0x72/0x2c0
    entry_SYSCALL_64_fastpath+0x1f/0xbe

    Fixes: 802c7f1c84e4 ("crypto: dh - Add DH software implementation")
    Signed-off-by: Eric Biggers
    Reviewed-by: Tudor Ambarus
    Signed-off-by: Herbert Xu
    Signed-off-by: Greg Kroah-Hartman

    Eric Biggers
     
  • commit ee34e2644a78e2561742bea8c4bdcf83cabf90a7 upstream.

    setkey can be called multiple times during the existence
    of the transformation object. In case of multiple setkey calls,
    the old key was not freed and we leaked memory.
    Free the old MPI key if any.

    Signed-off-by: Tudor Ambarus
    Signed-off-by: Herbert Xu
    Signed-off-by: Greg Kroah-Hartman

    Tudor-Dan Ambarus
     

21 Nov, 2017

3 commits

  • commit ccd9888f14a8019c0bbdeeae758aba1f58693712 upstream.

    The "qat-dh" DH implementation assumes that 'key' and 'g' can be copied
    into a buffer with size 'p_size'. However it was never checked that
    that was actually the case, which most likely allowed users to cause a
    buffer underflow via KEYCTL_DH_COMPUTE.

    Fix this by updating crypto_dh_decode_key() to verify this precondition
    for all DH implementations.

    Fixes: c9839143ebbf ("crypto: qat - Add DH support")
    Signed-off-by: Eric Biggers
    Reviewed-by: Tudor Ambarus
    Signed-off-by: Herbert Xu
    Signed-off-by: Greg Kroah-Hartman

    Eric Biggers
     
  • commit 199512b1234f09e44d592153ec82b44212b2f0c4 upstream.

    If 'p' is 0 for the software Diffie-Hellman implementation, then
    dh_max_size() returns 0. In the case of KEYCTL_DH_COMPUTE, this causes
    ZERO_SIZE_PTR to be passed to sg_init_one(), which with
    CONFIG_DEBUG_SG=y triggers the 'BUG_ON(!virt_addr_valid(buf));' in
    sg_set_buf().

    Fix this by making crypto_dh_decode_key() reject 0 for 'p'. p=0 makes
    no sense for any DH implementation because 'p' is supposed to be a prime
    number. Moreover, 'mod 0' is not mathematically defined.

    Bug report:

    kernel BUG at ./include/linux/scatterlist.h:140!
    invalid opcode: 0000 [#1] SMP KASAN
    CPU: 0 PID: 27112 Comm: syz-executor2 Not tainted 4.14.0-rc7-00010-gf5dbb5d0ce32-dirty #7
    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.3-20171021_125229-anatol 04/01/2014
    task: ffff88006caac0c0 task.stack: ffff88006c7c8000
    RIP: 0010:sg_set_buf include/linux/scatterlist.h:140 [inline]
    RIP: 0010:sg_init_one+0x1b3/0x240 lib/scatterlist.c:156
    RSP: 0018:ffff88006c7cfb08 EFLAGS: 00010216
    RAX: 0000000000010000 RBX: ffff88006c7cfe30 RCX: 00000000000064ee
    RDX: ffffffff81cf64c3 RSI: ffffc90000d72000 RDI: ffffffff92e937e0
    RBP: ffff88006c7cfb30 R08: ffffed000d8f9fab R09: ffff88006c7cfd30
    R10: 0000000000000005 R11: ffffed000d8f9faa R12: ffff88006c7cfd30
    R13: 0000000000000000 R14: 0000000000000010 R15: ffff88006c7cfc50
    FS: 00007fce190fa700(0000) GS:ffff88003ea00000(0000) knlGS:0000000000000000
    CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
    CR2: 00007fffc6b33db8 CR3: 000000003cf64000 CR4: 00000000000006f0
    Call Trace:
    __keyctl_dh_compute+0xa95/0x19b0 security/keys/dh.c:360
    keyctl_dh_compute+0xac/0x100 security/keys/dh.c:434
    SYSC_keyctl security/keys/keyctl.c:1745 [inline]
    SyS_keyctl+0x72/0x2c0 security/keys/keyctl.c:1641
    entry_SYSCALL_64_fastpath+0x1f/0xbe
    RIP: 0033:0x4585c9
    RSP: 002b:00007fce190f9bd8 EFLAGS: 00000216 ORIG_RAX: 00000000000000fa
    RAX: ffffffffffffffda RBX: 0000000000738020 RCX: 00000000004585c9
    RDX: 000000002000d000 RSI: 0000000020000ff4 RDI: 0000000000000017
    RBP: 0000000000000046 R08: 0000000020008000 R09: 0000000000000000
    R10: 0000000000000000 R11: 0000000000000216 R12: 00007fff6e610cde
    R13: 00007fff6e610cdf R14: 00007fce190fa700 R15: 0000000000000000
    Code: 03 0f b6 14 02 48 89 f8 83 e0 07 83 c0 03 38 d0 7c 04 84 d2 75 33 5b 45 89 6c 24 14 41 5c 41 5d 41 5e 41 5f 5d c3 e8 fd 8f 68 ff 0b e8 f6 8f 68 ff 0f 0b e8 ef 8f 68 ff 0f 0b e8 e8 8f 68 ff 20
    RIP: sg_set_buf include/linux/scatterlist.h:140 [inline] RSP: ffff88006c7cfb08
    RIP: sg_init_one+0x1b3/0x240 lib/scatterlist.c:156 RSP: ffff88006c7cfb08

    Fixes: 802c7f1c84e4 ("crypto: dh - Add DH software implementation")
    Reviewed-by: Tudor Ambarus
    Signed-off-by: Eric Biggers
    Signed-off-by: Herbert Xu
    Signed-off-by: Greg Kroah-Hartman

    Eric Biggers
     
  • This reverts commit 6145171a6bc0abdc3eca7a4b795ede467d2ba569.

    The commit fixes a bug that was only introduced in 4.10, thus is
    irrelevant for
    Signed-off-by: Greg Kroah-Hartman

    Sasha Levin
     

15 Nov, 2017

1 commit

  • commit 441f99c90497e15aa3ad1dbabd56187e29614348 upstream.

    The IV buffer used during CCM operations is used twice, during both the
    hashing step and the ciphering step.

    When using a hardware accelerator that updates the contents of the IV
    buffer at the end of ciphering operations, the value will be modified.
    In the decryption case, the subsequent setup of the hashing algorithm
    will interpret the updated IV instead of the original value, which can
    lead to out-of-bounds writes.

    Reuse the idata buffer, only used in the hashing step, to preserve the
    IV's value during the ciphering step in the decryption case.

    Signed-off-by: Romain Izard
    Reviewed-by: Tudor Ambarus
    Signed-off-by: Herbert Xu
    Signed-off-by: Greg Kroah-Hartman

    Romain Izard
     

27 Oct, 2017

1 commit


21 Oct, 2017

1 commit

  • [ Upstream commit 12cb3a1c4184f891d965d1f39f8cfcc9ef617647 ]

    Since the
    commit f1c131b45410a202eb45cc55980a7a9e4e4b4f40
    crypto: xts - Convert to skcipher
    the XTS mode is based on ECB, so the mode must select
    ECB otherwise it can fail to initialize.

    Signed-off-by: Milan Broz
    Signed-off-by: Herbert Xu
    Signed-off-by: Sasha Levin
    Signed-off-by: Greg Kroah-Hartman

    Milan Broz
     

18 Oct, 2017

1 commit

  • commit b61907bb42409adf9b3120f741af7c57dd7e3db2 upstream.

    The shash ahash digest adaptor function may crash if given a
    zero-length input together with a null SG list. This is because
    it tries to read the SG list before looking at the length.

    This patch fixes it by checking the length first.

    Reported-by: Stephan Müller
    Signed-off-by: Herbert Xu
    Tested-by: Stephan Müller
    Signed-off-by: Greg Kroah-Hartman

    Herbert Xu
     

05 Oct, 2017

1 commit

  • commit bd6227a150fdb56e7bb734976ef6e53a2c1cb334 upstream.

    During the change to use aligned buffers, the deallocation code path was
    not updated correctly. The current code tries to free the aligned buffer
    pointer and not the original buffer pointer as it is supposed to.

    Thus, the code is updated to free the original buffer pointer and set
    the aligned buffer pointer that is used throughout the code to NULL.

    Fixes: 3cfc3b9721123 ("crypto: drbg - use aligned buffers")
    CC: Herbert Xu
    Signed-off-by: Stephan Mueller
    Signed-off-by: Herbert Xu
    Signed-off-by: Greg Kroah-Hartman

    Stephan Mueller
     

27 Sep, 2017

1 commit

  • Fixed differently upstream as commit 2d97591ef43d ("crypto: af_alg - consolidation of duplicate code")

    The SGL is MAX_SGL_ENTS + 1 in size. The last SG entry is used for the
    chaining and is properly updated with the sg_chain invocation. During
    the filling-in of the initial SG entries, sg_mark_end is called for each
    SG entry. This is appropriate as long as no additional SGL is chained
    with the current SGL. However, when a new SGL is chained and the last
    SG entry is updated with sg_chain, the last but one entry still contains
    the end marker from the sg_mark_end. This end marker must be removed as
    otherwise a walk of the chained SGLs will cause a NULL pointer
    dereference at the last but one SG entry, because sg_next will return
    NULL.

    The patch only applies to all kernels up to and including 4.13. The
    patch 2d97591ef43d0587be22ad1b0d758d6df4999a0b added to 4.14-rc1
    introduced a complete new code base which addresses this bug in
    a different way. Yet, that patch is too invasive for stable kernels
    and was therefore not marked for stable.

    Fixes: 8ff590903d5fc ("crypto: algif_skcipher - User-space interface for skcipher operations")
    Signed-off-by: Stephan Mueller
    Acked-by: Herbert Xu
    Signed-off-by: Greg Kroah-Hartman

    Stephan Mueller
     

07 Sep, 2017

1 commit

  • commit 445a582738de6802669aeed9c33ca406c23c3b1f upstream.

    For asynchronous operation, SGs are allocated without a page mapped to
    them or with a page that is not used (ref-counted). If the SGL is freed,
    the code must only call put_page for an SG if there was a page assigned
    and ref-counted in the first place.

    This fixes a kernel crash when using io_submit with more than one iocb
    using the sendmsg and sendpage (vmsplice/splice) interface.

    Signed-off-by: Stephan Mueller
    Signed-off-by: Herbert Xu
    Signed-off-by: Greg Kroah-Hartman

    Stephan Mueller
     

07 Aug, 2017

1 commit

  • commit 41cdf7a45389e01991ee31e3301ed83cb3e3f7dc upstream.

    When authencesn is used together with digest_null a crash will
    occur on the decrypt path. This is because normally we perform
    a special setup to preserve the ESN, but this is skipped if there
    is no authentication. However, on the post-authentication path
    it always expects the preservation to be in place, thus causing
    a crash when digest_null is used.

    This patch fixes this by also skipping the post-processing when
    there is no authentication.

    Fixes: 104880a6b470 ("crypto: authencesn - Convert to new AEAD...")
    Reported-by: Jan Tluka
    Signed-off-by: Herbert Xu
    Signed-off-by: Greg Kroah-Hartman

    Herbert Xu
     

15 Jul, 2017

1 commit


12 Jul, 2017

1 commit