2018-10-20 07:10:02

by Dmitry Baryshkov

[permalink] [raw]
Subject: [PATCH 1/2] crypto: fix cfb mode decryption

crypto_cfb_decrypt_segment() incorrectly XOR'ed generated keystream with
IV, rather than with data stream, resulting in incorrect decryption.
Test vectors will be added in the next patch.

Signed-off-by: Dmitry Eremin-Solenikov <[email protected]>
Cc: [email protected]
---
crypto/cfb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crypto/cfb.c b/crypto/cfb.c
index a0d68c09e1b9..fd4e8500e121 100644
--- a/crypto/cfb.c
+++ b/crypto/cfb.c
@@ -144,7 +144,7 @@ static int crypto_cfb_decrypt_segment(struct skcipher_walk *walk,

do {
crypto_cfb_encrypt_one(tfm, iv, dst);
- crypto_xor(dst, iv, bsize);
+ crypto_xor(dst, src, bsize);
iv = src;

src += bsize;
--
2.19.1


2018-10-21 17:11:42

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH 1/2] crypto: fix cfb mode decryption

On 21 October 2018 at 10:07, James Bottomley
<[email protected]> wrote:
> On Sun, 2018-10-21 at 09:05 +0200, Ard Biesheuvel wrote:
>> (+ James)
>
> Thanks!
>
>> On 20 October 2018 at 01:01, Dmitry Eremin-Solenikov
>> <[email protected]> wrote:
>> > crypto_cfb_decrypt_segment() incorrectly XOR'ed generated keystream
>> > with
>> > IV, rather than with data stream, resulting in incorrect
>> > decryption.
>> > Test vectors will be added in the next patch.
>> >
>> > Signed-off-by: Dmitry Eremin-Solenikov <[email protected]>
>> > Cc: [email protected]
>> > ---
>> > crypto/cfb.c | 2 +-
>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/crypto/cfb.c b/crypto/cfb.c
>> > index a0d68c09e1b9..fd4e8500e121 100644
>> > --- a/crypto/cfb.c
>> > +++ b/crypto/cfb.c
>> > @@ -144,7 +144,7 @@ static int crypto_cfb_decrypt_segment(struct
>> > skcipher_walk *walk,
>> >
>> > do {
>> > crypto_cfb_encrypt_one(tfm, iv, dst);
>> > - crypto_xor(dst, iv, bsize);
>> > + crypto_xor(dst, src, bsize);
>
> This does look right. I think the reason the TPM code works is that it
> always does encrypt/decrypt in-place, which is a separate piece of the
> code which appears to be correct.
>

Yeah I figured that.

So where is the TPM code that actually uses this code?

2018-10-21 17:21:11

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH 1/2] crypto: fix cfb mode decryption

On 21 October 2018 at 11:00, James Bottomley
<[email protected]> wrote:
> On October 21, 2018 9:58:04 AM GMT, Ard Biesheuvel <[email protected]> wrote:
>>On 21 October 2018 at 10:07, James Bottomley
>><[email protected]> wrote:
>>> On Sun, 2018-10-21 at 09:05 +0200, Ard Biesheuvel wrote:
>>>> (+ James)
>>>
>>> Thanks!
>>>
>>>> On 20 October 2018 at 01:01, Dmitry Eremin-Solenikov
>>>> <[email protected]> wrote:
>>>> > crypto_cfb_decrypt_segment() incorrectly XOR'ed generated
>>keystream
>>>> > with
>>>> > IV, rather than with data stream, resulting in incorrect
>>>> > decryption.
>>>> > Test vectors will be added in the next patch.
>>>> >
>>>> > Signed-off-by: Dmitry Eremin-Solenikov <[email protected]>
>>>> > Cc: [email protected]
>>>> > ---
>>>> > crypto/cfb.c | 2 +-
>>>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>>>> >
>>>> > diff --git a/crypto/cfb.c b/crypto/cfb.c
>>>> > index a0d68c09e1b9..fd4e8500e121 100644
>>>> > --- a/crypto/cfb.c
>>>> > +++ b/crypto/cfb.c
>>>> > @@ -144,7 +144,7 @@ static int crypto_cfb_decrypt_segment(struct
>>>> > skcipher_walk *walk,
>>>> >
>>>> > do {
>>>> > crypto_cfb_encrypt_one(tfm, iv, dst);
>>>> > - crypto_xor(dst, iv, bsize);
>>>> > + crypto_xor(dst, src, bsize);
>>>
>>> This does look right. I think the reason the TPM code works is that
>>it
>>> always does encrypt/decrypt in-place, which is a separate piece of
>>the
>>> code which appears to be correct.
>>>
>>
>>Yeah I figured that.
>>
>>So where is the TPM code that actually uses this code?
>
> It was posted to the integrity list a while ago. I'm planning a repost shortly.
>

OK, found it. Mind cc'ing me on that repost?

2018-10-21 17:14:16

by James Bottomley

[permalink] [raw]
Subject: Re: [PATCH 1/2] crypto: fix cfb mode decryption

On October 21, 2018 9:58:04 AM GMT, Ard Biesheuvel <[email protected]> wrote:
>On 21 October 2018 at 10:07, James Bottomley
><[email protected]> wrote:
>> On Sun, 2018-10-21 at 09:05 +0200, Ard Biesheuvel wrote:
>>> (+ James)
>>
>> Thanks!
>>
>>> On 20 October 2018 at 01:01, Dmitry Eremin-Solenikov
>>> <[email protected]> wrote:
>>> > crypto_cfb_decrypt_segment() incorrectly XOR'ed generated
>keystream
>>> > with
>>> > IV, rather than with data stream, resulting in incorrect
>>> > decryption.
>>> > Test vectors will be added in the next patch.
>>> >
>>> > Signed-off-by: Dmitry Eremin-Solenikov <[email protected]>
>>> > Cc: [email protected]
>>> > ---
>>> > crypto/cfb.c | 2 +-
>>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>>> >
>>> > diff --git a/crypto/cfb.c b/crypto/cfb.c
>>> > index a0d68c09e1b9..fd4e8500e121 100644
>>> > --- a/crypto/cfb.c
>>> > +++ b/crypto/cfb.c
>>> > @@ -144,7 +144,7 @@ static int crypto_cfb_decrypt_segment(struct
>>> > skcipher_walk *walk,
>>> >
>>> > do {
>>> > crypto_cfb_encrypt_one(tfm, iv, dst);
>>> > - crypto_xor(dst, iv, bsize);
>>> > + crypto_xor(dst, src, bsize);
>>
>> This does look right. I think the reason the TPM code works is that
>it
>> always does encrypt/decrypt in-place, which is a separate piece of
>the
>> code which appears to be correct.
>>
>
>Yeah I figured that.
>
>So where is the TPM code that actually uses this code?

It was posted to the integrity list a while ago. I'm planning a repost shortly.

James


--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

2018-10-21 16:21:17

by James Bottomley

[permalink] [raw]
Subject: Re: [PATCH 1/2] crypto: fix cfb mode decryption

On Sun, 2018-10-21 at 09:05 +0200, Ard Biesheuvel wrote:
> (+ James)

Thanks!

> On 20 October 2018 at 01:01, Dmitry Eremin-Solenikov
> <[email protected]> wrote:
> > crypto_cfb_decrypt_segment() incorrectly XOR'ed generated keystream
> > with
> > IV, rather than with data stream, resulting in incorrect
> > decryption.
> > Test vectors will be added in the next patch.
> >
> > Signed-off-by: Dmitry Eremin-Solenikov <[email protected]>
> > Cc: [email protected]
> > ---
> > crypto/cfb.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/crypto/cfb.c b/crypto/cfb.c
> > index a0d68c09e1b9..fd4e8500e121 100644
> > --- a/crypto/cfb.c
> > +++ b/crypto/cfb.c
> > @@ -144,7 +144,7 @@ static int crypto_cfb_decrypt_segment(struct
> > skcipher_walk *walk,
> >
> > do {
> > crypto_cfb_encrypt_one(tfm, iv, dst);
> > - crypto_xor(dst, iv, bsize);
> > + crypto_xor(dst, src, bsize);

This does look right. I think the reason the TPM code works is that it
always does encrypt/decrypt in-place, which is a separate piece of the
code which appears to be correct.

James

2018-10-21 15:19:56

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH 2/2] crypto: testmgr: add AES-CFB tests

(+ James)

On 20 October 2018 at 01:01, Dmitry Eremin-Solenikov
<[email protected]> wrote:
> Add AES128/192/256-CFB testvectors from NIST SP800-38A.
>
> Signed-off-by: Dmitry Eremin-Solenikov <[email protected]>
> Cc: [email protected]
> Signed-off-by: Dmitry Eremin-Solenikov <[email protected]>
> ---
> crypto/tcrypt.c | 5 ++++
> crypto/testmgr.c | 7 +++++
> crypto/testmgr.h | 76 ++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 88 insertions(+)
>
> diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
> index bdde95e8d369..a6315827d240 100644
> --- a/crypto/tcrypt.c
> +++ b/crypto/tcrypt.c
> @@ -1733,6 +1733,7 @@ static int do_test(const char *alg, u32 type, u32 mask, int m, u32 num_mb)
> ret += tcrypt_test("xts(aes)");
> ret += tcrypt_test("ctr(aes)");
> ret += tcrypt_test("rfc3686(ctr(aes))");
> + ret += tcrypt_test("cfb(aes)");
> break;
>
> case 11:
> @@ -2059,6 +2060,10 @@ static int do_test(const char *alg, u32 type, u32 mask, int m, u32 num_mb)
> speed_template_16_24_32);
> test_cipher_speed("ctr(aes)", DECRYPT, sec, NULL, 0,
> speed_template_16_24_32);
> + test_cipher_speed("cfb(aes)", ENCRYPT, sec, NULL, 0,
> + speed_template_16_24_32);
> + test_cipher_speed("cfb(aes)", DECRYPT, sec, NULL, 0,
> + speed_template_16_24_32);
> break;
>
> case 201:
> diff --git a/crypto/testmgr.c b/crypto/testmgr.c
> index a1d42245082a..016d61c419fc 100644
> --- a/crypto/testmgr.c
> +++ b/crypto/testmgr.c
> @@ -2684,6 +2684,13 @@ static const struct alg_test_desc alg_test_descs[] = {
> .dec = __VECS(aes_ccm_dec_tv_template)
> }
> }
> + }, {
> + .alg = "cfb(aes)",
> + .test = alg_test_skcipher,
> + .fips_allowed = 1,
> + .suite = {
> + .cipher = __VECS(aes_cfb_tv_template)
> + },
> }, {
> .alg = "chacha20",
> .test = alg_test_skcipher,
> diff --git a/crypto/testmgr.h b/crypto/testmgr.h
> index 173111c70746..19b6d184c8fb 100644
> --- a/crypto/testmgr.h
> +++ b/crypto/testmgr.h
> @@ -12081,6 +12081,82 @@ static const struct cipher_testvec aes_cbc_tv_template[] = {
> },
> };
>
> +static const struct cipher_testvec aes_cfb_tv_template[] = {
> + { /* From NIST SP800-38A */
> + .key = "\x2b\x7e\x15\x16\x28\xae\xd2\xa6"
> + "\xab\xf7\x15\x88\x09\xcf\x4f\x3c",
> + .klen = 16,
> + .iv = "\x00\x01\x02\x03\x04\x05\x06\x07"
> + "\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f",
> + .ptext = "\x6b\xc1\xbe\xe2\x2e\x40\x9f\x96"
> + "\xe9\x3d\x7e\x11\x73\x93\x17\x2a"
> + "\xae\x2d\x8a\x57\x1e\x03\xac\x9c"
> + "\x9e\xb7\x6f\xac\x45\xaf\x8e\x51"
> + "\x30\xc8\x1c\x46\xa3\x5c\xe4\x11"
> + "\xe5\xfb\xc1\x19\x1a\x0a\x52\xef"
> + "\xf6\x9f\x24\x45\xdf\x4f\x9b\x17"
> + "\xad\x2b\x41\x7b\xe6\x6c\x37\x10",
> + .ctext = "\x3b\x3f\xd9\x2e\xb7\x2d\xad\x20"
> + "\x33\x34\x49\xf8\xe8\x3c\xfb\x4a"
> + "\xc8\xa6\x45\x37\xa0\xb3\xa9\x3f"
> + "\xcd\xe3\xcd\xad\x9f\x1c\xe5\x8b"
> + "\x26\x75\x1f\x67\xa3\xcb\xb1\x40"
> + "\xb1\x80\x8c\xf1\x87\xa4\xf4\xdf"
> + "\xc0\x4b\x05\x35\x7c\x5d\x1c\x0e"
> + "\xea\xc4\xc6\x6f\x9f\xf7\xf2\xe6",
> + .len = 64,
> + }, {
> + .key = "\x8e\x73\xb0\xf7\xda\x0e\x64\x52"
> + "\xc8\x10\xf3\x2b\x80\x90\x79\xe5"
> + "\x62\xf8\xea\xd2\x52\x2c\x6b\x7b",
> + .klen = 24,
> + .iv = "\x00\x01\x02\x03\x04\x05\x06\x07"
> + "\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f",
> + .ptext = "\x6b\xc1\xbe\xe2\x2e\x40\x9f\x96"
> + "\xe9\x3d\x7e\x11\x73\x93\x17\x2a"
> + "\xae\x2d\x8a\x57\x1e\x03\xac\x9c"
> + "\x9e\xb7\x6f\xac\x45\xaf\x8e\x51"
> + "\x30\xc8\x1c\x46\xa3\x5c\xe4\x11"
> + "\xe5\xfb\xc1\x19\x1a\x0a\x52\xef"
> + "\xf6\x9f\x24\x45\xdf\x4f\x9b\x17"
> + "\xad\x2b\x41\x7b\xe6\x6c\x37\x10",
> + .ctext = "\xcd\xc8\x0d\x6f\xdd\xf1\x8c\xab"
> + "\x34\xc2\x59\x09\xc9\x9a\x41\x74"
> + "\x67\xce\x7f\x7f\x81\x17\x36\x21"
> + "\x96\x1a\x2b\x70\x17\x1d\x3d\x7a"
> + "\x2e\x1e\x8a\x1d\xd5\x9b\x88\xb1"
> + "\xc8\xe6\x0f\xed\x1e\xfa\xc4\xc9"
> + "\xc0\x5f\x9f\x9c\xa9\x83\x4f\xa0"
> + "\x42\xae\x8f\xba\x58\x4b\x09\xff",
> + .len = 64,
> + }, {
> + .key = "\x60\x3d\xeb\x10\x15\xca\x71\xbe"
> + "\x2b\x73\xae\xf0\x85\x7d\x77\x81"
> + "\x1f\x35\x2c\x07\x3b\x61\x08\xd7"
> + "\x2d\x98\x10\xa3\x09\x14\xdf\xf4",
> + .klen = 32,
> + .iv = "\x00\x01\x02\x03\x04\x05\x06\x07"
> + "\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f",
> + .ptext = "\x6b\xc1\xbe\xe2\x2e\x40\x9f\x96"
> + "\xe9\x3d\x7e\x11\x73\x93\x17\x2a"
> + "\xae\x2d\x8a\x57\x1e\x03\xac\x9c"
> + "\x9e\xb7\x6f\xac\x45\xaf\x8e\x51"
> + "\x30\xc8\x1c\x46\xa3\x5c\xe4\x11"
> + "\xe5\xfb\xc1\x19\x1a\x0a\x52\xef"
> + "\xf6\x9f\x24\x45\xdf\x4f\x9b\x17"
> + "\xad\x2b\x41\x7b\xe6\x6c\x37\x10",
> + .ctext = "\xdc\x7e\x84\xbf\xda\x79\x16\x4b"
> + "\x7e\xcd\x84\x86\x98\x5d\x38\x60"
> + "\x39\xff\xed\x14\x3b\x28\xb1\xc8"
> + "\x32\x11\x3c\x63\x31\xe5\x40\x7b"
> + "\xdf\x10\x13\x24\x15\xe5\x4b\x92"
> + "\xa1\x3e\xd0\xa8\x26\x7a\xe2\xf9"
> + "\x75\xa3\x85\x74\x1a\xb9\xce\xf8"
> + "\x20\x31\x62\x3d\x55\xb1\xe4\x71",
> + .len = 64,
> + },
> +};
> +
> static const struct aead_testvec hmac_md5_ecb_cipher_null_enc_tv_template[] = {
> { /* Input data from RFC 2410 Case 1 */
> #ifdef __LITTLE_ENDIAN
> --
> 2.19.1
>

2018-10-20 07:10:04

by Dmitry Baryshkov

[permalink] [raw]
Subject: [PATCH 2/2] crypto: testmgr: add AES-CFB tests

Add AES128/192/256-CFB testvectors from NIST SP800-38A.

Signed-off-by: Dmitry Eremin-Solenikov <[email protected]>
Cc: [email protected]
Signed-off-by: Dmitry Eremin-Solenikov <[email protected]>
---
crypto/tcrypt.c | 5 ++++
crypto/testmgr.c | 7 +++++
crypto/testmgr.h | 76 ++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 88 insertions(+)

diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index bdde95e8d369..a6315827d240 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -1733,6 +1733,7 @@ static int do_test(const char *alg, u32 type, u32 mask, int m, u32 num_mb)
ret += tcrypt_test("xts(aes)");
ret += tcrypt_test("ctr(aes)");
ret += tcrypt_test("rfc3686(ctr(aes))");
+ ret += tcrypt_test("cfb(aes)");
break;

case 11:
@@ -2059,6 +2060,10 @@ static int do_test(const char *alg, u32 type, u32 mask, int m, u32 num_mb)
speed_template_16_24_32);
test_cipher_speed("ctr(aes)", DECRYPT, sec, NULL, 0,
speed_template_16_24_32);
+ test_cipher_speed("cfb(aes)", ENCRYPT, sec, NULL, 0,
+ speed_template_16_24_32);
+ test_cipher_speed("cfb(aes)", DECRYPT, sec, NULL, 0,
+ speed_template_16_24_32);
break;

case 201:
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index a1d42245082a..016d61c419fc 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -2684,6 +2684,13 @@ static const struct alg_test_desc alg_test_descs[] = {
.dec = __VECS(aes_ccm_dec_tv_template)
}
}
+ }, {
+ .alg = "cfb(aes)",
+ .test = alg_test_skcipher,
+ .fips_allowed = 1,
+ .suite = {
+ .cipher = __VECS(aes_cfb_tv_template)
+ },
}, {
.alg = "chacha20",
.test = alg_test_skcipher,
diff --git a/crypto/testmgr.h b/crypto/testmgr.h
index 173111c70746..19b6d184c8fb 100644
--- a/crypto/testmgr.h
+++ b/crypto/testmgr.h
@@ -12081,6 +12081,82 @@ static const struct cipher_testvec aes_cbc_tv_template[] = {
},
};

+static const struct cipher_testvec aes_cfb_tv_template[] = {
+ { /* From NIST SP800-38A */
+ .key = "\x2b\x7e\x15\x16\x28\xae\xd2\xa6"
+ "\xab\xf7\x15\x88\x09\xcf\x4f\x3c",
+ .klen = 16,
+ .iv = "\x00\x01\x02\x03\x04\x05\x06\x07"
+ "\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f",
+ .ptext = "\x6b\xc1\xbe\xe2\x2e\x40\x9f\x96"
+ "\xe9\x3d\x7e\x11\x73\x93\x17\x2a"
+ "\xae\x2d\x8a\x57\x1e\x03\xac\x9c"
+ "\x9e\xb7\x6f\xac\x45\xaf\x8e\x51"
+ "\x30\xc8\x1c\x46\xa3\x5c\xe4\x11"
+ "\xe5\xfb\xc1\x19\x1a\x0a\x52\xef"
+ "\xf6\x9f\x24\x45\xdf\x4f\x9b\x17"
+ "\xad\x2b\x41\x7b\xe6\x6c\x37\x10",
+ .ctext = "\x3b\x3f\xd9\x2e\xb7\x2d\xad\x20"
+ "\x33\x34\x49\xf8\xe8\x3c\xfb\x4a"
+ "\xc8\xa6\x45\x37\xa0\xb3\xa9\x3f"
+ "\xcd\xe3\xcd\xad\x9f\x1c\xe5\x8b"
+ "\x26\x75\x1f\x67\xa3\xcb\xb1\x40"
+ "\xb1\x80\x8c\xf1\x87\xa4\xf4\xdf"
+ "\xc0\x4b\x05\x35\x7c\x5d\x1c\x0e"
+ "\xea\xc4\xc6\x6f\x9f\xf7\xf2\xe6",
+ .len = 64,
+ }, {
+ .key = "\x8e\x73\xb0\xf7\xda\x0e\x64\x52"
+ "\xc8\x10\xf3\x2b\x80\x90\x79\xe5"
+ "\x62\xf8\xea\xd2\x52\x2c\x6b\x7b",
+ .klen = 24,
+ .iv = "\x00\x01\x02\x03\x04\x05\x06\x07"
+ "\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f",
+ .ptext = "\x6b\xc1\xbe\xe2\x2e\x40\x9f\x96"
+ "\xe9\x3d\x7e\x11\x73\x93\x17\x2a"
+ "\xae\x2d\x8a\x57\x1e\x03\xac\x9c"
+ "\x9e\xb7\x6f\xac\x45\xaf\x8e\x51"
+ "\x30\xc8\x1c\x46\xa3\x5c\xe4\x11"
+ "\xe5\xfb\xc1\x19\x1a\x0a\x52\xef"
+ "\xf6\x9f\x24\x45\xdf\x4f\x9b\x17"
+ "\xad\x2b\x41\x7b\xe6\x6c\x37\x10",
+ .ctext = "\xcd\xc8\x0d\x6f\xdd\xf1\x8c\xab"
+ "\x34\xc2\x59\x09\xc9\x9a\x41\x74"
+ "\x67\xce\x7f\x7f\x81\x17\x36\x21"
+ "\x96\x1a\x2b\x70\x17\x1d\x3d\x7a"
+ "\x2e\x1e\x8a\x1d\xd5\x9b\x88\xb1"
+ "\xc8\xe6\x0f\xed\x1e\xfa\xc4\xc9"
+ "\xc0\x5f\x9f\x9c\xa9\x83\x4f\xa0"
+ "\x42\xae\x8f\xba\x58\x4b\x09\xff",
+ .len = 64,
+ }, {
+ .key = "\x60\x3d\xeb\x10\x15\xca\x71\xbe"
+ "\x2b\x73\xae\xf0\x85\x7d\x77\x81"
+ "\x1f\x35\x2c\x07\x3b\x61\x08\xd7"
+ "\x2d\x98\x10\xa3\x09\x14\xdf\xf4",
+ .klen = 32,
+ .iv = "\x00\x01\x02\x03\x04\x05\x06\x07"
+ "\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f",
+ .ptext = "\x6b\xc1\xbe\xe2\x2e\x40\x9f\x96"
+ "\xe9\x3d\x7e\x11\x73\x93\x17\x2a"
+ "\xae\x2d\x8a\x57\x1e\x03\xac\x9c"
+ "\x9e\xb7\x6f\xac\x45\xaf\x8e\x51"
+ "\x30\xc8\x1c\x46\xa3\x5c\xe4\x11"
+ "\xe5\xfb\xc1\x19\x1a\x0a\x52\xef"
+ "\xf6\x9f\x24\x45\xdf\x4f\x9b\x17"
+ "\xad\x2b\x41\x7b\xe6\x6c\x37\x10",
+ .ctext = "\xdc\x7e\x84\xbf\xda\x79\x16\x4b"
+ "\x7e\xcd\x84\x86\x98\x5d\x38\x60"
+ "\x39\xff\xed\x14\x3b\x28\xb1\xc8"
+ "\x32\x11\x3c\x63\x31\xe5\x40\x7b"
+ "\xdf\x10\x13\x24\x15\xe5\x4b\x92"
+ "\xa1\x3e\xd0\xa8\x26\x7a\xe2\xf9"
+ "\x75\xa3\x85\x74\x1a\xb9\xce\xf8"
+ "\x20\x31\x62\x3d\x55\xb1\xe4\x71",
+ .len = 64,
+ },
+};
+
static const struct aead_testvec hmac_md5_ecb_cipher_null_enc_tv_template[] = {
{ /* Input data from RFC 2410 Case 1 */
#ifdef __LITTLE_ENDIAN
--
2.19.1

2018-10-21 15:19:19

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH 1/2] crypto: fix cfb mode decryption

(+ James)

On 20 October 2018 at 01:01, Dmitry Eremin-Solenikov
<[email protected]> wrote:
> crypto_cfb_decrypt_segment() incorrectly XOR'ed generated keystream with
> IV, rather than with data stream, resulting in incorrect decryption.
> Test vectors will be added in the next patch.
>
> Signed-off-by: Dmitry Eremin-Solenikov <[email protected]>
> Cc: [email protected]
> ---
> crypto/cfb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/crypto/cfb.c b/crypto/cfb.c
> index a0d68c09e1b9..fd4e8500e121 100644
> --- a/crypto/cfb.c
> +++ b/crypto/cfb.c
> @@ -144,7 +144,7 @@ static int crypto_cfb_decrypt_segment(struct skcipher_walk *walk,
>
> do {
> crypto_cfb_encrypt_one(tfm, iv, dst);
> - crypto_xor(dst, iv, bsize);
> + crypto_xor(dst, src, bsize);
> iv = src;
>
> src += bsize;
> --
> 2.19.1
>

2018-11-01 17:43:19

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH 1/2] crypto: fix cfb mode decryption

On Thu, Nov 01, 2018 at 11:32:37AM +0300, Dmitry Eremin-Solenikov wrote:
>
> Since 4.20 pull went into Linus'es tree, any change of getting these two patches
> in crypto tree?

These aren't critical enough for the current mainline so they will
go in at the next merge window.

Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2018-11-01 17:34:50

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH 1/2] crypto: fix cfb mode decryption

Hello,

вс, 21 окт. 2018 г. в 11:07, James Bottomley
<[email protected]>:
>
> On Sun, 2018-10-21 at 09:05 +0200, Ard Biesheuvel wrote:
> > (+ James)
>
> Thanks!
>
> > On 20 October 2018 at 01:01, Dmitry Eremin-Solenikov
> > <[email protected]> wrote:
> > > crypto_cfb_decrypt_segment() incorrectly XOR'ed generated keystream
> > > with
> > > IV, rather than with data stream, resulting in incorrect
> > > decryption.
> > > Test vectors will be added in the next patch.
> > >
> > > Signed-off-by: Dmitry Eremin-Solenikov <[email protected]>
> > > Cc: [email protected]
> > > ---
> > > crypto/cfb.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/crypto/cfb.c b/crypto/cfb.c
> > > index a0d68c09e1b9..fd4e8500e121 100644
> > > --- a/crypto/cfb.c
> > > +++ b/crypto/cfb.c
> > > @@ -144,7 +144,7 @@ static int crypto_cfb_decrypt_segment(struct
> > > skcipher_walk *walk,
> > >
> > > do {
> > > crypto_cfb_encrypt_one(tfm, iv, dst);
> > > - crypto_xor(dst, iv, bsize);
> > > + crypto_xor(dst, src, bsize);
>
> This does look right. I think the reason the TPM code works is that it
> always does encrypt/decrypt in-place, which is a separate piece of the
> code which appears to be correct.

Since 4.20 pull went into Linus'es tree, any change of getting these two patches
in crypto tree?

--
With best wishes
Dmitry

2018-11-01 17:44:19

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH 1/2] crypto: fix cfb mode decryption

чт, 1 нояб. 2018 г. в 11:41, Herbert Xu <[email protected]>:
>
> On Thu, Nov 01, 2018 at 11:32:37AM +0300, Dmitry Eremin-Solenikov wrote:
> >
> > Since 4.20 pull went into Linus'es tree, any change of getting these two patches
> > in crypto tree?
>
> These aren't critical enough for the current mainline so they will
> go in at the next merge window.

Thank you.


--
With best wishes
Dmitry

2018-11-09 19:32:04

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH 1/2] crypto: fix cfb mode decryption

On Sat, Oct 20, 2018 at 02:01:52AM +0300, Dmitry Eremin-Solenikov wrote:
> crypto_cfb_decrypt_segment() incorrectly XOR'ed generated keystream with
> IV, rather than with data stream, resulting in incorrect decryption.
> Test vectors will be added in the next patch.
>
> Signed-off-by: Dmitry Eremin-Solenikov <[email protected]>
> Cc: [email protected]
> ---
> crypto/cfb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

All applied. Thanks.
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt