[openssl-dev] [openssl.org #4532] Replacing the “div_spoiler” hack in CBC code with Barrett reduction.
(This is a pointer to a pull request: https://github.com/openssl/openssl/pull/1027) The “div_spoiler” was designed to always trigger the slow path division on Intel chips and be sufficiently obfuscated to stop the compiler optimising it away. It was always a huge hack but I didn't know the correct solution at the time. Replace that hack with Barrett reduction, which solves the problem without using division in the first place. This change has been running in BoringSSL for some time without issues. -- Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4532 Please log in as guest with password guest if prompted -- openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Re: [openssl-dev] [openssl.org #4131] AutoReply: Memory leak when parsing invalid X509_ATTRIBUTE
(Please note that credit for this goes to libFuzzer.) ___ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
[openssl-dev] [openssl.org #4131] Memory leak when parsing invalid X509_ATTRIBUTE
The following code: EVP_PKEY_free(d2i_AutoPrivateKey(NULL, , n)); Will leak memory when fed this input: 30390201023009060138080469303080301901029ff88b298030b01b060922be0804e930864886f70d3a0180093080060102308030013b020420308204e930c3e8300105308030013b01040202ff003029021103292902009930800621022404e83001010430801b06092a86483001aa0286c030dfe980 I believe this is because the X509_ATTRIBUTE item "combines" the first and second members. Thus, after parsing an ASN1_OBJECT, the main loop in ASN1_item_ex_d2i does `pseqval = asn1_get_field_ptr(pval, seqtt);` with i=1 and gets pseqval=pval. ASN1_item_ex_d2i has code to "/* Free up and zero CHOICE value if initialised */", but it doesn't trigger in this situation and then ASN1_item_ex_d2i overwrites the ASN1_OBJECT pointer and leaks it. The trace of the leak is the following, from 1.0.1 HEAD: ==12959== 41 (40 direct, 1 indirect) bytes in 1 blocks are definitely lost in loss record 7 of 16 ==12959==at 0x40307C4: malloc (valgrind/coregrind/m_replacemalloc/vg_replace_malloc.c:270) ==12959==by 0x40AB43: default_malloc_ex (/agl/openssl/crypto/mem.c:79) ==12959==by 0x40B1D3: CRYPTO_malloc (/agl/openssl/crypto/mem.c:342) ==12959==by 0x432006: ASN1_OBJECT_new (/agl/openssl/crypto/asn1/a_object.c:346) ==12959==by 0x431E6E: c2i_ASN1_OBJECT (/agl/openssl/crypto/asn1/a_object.c:301) ==12959==by 0x4069D7: asn1_ex_c2i (/agl/openssl/crypto/asn1/tasn_dec.c:874) ==12959==by 0x40682A: asn1_d2i_ex_primitive (/agl/openssl/crypto/asn1/tasn_dec.c:831) ==12959==by 0x404F15: ASN1_item_ex_d2i (/agl/openssl/crypto/asn1/tasn_dec.c:207) ==12959==by 0x40632C: asn1_template_noexp_d2i (/agl/openssl/crypto/asn1/tasn_dec.c:691) ==12959==by 0x405F20: asn1_template_ex_d2i (/agl/openssl/crypto/asn1/tasn_dec.c:579) ==12959==by 0x4059DD: ASN1_item_ex_d2i (/agl/openssl/crypto/asn1/tasn_dec.c:443) ==12959==by 0x4061B6: asn1_template_noexp_d2i (/agl/openssl/crypto/asn1/tasn_dec.c:663) ___ openssl-bugs-mod mailing list openssl-bugs-...@openssl.org https://mta.openssl.org/mailman/listinfo/openssl-bugs-mod ___ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Re: [openssl-dev] [openssl.org #3607] nistz256 is broken.
On Thu, Dec 11, 2014 at 3:30 PM, Adam Langley a...@google.com wrote: Thanks. So far that version is good to ~1B random tests. I'll leave it going until Monday. It's good for ~6B random tests. Of course, that's not as compelling for 64-bit code as it would be for 32-bit, but I think we can safely say that there aren't any low-hanging bugs left. Thanks Andy. Cheers AGL ___ openssl-dev mailing list openssl-dev@openssl.org https://mta.opensslfoundation.net/mailman/listinfo/openssl-dev
Re: [openssl-dev] [openssl.org #3607] nistz256 is broken.
On Wed, Dec 10, 2014 at 10:05 AM, Andy Polyakov via RT r...@openssl.org wrote: Patching went wrong for you. As you seem to operate in 1.0.2 context attached is corresponding ecp_nistz256.pl. Thanks. So far that version is good to ~1B random tests. I'll leave it going until Monday. Cheers AGL ___ openssl-dev mailing list openssl-dev@openssl.org https://mta.opensslfoundation.net/mailman/listinfo/openssl-dev
Re: [openssl-dev] [openssl.org #3607] nistz256 is broken.
On Fri, Dec 5, 2014 at 6:33 AM, Andy Polyakov via RT r...@openssl.org wrote: Attached. A little bit worse performance on some CPUs. I also took opportunity to harmonize ecp_nistz256_from_mont by applying same pattern for reduction. The patch is cumulative, i.e. is not incremental to previously posted one[s], and addresses both problems, originally reported one and discovered in the course. Patch to ecp_nistz256.c referred above doesn't matter. When applying just that patch, the original test case fails. Specially this test code (C++): BIGNUM *n = nullptr, *X = nullptr, *Y = nullptr, *Z = nullptr; BIGNUM *x = BN_new(); BIGNUM *y = BN_new(); ASSERT_NE(BN_hex2bn(n, 2269520AFB46450398DE95AE59DDBDC1D42B8B7030F81BCFEF12D819C1D678DD), 0); ASSERT_NE(BN_hex2bn(X, C4EB2994C09557B400FF6A543CFB257F945E86FE3DF1D32A8128F32927666A8F), 0); ASSERT_NE(BN_hex2bn(Y, 3D5283F8F10F559AE5310005005F321B28D2D699F3E01F179F91AC6660013328), 0); ASSERT_NE(BN_hex2bn(Z, F97FD7E6757991A2C7E0C2488FF3C54E58030BCACF3FB95954FD3EF211C24631), 0); EC_GROUP *group = EC_GROUP_new_by_curve_name(NID_X9_62_prime256v1); EC_POINT *p = EC_POINT_new(group); BN_CTX *ctx = BN_CTX_new(); ASSERT_EQ(1, EC_POINT_set_Jprojective_coordinates_GFp(group, p, X, Y, Z, ctx)); EC_POINT *r = EC_POINT_new(group); // Set r = 핡×n. ASSERT_EQ(1, EC_POINT_mul(group, r, NULL, p, n, ctx)); ASSERT_EQ(1, EC_POINT_get_affine_coordinates_GFp(group, r, x, y, ctx)); char *x_out = BN_bn2hex(x); char *y_out = BN_bn2hex(y); EXPECT_STREQ(C2910AA0216D12DE30C5573CCFC4116546E3091DC1E9EC8604F634185CE40863, x_out); EXPECT_STREQ(C9071E13D688C305CE179C6168DD9066657BC6CDC1639A44B68DF7F1E0A40EDF, y_out); free(x_out); free(y_out); BN_free(x); BN_free(y); BN_free(X); BN_free(Y); BN_free(Z); BN_free(n); EC_POINT_free(r); EC_POINT_free(p); BN_CTX_free(ctx); EC_GROUP_free(group); Just to check that I'm not doing anything stupid (which is always a distinct possibility), here are the .pl[1] and resulting .s[2] file that I ended up with. [1] https://drive.google.com/file/d/0B_OzbbAp1CG5OVdVc196QmV3bG8/view?usp=sharing [2] https://drive.google.com/file/d/0B_OzbbAp1CG5Z3NoZzBqU09scFE/view?usp=sharing Cheers AGL ___ openssl-dev mailing list openssl-dev@openssl.org https://mta.opensslfoundation.net/mailman/listinfo/openssl-dev
Re: [openssl.org #3607] nistz256 is broken.
On Wed, Dec 3, 2014 at 10:12 AM, Andy Polyakov via RT r...@openssl.org wrote: Oops! Wrong patch! Correct one attached. If you feel like testing the wrong one, go ahead, but there are some later non-essential adjustments. diff --git a/crypto/ec/ecp_nistz256.c b/crypto/ec/ecp_nistz256.c index bf3fcc6..33b07ce 100644 --- a/crypto/ec/ecp_nistz256.c +++ b/crypto/ec/ecp_nistz256.c @@ -637,7 +637,7 @@ static void ecp_nistz256_windowed_mul(const EC_GROUP * group, ecp_nistz256_point_double(row[10 - 1], row[ 5 - 1]); ecp_nistz256_point_add (row[15 - 1], row[14 - 1], row[1 - 1]); ecp_nistz256_point_add (row[11 - 1], row[10 - 1], row[1 - 1]); -ecp_nistz256_point_add (row[16 - 1], row[15 - 1], row[1 - 1]); +ecp_nistz256_point_double(row[16 - 1], row[ 8 - 1]); } index = 255; I can believe that this fixes the issue, but it's just masking it, no? I'll see if I can track it down more precisely tomorrow. Cheers AGL __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
Re: [openssl.org #3607] nistz256 is broken.
On Mon, Dec 1, 2014 at 3:23 PM, Andy Polyakov via RT r...@openssl.org wrote: (Affects 1.0.2 only.) In crypto/ec/asm/ecp_nistz256-x86_64.pl, __ecp_nistz256_sqr_montq, under Now the reduction there are a number of comments saying doesn't overflow. Unfortunately, they aren't correct. Got math wrong:-( Attached is not only fixed version, but even faster one. Please test attached one instead. Squaring didn't cover one case, and AD*X is optimized. thanks! Was away last week and so didn't have a chance to try fixing this. I'll patch that it and run the tests against it. Cheers AGL On related note. It's possible to improve server-side DSA by ~5% by switching [back] to scatter-gather. [Change from scatter-gather was caused by concern about timing dependency, but I argue that concern is not valid in most cases.] There also are x86 and and ARM versions pending: # with/without -DECP_NISTZ256_ASM # Pentium +66-168% # PIII +73-175% # P4+68-140% # Core2 +90-215% # Sandy Bridge +105-265% (contemporary i[57]-* are all close to this) # Atom +66-160% # Opteron +54-112% # Bulldozer +99-240% # VIA Nano +93-300% # with/without -DECP_NISTZ256_ASM # Cortex-A8 +53-173% # Cortex-A9 +76-205% # Cortex-A15+100-316% # Snapdragon S4 +66-187% No, bug in question is not there. Nor is AD*X code path is affected. diff --git a/crypto/ec/asm/ecp_nistz256-x86_64.pl b/crypto/ec/asm/ecp_nistz256-x86_64.pl index 4486a5e..56f6c2b 100755 --- a/crypto/ec/asm/ecp_nistz256-x86_64.pl +++ b/crypto/ec/asm/ecp_nistz256-x86_64.pl @@ -31,15 +31,15 @@ # Further optimization by ap...@openssl.org: # # this/original -# Opteron +8-33% -# Bulldozer+10-30% -# P4 +14-38% -# Westmere +8-23% -# Sandy Bridge +8-24% -# Ivy Bridge +7-25% -# Haswell +5-25% -# Atom +10-32% -# VIA Nano +37-130% +# Opteron +10-43% +# Bulldozer+14-43% +# P4 +18-50% +# Westmere +12-36% +# Sandy Bridge +9-36% +# Ivy Bridge +9-36% +# Haswell +8-37% +# Atom +15-50% +# VIA Nano +43-160% # # Ranges denote minimum and maximum improvement coefficients depending # on benchmark. Lower coefficients are for ECDSA sign, relatively @@ -550,28 +550,20 @@ __ecp_nistz256_mul_montq: # and add the result to the acc. # Due to the special form of p256 we do some optimizations # - # acc[0] x p256[0] = acc[0] x 2^64 - acc[0] - # then we add acc[0] and get acc[0] x 2^64 - - mulq$poly1 - xor $t0, $t0 - add $acc0, $acc1# +=acc[0]*2^64 - adc \$0, %rdx - add %rax, $acc1 - mov $acc0, %rax - - # acc[0] x p256[2] = 0 - adc %rdx, $acc2 - adc \$0, $t0 + # acc[0] x p256[0..1] = acc[0] x 2^96 - acc[0] + # then we add acc[0] and get acc[0] x 2^96 + mov $acc0, $t1 + shl \$32, $acc0 mulq$poly3 - xor $acc0, $acc0 - add $t0, $acc3 - adc \$0, %rdx - add %rax, $acc3 + shr \$32, $t1 + add $acc0, $acc1# +=acc[0]96 + adc $t1, $acc2 + adc %rax, $acc3 mov8*1($b_ptr), %rax adc %rdx, $acc4 adc \$0, $acc5 + xor $acc0, $acc0 # Multiply by b[1] @@ -608,23 +600,17 @@ __ecp_nistz256_mul_montq: # Second reduction step - mulq$poly1 - xor $t0, $t0 - add $acc1, $acc2 - adc \$0, %rdx - add %rax, $acc2 - mov $acc1, %rax - adc %rdx, $acc3 - adc \$0, $t0 - + mov $acc1, $t1 + shl \$32, $acc1 mulq$poly3 - xor $acc1, $acc1 - add $t0, $acc4 - adc \$0, %rdx - add %rax, $acc4 + shr \$32, $t1 + add $acc1, $acc2 + adc $t1, $acc3 + adc %rax, $acc4 mov8*2($b_ptr), %rax adc %rdx, $acc5 adc \$0, $acc0 + xor $acc1, $acc1 # Multiply by b[2] @@ -661,23 +647,17 @@ __ecp_nistz256_mul_montq: # Third reduction step - mulq$poly1 - xor $t0, $t0 - add $acc2, $acc3 - adc \$0, %rdx - add %rax, $acc3 - mov $acc2, %rax - adc %rdx, $acc4 - adc \$0, $t0 - + mov $acc2, $t1 + shl
Re: [openssl.org #3607] nistz256 is broken.
On Tue, Dec 2, 2014 at 12:33 PM, Adam Langley a...@google.com wrote: thanks! Was away last week and so didn't have a chance to try fixing this. I'll patch that it and run the tests against it. I've run out of time tracking this down for today, but I got to the point where setting the Jacobian coordinates: X: C4EB2994C09557B400FF6A543CFB257F945E86FE3DF1D32A8128F32927666A8F Y: 3D5283F8F10F559AE5310005005F321B28D2D699F3E01F179F91AC6660013328 Z: F97FD7E6757991A2C7E0C2488FF3C54E58030BCACF3FB95954FD3EF211C24631 and multiplying that point by 2269520AFB46450398DE95AE59DDBDC1D42B8B7030F81BCFEF12D819C1D678DD results in the affine point: x: 4BBC2813F69EF6A4D3E69E2832E9A9E97FF59F8C136DCDBD9509BC685FF337FD y: BDCB623715CE2D983CFC2776C6EED4375454BE2C88932D43856906C1DC7A0BD7 However, I believe that the result should be: x: C2910AA0216D12DE30C5573CCFC4116546E3091DC1E9EC8604F634185CE40863 y: C9071E13D688C305CE179C6168DD9066657BC6CDC1639A44B68DF7F1E0A40EDF Cheers AGL __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
[openssl.org #3607] nistz256 is broken.
(Affects 1.0.2 only.) In crypto/ec/asm/ecp_nistz256-x86_64.pl, __ecp_nistz256_sqr_montq, under Now the reduction there are a number of comments saying doesn't overflow. Unfortunately, they aren't correct. Let f be a field element with value 52998265219372519138277318009572834528257482223861497652862868020346603903843. In Montgomery form, it's represented in memory as f*2^256 mod p, which is 58733536287848456860684025065811053850702581988990452502702607007944524443511. When passed to ecp_nistz256_sqr_mont, this results in the intermediate value (before any reduction) 0x41dd6e8bcf7e19f499c19d0f5f3bba78272201eee64c6a44ca8a4ff275b53fa93b41d5b7035af3e40a05dc36f424ab9438cdec4fa193faebf6ce951. r10 in this case contains 0x40a05dc3 and the high-word output of the multiplication after # First iteration is 0xfa193fad. The addition of r8 and r9 overflows into it leaving it as 0xfa193fae. The addition of rax and r9 also sets the carry flag thus the final add-with-carry of rdx into r10 easily overflows and leaves r10 as 0x3ab99d72. Additionally, I'm not sure about any of the other cases in the same function that have been annotated the same way. There is also a similar annotation in ecp_nistz256_mul_mont that I've not investigated. Cheers AGL __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
Re: [openssl.org #3067] [PATCH] premaster_constant_time
On Wed, Sep 24, 2014 at 7:52 AM, Emilia Käsper via RT r...@openssl.org wrote: If you'd like to verify that I didn't mess up the rewrite, that'd be great! LGTM. Thanks! I'll have to steal that for BoringSSL :) Cheers AGL __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
[openssl.org #3397]
I think the above patch is good, but incomplete. (As a niggle, it uses jl, which I think is correct because the argument is signed, but the rest of the file is using jb. The best answer would be to fix the file to use jl before applying it, but I've used jb for consistency below.) Once the crash is fixed, the code still accesses out of bounds of the modulus when the modulus is a single limb because modulus[1] is accessed unconditionally. Depending on the phase of the moon, that might crash the address space. I've fixed that in the attached and have included a test case. (The patch to bn_test.c will need to be applied manually but that's simple.) However, even with that fix there's still and out-of-bounds access of the precomputed table in bn_mul_mont_gather5. This leads to the wrong result being calculated. This only affects cases where the modulus is = 312 bits because that leads to a window size of 5 and the mont5 code being used. Sadly, that code is too odd for me to figure out what it's doing so I don't have a patch for that. Cheers AGL patch Description: Binary data
Re: [openssl.org #3106] [PATCH] Fix build with OPENSSL_NO_NEXTPROTONEG.
On Mon, Aug 5, 2013 at 7:50 PM, Piotr Sikora pi...@cloudflare.com wrote: While it cannot be enabled via ./config options, compiling OpenSSL with this define turned out to be extremely useful while adding ALPN support to 3rd-party software (i.e. to make sure that nothing in the added ALPN support relies on NPN code). Also, I wanted to make sure that SSL_select_next_proto() is not part of NPN and that it will be available going forward. I need to backport ALPN to 1.0.2 at some point, but I'll wait until this is in. (So, likely will be after my vacation.) __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
[openssl.org #3060] [PATCH] empty_record_limit
Limit the number of empty records that will be processed consecutively in order to prevent ssl3_get_record from never returning. Reported by oftc_must_be_destroyed and George Kadianakis. 0004-empty_record_limit.patch Description: Binary data
[openssl.org #3061] [PATCH] dsa_crash
Don't SEGFAULT when trying to export a public DSA key as a private key. 0005-dsa_crash.patch Description: Binary data
[openssl.org #3062] [PATCH] asm_volatile
Add volatile qualifications to two blocks of inline asm to stop GCC from eliminating them as dead code. Both volatile and memory are used because of some concern that the compiler may still cache values across the asm block without it, and because this was such a painful debugging session that I wanted to ensure that it's never repeated. 0006-asm_volatile.patch Description: Binary data
[openssl.org #3063] [PATCH] exp_zero_mod_one
Ensure that x**0 mod 1 = 0. 0007-exp_zero_mod_one.patch Description: Binary data
[openssl.org #3064] [PATCH] small_prime_generation
Ensure that, when generating small primes, the result is actually of the requested size. Fixes OpenSSL #2701. This change does not address the cases of generating safe primes, or where the |add| parameter is non-NULL. 0008-small_prime_generation.patch Description: Binary data
[openssl.org #3065] [PATCH] ec_private_key_dont_crash
This change saves several EC routines from crashing when an EC_KEY is missing a public key. The public key is optional in the EC private key format and, without this patch, running the following through `openssl ec` causes a crash: -BEGIN EC PRIVATE KEY- MBkCAQEECAECAwQFBgcIoAoGCCqGSM49AwEH -END EC PRIVATE KEY- 0009-ec_private_key_dont_crash.patch Description: Binary data
[openssl.org #3066] [PATCH] constant_time_rsa_padding
This patch tweaks the OAEP padding check to be slightly more constant time and rewrites the PKCS#1 v1.5 padding check to the same end. 0010-constant_time_rsa_padding.patch Description: Binary data
[openssl.org #3067] [PATCH] premaster_constant_time
This change alters the processing of invalid, RSA pre-master secrets so that bad encryptions are treated like random session keys in constant time. 0011-premaster_constant_time.patch Description: Binary data
[openssl.org #3056] Add secure DSA nonce flag.
This change adds the option to calculate (EC)DSA nonces by hashing the message and private key along with entropy to avoid leaking the private key if the PRNG fails. Note that this depends on the build fixes in http://rt.openssl.org/Ticket/Display.html?id=3051 dsa_nonce.patch Description: Binary data
[openssl.org #3051] [PATCH] Build fixes for 8a97a330
This patch fixes a couple of minor build issues on the current master branch (8a97a330). patch Description: Binary data
[openssl.org #2716] Re: Exporter return value confusion
On Thu, Feb 9, 2012 at 4:33 PM, Adam Langley a...@google.com wrote: This is my bad, I didn't realise that s_client had any calls in it. I'll fix it. (By fixing s_client I think). Dear Ben, please see attached patch. Cheers AGL patch Description: Binary data
