All of lore.kernel.org
 help / color / mirror / Atom feed
* [jethro][fido][PATCH 1/4] openssl: fix for CVE-2015-3193
@ 2015-12-08  1:47 Armin Kuster
  2015-12-08  1:47 ` [jethro][fido][PATCH 2/4] openssl: fix for CVE-2015-3194 Armin Kuster
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Armin Kuster @ 2015-12-08  1:47 UTC (permalink / raw)
  To: openembedded-core; +Cc: Armin Kuster

From: Armin Kuster <akuster@mvista.com>

Signed-off-by: Armin Kuster <akuster@mvista.com>
---
 ...64-mont5.pl-fix-carry-propagating-bug-CVE.patch | 101 +++++++++++++++++++++
 1 file changed, 101 insertions(+)
 create mode 100644 meta/recipes-connectivity/openssl/openssl/CVE-2015-3193-bn-asm-x86_64-mont5.pl-fix-carry-propagating-bug-CVE.patch

diff --git a/meta/recipes-connectivity/openssl/openssl/CVE-2015-3193-bn-asm-x86_64-mont5.pl-fix-carry-propagating-bug-CVE.patch b/meta/recipes-connectivity/openssl/openssl/CVE-2015-3193-bn-asm-x86_64-mont5.pl-fix-carry-propagating-bug-CVE.patch
new file mode 100644
index 0000000..125016a
--- /dev/null
+++ b/meta/recipes-connectivity/openssl/openssl/CVE-2015-3193-bn-asm-x86_64-mont5.pl-fix-carry-propagating-bug-CVE.patch
@@ -0,0 +1,101 @@
+From d73cc256c8e256c32ed959456101b73ba9842f72 Mon Sep 17 00:00:00 2001
+From: Andy Polyakov <appro@openssl.org>
+Date: Tue, 1 Dec 2015 09:00:32 +0100
+Subject: [PATCH] bn/asm/x86_64-mont5.pl: fix carry propagating bug
+ (CVE-2015-3193).
+
+Reviewed-by: Richard Levitte <levitte@openssl.org>
+(cherry picked from commit e7c078db57908cbf16074c68034977565ffaf107)
+
+Upstream-Status: Backport
+
+This patch was imported from 
+https://git.openssl.org/?p=openssl.git;a=commit;h=d73cc256c8e256c32ed959456101b73ba9842f72
+
+Signed-off-by: Armin Kuster <akuster@mvista.com>
+
+---
+ crypto/bn/asm/x86_64-mont5.pl | 22 +++++++++++++++++++---
+ crypto/bn/bntest.c            | 18 ++++++++++++++++++
+ 2 files changed, 37 insertions(+), 3 deletions(-)
+
+Index: openssl-1.0.2d/crypto/bn/asm/x86_64-mont5.pl
+===================================================================
+--- openssl-1.0.2d.orig/crypto/bn/asm/x86_64-mont5.pl
++++ openssl-1.0.2d/crypto/bn/asm/x86_64-mont5.pl
+@@ -1779,6 +1779,15 @@ sqr8x_reduction:
+ .align	32
+ .L8x_tail_done:
+ 	add	(%rdx),%r8		# can this overflow?
++	adc	\$0,%r9
++	adc	\$0,%r10
++	adc	\$0,%r11
++	adc	\$0,%r12
++	adc	\$0,%r13
++	adc	\$0,%r14
++	adc	\$0,%r15		# can't overflow, because we
++					# started with "overhung" part
++					# of multiplication
+ 	xor	%rax,%rax
+ 
+ 	neg	$carry
+@@ -3125,6 +3134,15 @@ sqrx8x_reduction:
+ .align	32
+ .Lsqrx8x_tail_done:
+ 	add	24+8(%rsp),%r8		# can this overflow?
++	adc	\$0,%r9
++	adc	\$0,%r10
++	adc	\$0,%r11
++	adc	\$0,%r12
++	adc	\$0,%r13
++	adc	\$0,%r14
++	adc	\$0,%r15		# can't overflow, because we
++					# started with "overhung" part
++					# of multiplication
+ 	mov	$carry,%rax		# xor	%rax,%rax
+ 
+ 	sub	16+8(%rsp),$carry	# mov 16(%rsp),%cf
+@@ -3168,13 +3186,11 @@ my ($rptr,$nptr)=("%rdx","%rbp");
+ my @ri=map("%r$_",(10..13));
+ my @ni=map("%r$_",(14..15));
+ $code.=<<___;
+-	xor	%rbx,%rbx
++	xor	%ebx,%ebx
+ 	sub	%r15,%rsi		# compare top-most words
+ 	adc	%rbx,%rbx
+ 	mov	%rcx,%r10		# -$num
+-	.byte	0x67
+ 	or	%rbx,%rax
+-	.byte	0x67
+ 	mov	%rcx,%r9		# -$num
+ 	xor	\$1,%rax
+ 	sar	\$3+2,%rcx		# cf=0
+Index: openssl-1.0.2d/crypto/bn/bntest.c
+===================================================================
+--- openssl-1.0.2d.orig/crypto/bn/bntest.c
++++ openssl-1.0.2d/crypto/bn/bntest.c
+@@ -1027,6 +1027,24 @@ int test_mod_exp_mont_consttime(BIO *bp,
+             return 0;
+         }
+     }
++
++    /* Regression test for carry propagation bug in sqr8x_reduction */
++    BN_hex2bn(&a, "050505050505");
++    BN_hex2bn(&b, "02");
++    BN_hex2bn(&c,
++        "4141414141414141414141274141414141414141414141414141414141414141"
++        "4141414141414141414141414141414141414141414141414141414141414141"
++        "4141414141414141414141800000000000000000000000000000000000000000"
++        "0000000000000000000000000000000000000000000000000000000000000000"
++        "0000000000000000000000000000000000000000000000000000000000000000"
++        "0000000000000000000000000000000000000000000000000000000001");
++    BN_mod_exp(d, a, b, c, ctx);
++    BN_mul(e, a, a, ctx);
++    if (BN_cmp(d, e)) {
++        fprintf(stderr, "BN_mod_exp and BN_mul produce different results!\n");
++        return 0;
++    }
++
+     BN_free(a);
+     BN_free(b);
+     BN_free(c);
-- 
2.3.5



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [jethro][fido][PATCH 2/4] openssl: fix for CVE-2015-3194
  2015-12-08  1:47 [jethro][fido][PATCH 1/4] openssl: fix for CVE-2015-3193 Armin Kuster
@ 2015-12-08  1:47 ` Armin Kuster
  2015-12-08  1:47 ` [jethro][fido][PATCH 3/4] openssl: fix for CVE-2015-3195 Armin Kuster
  2015-12-08  1:47 ` [jethro][fido][PATCH 4/4] openssl: three CVE fixes Armin Kuster
  2 siblings, 0 replies; 9+ messages in thread
From: Armin Kuster @ 2015-12-08  1:47 UTC (permalink / raw)
  To: openembedded-core; +Cc: Armin Kuster

From: Armin Kuster <akuster@mvista.com>

Signed-off-by: Armin Kuster <akuster@mvista.com>
---
 .../openssl/0001-Add-test-for-CVE-2015-3194.patch  | 66 ++++++++++++++++++++++
 .../CVE-2015-3194-1-Add-PSS-parameter-check.patch  | 45 +++++++++++++++
 2 files changed, 111 insertions(+)
 create mode 100644 meta/recipes-connectivity/openssl/openssl/0001-Add-test-for-CVE-2015-3194.patch
 create mode 100644 meta/recipes-connectivity/openssl/openssl/CVE-2015-3194-1-Add-PSS-parameter-check.patch

diff --git a/meta/recipes-connectivity/openssl/openssl/0001-Add-test-for-CVE-2015-3194.patch b/meta/recipes-connectivity/openssl/openssl/0001-Add-test-for-CVE-2015-3194.patch
new file mode 100644
index 0000000..39a2e5a
--- /dev/null
+++ b/meta/recipes-connectivity/openssl/openssl/0001-Add-test-for-CVE-2015-3194.patch
@@ -0,0 +1,66 @@
+From 00456fded43eadd4bb94bf675ae4ea5d158a764f Mon Sep 17 00:00:00 2001
+From: "Dr. Stephen Henson" <steve@openssl.org>
+Date: Wed, 4 Nov 2015 13:30:03 +0000
+Subject: [PATCH] Add test for CVE-2015-3194
+
+Reviewed-by: Richard Levitte <levitte@openssl.org>
+
+Upstream-Status: Backport
+
+This patch was imported from 
+https://git.openssl.org/?p=openssl.git;a=commit;h=00456fded43eadd4bb94bf675ae4ea5d158a764f
+Signed-off-by: Armin Kuster <akuster@mvista.com>
+
+---
+ test/certs/pss1.pem | 21 +++++++++++++++++++++
+ test/tx509          |  7 +++++++
+ 2 files changed, 28 insertions(+)
+ create mode 100644 test/certs/pss1.pem
+
+diff --git a/test/certs/pss1.pem b/test/certs/pss1.pem
+new file mode 100644
+index 0000000..29da71d
+--- /dev/null
++++ b/test/certs/pss1.pem
+@@ -0,0 +1,21 @@
++-----BEGIN CERTIFICATE-----
++MIIDdjCCAjqgAwIBAgIJANcwZLyfEv7DMD4GCSqGSIb3DQEBCjAxoA0wCwYJYIZI
++AWUDBAIBoRowGAYJKoZIhvcNAQEIMAsGCWCGSAFlAwQCAaIEAgIA3jAnMSUwIwYD
++VQQDDBxUZXN0IEludmFsaWQgUFNTIGNlcnRpZmljYXRlMB4XDTE1MTEwNDE2MDIz
++NVoXDTE1MTIwNDE2MDIzNVowJzElMCMGA1UEAwwcVGVzdCBJbnZhbGlkIFBTUyBj
++ZXJ0aWZpY2F0ZTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAMTaM7WH
++qVCAGAIA+zL1KWvvASTrhlq+1ePdO7wsrWX2KiYoTYrJYTnxhLnn0wrHqApt79nL
++IBG7cfShyZqFHOY/IzlYPMVt+gPo293gw96Fds5JBsjhjkyGnOyr9OUntFqvxDbT
++IIFU7o9IdxD4edaqjRv+fegVE+B79pDk4s0ujsk6dULtCg9Rst0ucGFo19mr+b7k
++dbfn8pZ72ZNDJPueVdrUAWw9oll61UcYfk75XdrLk6JlL41GrYHc8KlfXf43gGQq
++QfrpHkg4Ih2cI6Wt2nhFGAzrlcorzLliQIUJRIhM8h4IgDfpBpaPdVQLqS2pFbXa
++5eQjqiyJwak2vJ8CAwEAAaNQME4wHQYDVR0OBBYEFCt180N4oGUt5LbzBwQ4Ia+2
++4V97MB8GA1UdIwQYMBaAFCt180N4oGUt5LbzBwQ4Ia+24V97MAwGA1UdEwQFMAMB
++Af8wMQYJKoZIhvcNAQEKMCSgDTALBglghkgBZQMEAgGhDTALBgkqhkiG9w0BAQii
++BAICAN4DggEBAAjBtm90lGxgddjc4Xu/nbXXFHVs2zVcHv/mqOZoQkGB9r/BVgLb
++xhHrFZ2pHGElbUYPfifdS9ztB73e1d4J+P29o0yBqfd4/wGAc/JA8qgn6AAEO/Xn
++plhFeTRJQtLZVl75CkHXgUGUd3h+ADvKtcBuW9dSUncaUrgNKR8u/h/2sMG38RWY
++DzBddC/66YTa3r7KkVUfW7yqRQfELiGKdcm+bjlTEMsvS+EhHup9CzbpoCx2Fx9p
++NPtFY3yEObQhmL1JyoCRWqBE75GzFPbRaiux5UpEkns+i3trkGssZzsOuVqHNTNZ
++lC9+9hPHIoc9UMmAQNo1vGIW3NWVoeGbaJ8=
++-----END CERTIFICATE-----
+diff --git a/test/tx509 b/test/tx509
+index 0ce3b52..77f5cac 100644
+--- a/test/tx509
++++ b/test/tx509
+@@ -74,5 +74,12 @@ if [ $? != 0 ]; then exit 1; fi
+ cmp x509-f.p x509-ff.p3
+ if [ $? != 0 ]; then exit 1; fi
+ 
++echo "Parsing test certificates"
++
++$cmd -in certs/pss1.pem -text -noout >/dev/null
++if [ $? != 0 ]; then exit 1; fi
++
++echo OK
++
+ /bin/rm -f x509-f.* x509-ff.* x509-fff.*
+ exit 0
+-- 
+2.3.5
+
diff --git a/meta/recipes-connectivity/openssl/openssl/CVE-2015-3194-1-Add-PSS-parameter-check.patch b/meta/recipes-connectivity/openssl/openssl/CVE-2015-3194-1-Add-PSS-parameter-check.patch
new file mode 100644
index 0000000..13d4891
--- /dev/null
+++ b/meta/recipes-connectivity/openssl/openssl/CVE-2015-3194-1-Add-PSS-parameter-check.patch
@@ -0,0 +1,45 @@
+From c394a488942387246653833359a5c94b5832674e Mon Sep 17 00:00:00 2001
+From: "Dr. Stephen Henson" <steve@openssl.org>
+Date: Fri, 2 Oct 2015 12:35:19 +0100
+Subject: [PATCH] Add PSS parameter check.
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+Avoid seg fault by checking mgf1 parameter is not NULL. This can be
+triggered during certificate verification so could be a DoS attack
+against a client or a server enabling client authentication.
+
+Thanks to Loïc Jonas Etienne (Qnective AG) for discovering this bug.
+
+CVE-2015-3194
+
+Reviewed-by: Richard Levitte <levitte@openssl.org>
+
+Upstream-Status: Backport
+
+This patch was imported from 
+https://git.openssl.org/?p=openssl.git;a=commit;h=c394a488942387246653833359a5c94b5832674e
+
+Signed-off-by: Armin Kuster <akuster@mvista.com>
+
+---
+ crypto/rsa/rsa_ameth.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/crypto/rsa/rsa_ameth.c b/crypto/rsa/rsa_ameth.c
+index ca3922e..4e06218 100644
+--- a/crypto/rsa/rsa_ameth.c
++++ b/crypto/rsa/rsa_ameth.c
+@@ -268,7 +268,7 @@ static X509_ALGOR *rsa_mgf1_decode(X509_ALGOR *alg)
+ {
+     const unsigned char *p;
+     int plen;
+-    if (alg == NULL)
++    if (alg == NULL || alg->parameter == NULL)
+         return NULL;
+     if (OBJ_obj2nid(alg->algorithm) != NID_mgf1)
+         return NULL;
+-- 
+2.3.5
+
-- 
2.3.5



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [jethro][fido][PATCH 3/4] openssl: fix for CVE-2015-3195
  2015-12-08  1:47 [jethro][fido][PATCH 1/4] openssl: fix for CVE-2015-3193 Armin Kuster
  2015-12-08  1:47 ` [jethro][fido][PATCH 2/4] openssl: fix for CVE-2015-3194 Armin Kuster
@ 2015-12-08  1:47 ` Armin Kuster
  2015-12-08  1:47 ` [jethro][fido][PATCH 4/4] openssl: three CVE fixes Armin Kuster
  2 siblings, 0 replies; 9+ messages in thread
From: Armin Kuster @ 2015-12-08  1:47 UTC (permalink / raw)
  To: openembedded-core; +Cc: Armin Kuster

From: Armin Kuster <akuster@mvista.com>

Signed-off-by: Armin Kuster <akuster@mvista.com>
---
 ...CVE-2015-3195-Fix-leak-with-ASN.1-combine.patch | 64 ++++++++++++++++++++++
 1 file changed, 64 insertions(+)
 create mode 100644 meta/recipes-connectivity/openssl/openssl/CVE-2015-3195-Fix-leak-with-ASN.1-combine.patch

diff --git a/meta/recipes-connectivity/openssl/openssl/CVE-2015-3195-Fix-leak-with-ASN.1-combine.patch b/meta/recipes-connectivity/openssl/openssl/CVE-2015-3195-Fix-leak-with-ASN.1-combine.patch
new file mode 100644
index 0000000..08adc0c
--- /dev/null
+++ b/meta/recipes-connectivity/openssl/openssl/CVE-2015-3195-Fix-leak-with-ASN.1-combine.patch
@@ -0,0 +1,64 @@
+From cc598f321fbac9c04da5766243ed55d55948637d Mon Sep 17 00:00:00 2001
+From: "Dr. Stephen Henson" <steve@openssl.org>
+Date: Tue, 10 Nov 2015 19:03:07 +0000
+Subject: [PATCH] Fix leak with ASN.1 combine.
+
+When parsing a combined structure pass a flag to the decode routine
+so on error a pointer to the parent structure is not zeroed as
+this will leak any additional components in the parent.
+
+This can leak memory in any application parsing PKCS#7 or CMS structures.
+
+CVE-2015-3195.
+
+Thanks to Adam Langley (Google/BoringSSL) for discovering this bug using
+libFuzzer.
+
+PR#4131
+
+Reviewed-by: Richard Levitte <levitte@openssl.org>
+
+This patch was imprted from
+https://git.openssl.org/?p=openssl.git;a=commit;h=cc598f321fbac9c04da5766243ed55d55948637d
+
+Signed-off-by: Armin Kuster <akuster@mvista.com>
+
+---
+ crypto/asn1/tasn_dec.c | 7 +++++--
+ 1 file changed, 5 insertions(+), 2 deletions(-)
+
+diff --git a/crypto/asn1/tasn_dec.c b/crypto/asn1/tasn_dec.c
+index febf605..9256049 100644
+--- a/crypto/asn1/tasn_dec.c
++++ b/crypto/asn1/tasn_dec.c
+@@ -180,6 +180,8 @@ int ASN1_item_ex_d2i(ASN1_VALUE **pval, const unsigned char **in, long len,
+     int otag;
+     int ret = 0;
+     ASN1_VALUE **pchptr, *ptmpval;
++    int combine = aclass & ASN1_TFLG_COMBINE;
++    aclass &= ~ASN1_TFLG_COMBINE;
+     if (!pval)
+         return 0;
+     if (aux && aux->asn1_cb)
+@@ -500,7 +502,8 @@ int ASN1_item_ex_d2i(ASN1_VALUE **pval, const unsigned char **in, long len,
+  auxerr:
+     ASN1err(ASN1_F_ASN1_ITEM_EX_D2I, ASN1_R_AUX_ERROR);
+  err:
+-    ASN1_item_ex_free(pval, it);
++    if (combine == 0)
++        ASN1_item_ex_free(pval, it);
+     if (errtt)
+         ERR_add_error_data(4, "Field=", errtt->field_name,
+                            ", Type=", it->sname);
+@@ -689,7 +692,7 @@ static int asn1_template_noexp_d2i(ASN1_VALUE **val,
+     } else {
+         /* Nothing special */
+         ret = ASN1_item_ex_d2i(val, &p, len, ASN1_ITEM_ptr(tt->item),
+-                               -1, 0, opt, ctx);
++                               -1, tt->flags & ASN1_TFLG_COMBINE, opt, ctx);
+         if (!ret) {
+             ASN1err(ASN1_F_ASN1_TEMPLATE_NOEXP_D2I, ERR_R_NESTED_ASN1_ERROR);
+             goto err;
+-- 
+2.3.5
+
-- 
2.3.5



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [jethro][fido][PATCH 4/4] openssl: three CVE fixes
  2015-12-08  1:47 [jethro][fido][PATCH 1/4] openssl: fix for CVE-2015-3193 Armin Kuster
  2015-12-08  1:47 ` [jethro][fido][PATCH 2/4] openssl: fix for CVE-2015-3194 Armin Kuster
  2015-12-08  1:47 ` [jethro][fido][PATCH 3/4] openssl: fix for CVE-2015-3195 Armin Kuster
@ 2015-12-08  1:47 ` Armin Kuster
  2015-12-08  7:49   ` Anders Darander
  2 siblings, 1 reply; 9+ messages in thread
From: Armin Kuster @ 2015-12-08  1:47 UTC (permalink / raw)
  To: openembedded-core; +Cc: Armin Kuster

From: Armin Kuster <akuster@mvista.com>

CVE-2015-3193
CVE-2015-3194
CVE-2105-3195

Signed-off-by: Armin Kuster <akuster@mvista.com>
---
 meta/recipes-connectivity/openssl/openssl_1.0.2d.bb | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/meta/recipes-connectivity/openssl/openssl_1.0.2d.bb b/meta/recipes-connectivity/openssl/openssl_1.0.2d.bb
index fd56841..3864e88 100644
--- a/meta/recipes-connectivity/openssl/openssl_1.0.2d.bb
+++ b/meta/recipes-connectivity/openssl/openssl_1.0.2d.bb
@@ -37,6 +37,10 @@ SRC_URI += "file://configure-targets.patch \
             file://crypto_use_bigint_in_x86-64_perl.patch \
             file://openssl-1.0.2a-x32-asm.patch \
             file://ptest_makefile_deps.patch  \
+            file://CVE-2015-3193-bn-asm-x86_64-mont5.pl-fix-carry-propagating-bug-CVE.patch \
+            file://CVE-2015-3194-1-Add-PSS-parameter-check.patch \
+            file://0001-Add-test-for-CVE-2015-3194.patch \
+            file://CVE-2015-3195-Fix-leak-with-ASN.1-combine.patch \
            "
 
 SRC_URI[md5sum] = "38dd619b2e77cbac69b99f52a053d25a"
-- 
2.3.5



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [jethro][fido][PATCH 4/4] openssl: three CVE fixes
  2015-12-08  1:47 ` [jethro][fido][PATCH 4/4] openssl: three CVE fixes Armin Kuster
@ 2015-12-08  7:49   ` Anders Darander
  2015-12-08  8:14     ` Robert Yang
  2015-12-12 21:14     ` akuster808
  0 siblings, 2 replies; 9+ messages in thread
From: Anders Darander @ 2015-12-08  7:49 UTC (permalink / raw)
  To: openembedded-core

Hi,

* Armin Kuster <akuster808@gmail.com> [151208 02:49]:

>  meta/recipes-connectivity/openssl/openssl_1.0.2d.bb | 4 ++++
>  1 file changed, 4 insertions(+)

I'm just a little curious about this serious, and a few others that I've
seen recently. They all add a number of CVE-patches, with one commit per
patch, and as the last commit, they all get added to SRC_URI in a single
patch.

What's the reason to do it like this? i

I'd personally prefer to have each CVE-path also add the patch to
SRC_URI, as that make cherry-picking more straightforward. And it also
ensures that if we have a need to bisect some issue, that'll work. At
the same time that will make the meta-data consistent, i.e. no dead
patches.

I'd personally even prefer that whole series squashed to one commit,
compared to this adding a lot of un-applied patches. 

Any comments on this?

Cheers,
Anders

> diff --git a/meta/recipes-connectivity/openssl/openssl_1.0.2d.bb b/meta/recipes-connectivity/openssl/openssl_1.0.2d.bb
> index fd56841..3864e88 100644
> --- a/meta/recipes-connectivity/openssl/openssl_1.0.2d.bb
> +++ b/meta/recipes-connectivity/openssl/openssl_1.0.2d.bb
> @@ -37,6 +37,10 @@ SRC_URI += "file://configure-targets.patch \
>              file://crypto_use_bigint_in_x86-64_perl.patch \
>              file://openssl-1.0.2a-x32-asm.patch \
>              file://ptest_makefile_deps.patch  \
> +            file://CVE-2015-3193-bn-asm-x86_64-mont5.pl-fix-carry-propagating-bug-CVE.patch \
> +            file://CVE-2015-3194-1-Add-PSS-parameter-check.patch \
> +            file://0001-Add-test-for-CVE-2015-3194.patch \
> +            file://CVE-2015-3195-Fix-leak-with-ASN.1-combine.patch \
>             "

-- 
Anders Darander, Senior System Architect
ChargeStorm AB / eStorm AB


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [jethro][fido][PATCH 4/4] openssl: three CVE fixes
  2015-12-08  7:49   ` Anders Darander
@ 2015-12-08  8:14     ` Robert Yang
  2015-12-12 21:14     ` akuster808
  1 sibling, 0 replies; 9+ messages in thread
From: Robert Yang @ 2015-12-08  8:14 UTC (permalink / raw)
  To: openembedded-core



On 12/08/2015 03:49 PM, Anders Darander wrote:
> Hi,
>
> * Armin Kuster <akuster808@gmail.com> [151208 02:49]:
>
>>   meta/recipes-connectivity/openssl/openssl_1.0.2d.bb | 4 ++++
>>   1 file changed, 4 insertions(+)
>
> I'm just a little curious about this serious, and a few others that I've
> seen recently. They all add a number of CVE-patches, with one commit per
> patch, and as the last commit, they all get added to SRC_URI in a single
> patch.
>
> What's the reason to do it like this? i
>
> I'd personally prefer to have each CVE-path also add the patch to
> SRC_URI, as that make cherry-picking more straightforward. And it also
> ensures that if we have a need to bisect some issue, that'll work. At
> the same time that will make the meta-data consistent, i.e. no dead
> patches.
>
> I'd personally even prefer that whole series squashed to one commit,
> compared to this adding a lot of un-applied patches.

Yes, I think that would be better.

// Robert.

>
> Any comments on this?
>
> Cheers,
> Anders
>
>> diff --git a/meta/recipes-connectivity/openssl/openssl_1.0.2d.bb b/meta/recipes-connectivity/openssl/openssl_1.0.2d.bb
>> index fd56841..3864e88 100644
>> --- a/meta/recipes-connectivity/openssl/openssl_1.0.2d.bb
>> +++ b/meta/recipes-connectivity/openssl/openssl_1.0.2d.bb
>> @@ -37,6 +37,10 @@ SRC_URI += "file://configure-targets.patch \
>>               file://crypto_use_bigint_in_x86-64_perl.patch \
>>               file://openssl-1.0.2a-x32-asm.patch \
>>               file://ptest_makefile_deps.patch  \
>> +            file://CVE-2015-3193-bn-asm-x86_64-mont5.pl-fix-carry-propagating-bug-CVE.patch \
>> +            file://CVE-2015-3194-1-Add-PSS-parameter-check.patch \
>> +            file://0001-Add-test-for-CVE-2015-3194.patch \
>> +            file://CVE-2015-3195-Fix-leak-with-ASN.1-combine.patch \
>>              "
>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [jethro][fido][PATCH 4/4] openssl: three CVE fixes
  2015-12-08  7:49   ` Anders Darander
  2015-12-08  8:14     ` Robert Yang
@ 2015-12-12 21:14     ` akuster808
  2015-12-13 19:32       ` Paul Eggleton
  2015-12-14 10:40       ` Anders Darander
  1 sibling, 2 replies; 9+ messages in thread
From: akuster808 @ 2015-12-12 21:14 UTC (permalink / raw)
  To: openembedded-core, anders



On 12/07/2015 11:49 PM, Anders Darander wrote:
> Hi,
> 
> * Armin Kuster <akuster808@gmail.com> [151208 02:49]:
> 
>>  meta/recipes-connectivity/openssl/openssl_1.0.2d.bb | 4 ++++
>>  1 file changed, 4 insertions(+)
> 
> I'm just a little curious about this serious, and a few others that I've
> seen recently. They all add a number of CVE-patches, with one commit per
> patch, and as the last commit, they all get added to SRC_URI in a single
> patch.
> 
> What's the reason to do it like this? i

Each CVE patch can be leveraged independently so back porting to other
branches is simpler and less work.  The recipe file is  where merge
conflicts will occur. Not all CVE's are weighted the same so someone who
has a product in the field can easily cherry pick the CVE's they want or
need. This was talked about on IRC a few weeks ago.

> 
> I'd personally prefer to have each CVE-path also add the patch to
> SRC_URI, as that make cherry-picking more straightforward. And it also
> ensures that if we have a need to bisect some issue, that'll work. At
> the same time that will make the meta-data consistent, i.e. no dead
> patches.
> 
> I'd personally even prefer that whole series squashed to one commit,
> compared to this adding a lot of un-applied patches.

That would add more overhead to the work I do internally as I need them
in the format you have seen here.

Are this patches not in the preferred method as described on wiki?

Regards,
- armin

> Any comments on this?


> 
> Cheers,
> Anders
> 
>> diff --git a/meta/recipes-connectivity/openssl/openssl_1.0.2d.bb b/meta/recipes-connectivity/openssl/openssl_1.0.2d.bb
>> index fd56841..3864e88 100644
>> --- a/meta/recipes-connectivity/openssl/openssl_1.0.2d.bb
>> +++ b/meta/recipes-connectivity/openssl/openssl_1.0.2d.bb
>> @@ -37,6 +37,10 @@ SRC_URI += "file://configure-targets.patch \
>>              file://crypto_use_bigint_in_x86-64_perl.patch \
>>              file://openssl-1.0.2a-x32-asm.patch \
>>              file://ptest_makefile_deps.patch  \
>> +            file://CVE-2015-3193-bn-asm-x86_64-mont5.pl-fix-carry-propagating-bug-CVE.patch \
>> +            file://CVE-2015-3194-1-Add-PSS-parameter-check.patch \
>> +            file://0001-Add-test-for-CVE-2015-3194.patch \
>> +            file://CVE-2015-3195-Fix-leak-with-ASN.1-combine.patch \
>>             "
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [jethro][fido][PATCH 4/4] openssl: three CVE fixes
  2015-12-12 21:14     ` akuster808
@ 2015-12-13 19:32       ` Paul Eggleton
  2015-12-14 10:40       ` Anders Darander
  1 sibling, 0 replies; 9+ messages in thread
From: Paul Eggleton @ 2015-12-13 19:32 UTC (permalink / raw)
  To: openembedded-core, akuster808; +Cc: anders

On Sat, 12 Dec 2015 13:14:52 akuster808 wrote:
> On 12/07/2015 11:49 PM, Anders Darander wrote:
> > Hi,
> > 
> > * Armin Kuster <akuster808@gmail.com> [151208 02:49]:
> >>  meta/recipes-connectivity/openssl/openssl_1.0.2d.bb | 4 ++++
> >>  1 file changed, 4 insertions(+)
> > 
> > I'm just a little curious about this serious, and a few others that I've
> > seen recently. They all add a number of CVE-patches, with one commit per
> > patch, and as the last commit, they all get added to SRC_URI in a single
> > patch.
> > 
> > What's the reason to do it like this? i
> 
> Each CVE patch can be leveraged independently so back porting to other
> branches is simpler and less work.  The recipe file is  where merge
> conflicts will occur. Not all CVE's are weighted the same so someone who
> has a product in the field can easily cherry pick the CVE's they want or
> need. This was talked about on IRC a few weeks ago.

Well, except they might cherry-pick the fix commit on the assumption that it 
fixes the CVE, when unfortunately it doesn't because the included patch isn't 
actually applied within the recipe in that commit.

I can see how this makes things slightly easier for backporting, but honestly 
I don't like it. I don't believe it matches with our practice up to this point 
either.

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [jethro][fido][PATCH 4/4] openssl: three CVE fixes
  2015-12-12 21:14     ` akuster808
  2015-12-13 19:32       ` Paul Eggleton
@ 2015-12-14 10:40       ` Anders Darander
  1 sibling, 0 replies; 9+ messages in thread
From: Anders Darander @ 2015-12-14 10:40 UTC (permalink / raw)
  To: akuster808; +Cc: openembedded-core

* akuster808 <akuster808@gmail.com> [151212 22:14]:
> On 12/07/2015 11:49 PM, Anders Darander wrote:
> > Hi,

> > * Armin Kuster <akuster808@gmail.com> [151208 02:49]:

> >>  meta/recipes-connectivity/openssl/openssl_1.0.2d.bb | 4 ++++
> >>  1 file changed, 4 insertions(+)

> > I'm just a little curious about this serious, and a few others that I've
> > seen recently. They all add a number of CVE-patches, with one commit per
> > patch, and as the last commit, they all get added to SRC_URI in a single
> > patch.

> > What's the reason to do it like this? i

> Each CVE patch can be leveraged independently so back porting to other
> branches is simpler and less work.  The recipe file is  where merge
> conflicts will occur. Not all CVE's are weighted the same so someone who
> has a product in the field can easily cherry pick the CVE's they want or
> need. This was talked about on IRC a few weeks ago.

Well, you have the obviuos risk of backporting a fix, and assuming that
it gets applied.

The confligt will anyway appear on in the recipe, so there's no real
issue. Handling that conflict (which is easy to spot and solve) will
also ensure that you're aware of the fact that you have a difference
between your local branch and upstream. 

> > I'd personally prefer to have each CVE-path also add the patch to
> > SRC_URI, as that make cherry-picking more straightforward. And it also
> > ensures that if we have a need to bisect some issue, that'll work. At
> > the same time that will make the meta-data consistent, i.e. no dead
> > patches.

> > I'd personally even prefer that whole series squashed to one commit,
> > compared to this adding a lot of un-applied patches.

> That would add more overhead to the work I do internally as I need them
> in the format you have seen here.

Sure, I don̈́'t want the squashed patch either, but I want commit that
introduces a patch to also apply it.

> Are this patches not in the preferred method as described on wiki?

In my opinion, the patches should be applied by the commit adding them.
Otherwise, we've got commits that just adds dead weight to the
meta-data.  Just as we prefer a commit that removes a patch from SRC_URI
to also remove the actual patch itself.

Cheers,
Anders

-- 
Anders Darander, Senior System Architect
ChargeStorm AB / eStorm AB


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2015-12-14 10:40 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-08  1:47 [jethro][fido][PATCH 1/4] openssl: fix for CVE-2015-3193 Armin Kuster
2015-12-08  1:47 ` [jethro][fido][PATCH 2/4] openssl: fix for CVE-2015-3194 Armin Kuster
2015-12-08  1:47 ` [jethro][fido][PATCH 3/4] openssl: fix for CVE-2015-3195 Armin Kuster
2015-12-08  1:47 ` [jethro][fido][PATCH 4/4] openssl: three CVE fixes Armin Kuster
2015-12-08  7:49   ` Anders Darander
2015-12-08  8:14     ` Robert Yang
2015-12-12 21:14     ` akuster808
2015-12-13 19:32       ` Paul Eggleton
2015-12-14 10:40       ` Anders Darander

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.