openssl: fix for CVE-2015-3193
PATCH 1/4 openssl: fix for CVE-2015-3193
From: Armin Kuster <[email protected]>
--- ...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
meta/recipes-connectivity/openssl/openssl/CVE-2015-3193-bn-asm-x86_64-mont5.pl-fix-carry-propagating-bug-CVE.patch new file +101 -0
@@ -0,0 +1,101 @@ +From d73cc256c8e256c32ed959456101b73ba9842f72 Mon Sep 17 00:00:00 2001 +From: Andy Polyakov <[email protected]> +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 <[email protected]> +(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 <[email protected]> + +--- + 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);
Signature · 1 line
2.3.5
PATCH 2/4 openssl: fix for CVE-2015-3194
From: Armin Kuster <[email protected]>
--- .../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
meta/recipes-connectivity/openssl/openssl/0001-Add-test-for-CVE-2015-3194.patch new file +66 -0
@@ -0,0 +1,66 @@ +From 00456fded43eadd4bb94bf675ae4ea5d158a764f Mon Sep 17 00:00:00 2001 +From: "Dr. Stephen Henson" <[email protected]> +Date: Wed, 4 Nov 2015 13:30:03 +0000 +Subject: [PATCH] Add test for CVE-2015-3194 + +Reviewed-by: Richard Levitte <[email protected]> + +Upstream-Status: Backport + +This patch was imported from +https://git.openssl.org/?p=openssl.git;a=commit;h=00456fded43eadd4bb94bf675ae4ea5d158a764f +Signed-off-by: Armin Kuster <[email protected]> + +--- + 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 +
meta/recipes-connectivity/openssl/openssl/CVE-2015-3194-1-Add-PSS-parameter-check.patch new file +45 -0
45 lines not on this page · raw message
Signature · 1 line
2.3.5
PATCH 3/4 openssl: fix for CVE-2015-3195
In reply to Armin KusterFrom: Armin Kuster <[email protected]>
--- ...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
meta/recipes-connectivity/openssl/openssl/CVE-2015-3195-Fix-leak-with-ASN.1-combine.patch new file +64 -0
@@ -0,0 +1,64 @@ +From cc598f321fbac9c04da5766243ed55d55948637d Mon Sep 17 00:00:00 2001 +From: "Dr. Stephen Henson" <[email protected]> +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 <[email protected]> + +This patch was imprted from +https://git.openssl.org/?p=openssl.git;a=commit;h=cc598f321fbac9c04da5766243ed55d55948637d + +Signed-off-by: Armin Kuster <[email protected]> + +--- + 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 +
Signature · 1 line
2.3.5
PATCH 4/4 openssl: three CVE fixes
In reply to Armin KusterFrom: Armin Kuster <[email protected]>
CVE-2015-3193
CVE-2015-3194
CVE-2105-3195
--- meta/recipes-connectivity/openssl/openssl_1.0.2d.bb | 4 ++++ 1 file changed, 4 insertions(+)
meta/recipes-connectivity/openssl/openssl_1.0.2d.bb +4 -0
@@ -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"
Signature · 1 line
2.3.5
Re: [jethro][fido][PATCH 4/4] openssl: three CVE fixes
Hi,
Quoted 2 lines · Armin Kuster
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
Quoted 13 lines
meta/recipes-connectivity/openssl/openssl_1.0.2d.bb +4
-0@@ -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 \ "
Signature · 2 lines
Anders Darander, Senior System Architect ChargeStorm AB / eStorm AB
Re: [jethro][fido][PATCH 4/4] openssl: three CVE fixes
Quoted 14 lines · Anders Darander
Hi,
Quoted 2 lines · Armin Kuster
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_URIin a single patch.What's the reason to do it like this? i
Quoted 8 lines
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.
Quoted 20 lines
Any comments on this?
Cheers,
AndersQuoted 13 lines
meta/recipes-connectivity/openssl/openssl_1.0.2d.bb +4
-0@@ -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 \ "
Re: [jethro][fido][PATCH 4/4] openssl: three CVE fixes
In reply to Anders DaranderQuoted 7 lines · Anders Darander
Hi,
Quoted 2 lines · Armin Kuster
meta/recipes-connectivity/openssl/openssl_1.0.2d.bb | 4 ++++ 1 file changed, 4 insertions(+)
Quoted 6 lines
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_URIin 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.
Quoted 9 lines
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
Quoted 21 lines
Any comments on this?
Cheers,
AndersQuoted 13 lines
meta/recipes-connectivity/openssl/openssl_1.0.2d.bb +4
-0@@ -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 \ "
Re: [jethro][fido][PATCH 4/4] openssl: three CVE fixes
Quoted 12 lines · On Sat, 12 Dec 2015 13:14:52 akuster808 wrote:
Quoted 11 lines · Anders Darander
Hi,
Quoted 2 lines · Armin Kuster
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_URIin a single patch.
Quoted 7 lines
Quoted 1 line
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
Signature · 2 lines
Paul Eggleton Intel Open Source Technology Centre
Re: [jethro][fido][PATCH 4/4] openssl: three CVE fixes
In reply to akuster808Quoted 13 lines · * akuster808 <[email protected]> [151212 22:14]:
Quoted 11 lines · Anders Darander
Hi,
Quoted 2 lines · Armin Kuster
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_URIin a single patch.
Quoted 7 lines
Quoted 1 line
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.
Quoted 11 lines
Quoted 8 lines
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.
Quoted 1 line
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
Signature · 2 lines
Anders Darander, Senior System Architect ChargeStorm AB / eStorm AB