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

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

[Dmitry Eremin-Solenikov]

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 <dbaryshkov@gmail.com>
Cc: stable@vger.kernel.org
---
 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

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

[Dmitry Eremin-Solenikov]

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

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
Cc: stable@vger.kernel.org
Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
 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

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

[Ard Biesheuvel]

(+ James)

On 20 October 2018 at 01:01, Dmitry Eremin-Solenikov
<dbaryshkov@gmail.com> wrote:
> Add AES128/192/256-CFB testvectors from NIST SP800-38A.
>
> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
> ---
>  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
>

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

[Ard Biesheuvel]

(+ James)

On 20 October 2018 at 01:01, Dmitry Eremin-Solenikov
<dbaryshkov@gmail.com> 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 <dbaryshkov@gmail.com>
> Cc: stable@vger.kernel.org
> ---
>  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
>

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

[James Bottomley]

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

Thanks!

> On 20 October 2018 at 01:01, Dmitry Eremin-Solenikov
> <dbaryshkov@gmail.com> 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 <dbaryshkov@gmail.com>
> > Cc: stable@vger.kernel.org
> > ---
> >  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

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

[Ard Biesheuvel]

On 21 October 2018 at 10:07, James Bottomley
<James.Bottomley@hansenpartnership.com> 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
>> <dbaryshkov@gmail.com> 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 <dbaryshkov@gmail.com>
>> > Cc: stable@vger.kernel.org
>> > ---
>> >  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?

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

[James Bottomley]

On October 21, 2018 9:58:04 AM GMT, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>On 21 October 2018 at 10:07, James Bottomley
><James.Bottomley@hansenpartnership.com> 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
>>> <dbaryshkov@gmail.com> 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 <dbaryshkov@gmail.com>
>>> > Cc: stable@vger.kernel.org
>>> > ---
>>> >  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.

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

[Ard Biesheuvel]

On 21 October 2018 at 11:00, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
> On October 21, 2018 9:58:04 AM GMT, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>>On 21 October 2018 at 10:07, James Bottomley
>><James.Bottomley@hansenpartnership.com> 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
>>>> <dbaryshkov@gmail.com> 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 <dbaryshkov@gmail.com>
>>>> > Cc: stable@vger.kernel.org
>>>> > ---
>>>> >  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?

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

[Dmitry Eremin-Solenikov]

Hello,

вс, 21 окт. 2018 г. в 11:07, James Bottomley
<James.Bottomley@hansenpartnership.com>:
>
> On Sun, 2018-10-21 at 09:05 +0200, Ard Biesheuvel wrote:
> > (+ James)
>
> Thanks!
>
> > On 20 October 2018 at 01:01, Dmitry Eremin-Solenikov
> > <dbaryshkov@gmail.com> 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 <dbaryshkov@gmail.com>
> > > Cc: stable@vger.kernel.org
> > > ---
> > >  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

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

[Herbert Xu]

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 <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

[Dmitry Eremin-Solenikov]

чт, 1 нояб. 2018 г. в 11:41, Herbert Xu <herbert@gondor.apana.org.au>:
>
> 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

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

[Herbert Xu]

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 <dbaryshkov@gmail.com>
> Cc: stable@vger.kernel.org
> ---
>  crypto/cfb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

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