It always returns one, so just void it.
Change-Id: I8733cc3d6b20185e782cf0291e9c0dc57712bb63
Reviewed-on: https://boringssl-review.googlesource.com/22564
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Node's default settings spell P-256 as prime256v1. This comes from
OpenSSL additionally allowing the long and short names of each curve's
NID. This works out to one additional name per curve for the ones we
support. To avoid depending on the giant OID table, this replicates the
names in libssl.
Change-Id: I456a2db6939eb6745e5a9d2f12cf6886e6265b9f
Reviewed-on: https://boringssl-review.googlesource.com/22545
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Credit to OSS-Fuzz for finding this.
CVE-2017-3736
(Imported from upstream's 668a709a8d7ea374ee72ad2d43ac72ec60a80eee and
420b88cec8c6f7c67fad07bf508dcccab094f134.)
This bug does not affect BoringSSL as we do not enable the ADX code.
Note the test vector had to be tweaked to take things in and out of
Montgomery form. (There may be something to be said for test vectors for
just BN_mod_mul_montgomery, though we'd need separate 64-bit and 32-bit
ones because R can be different.)
Change-Id: I832070731ac1c5f893f9c1746892fc4a32f023f5
Reviewed-on: https://boringssl-review.googlesource.com/22484
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Change-Id: I46686aea9b68105cfe70a11db0e88052781e179c
Reviewed-on: https://boringssl-review.googlesource.com/22164
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
RC4 is dead and gone. This trims away the suiteNoDTLS flag.
Change-Id: I1ddc5d0811ad8cfb073e6e3c73100240bc649615
Reviewed-on: https://boringssl-review.googlesource.com/22469
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This avoids needing to keep track of which rules do and don't need it.
Change-Id: Id086b0622305f7f4acd3892f5d24d8e0c970febb
Reviewed-on: https://boringssl-review.googlesource.com/22468
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
RC4 is gone. The only remaining exception was the dumb SSL_eNULL cipher,
which works fine in DTLS. It doesn't seem worth the trouble to retain
this special-case.
Change-Id: I31023b71192808e4d21e82109255dc4d6d381df8
Reviewed-on: https://boringssl-review.googlesource.com/22467
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
We broke C++ file scanning. It also was silently failing on Windows
because os.Rename's error was ignored. Also make it work on Windows; we
just need to close the files early.
Change-Id: I1aa976ef67a1feaf574c41cf07d2202c245f027a
Reviewed-on: https://boringssl-review.googlesource.com/22466
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Due to a copy-paste error, the call to |left_shift_3| is missing after
reducing the password scalar in SPAKE2. This means that three bits of
the password leak in Alice's message. (Two in Bob's message as the point
N happens to have order 4l, not 8l.)
The “correct” fix is to put in the missing call to |left_shift_3|, but
that would be a breaking change. In order to fix this in a unilateral
way, we add points of small order to the masking point to bring it into
prime-order subgroup.
BUG=chromium:778101
Change-Id: I440931a3df7f009b324d2a3e3af2d893a101804f
Reviewed-on: https://boringssl-review.googlesource.com/22445
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This partially reverts commit 38636aba74.
Some build on Android seems to break now. I'm not really sure what the
situation is, but if the weird common symbols are still there (can we
remove them?), they probably ought to have the right flags.
Change-Id: Ief589d763d16b995ac6be536505acf7596a87b30
Reviewed-on: https://boringssl-review.googlesource.com/22404
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Those EXPECTs should be ASSERTs to ensure bn is not null.
Change-Id: Icb54c242ffbde5f8eaa67f19f214c9eef13705ea
Reviewed-on: https://boringssl-review.googlesource.com/22366
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
An embedded item wasn't allocated separately on the heap, so don't
free it as if it was.
Issue discovered by Pavel Kopyl
(Imported from upstream's cdc3307d4257f4fcebbab3b2b44207e1a399da05 and
65d414434aeecd5aa86a46adbfbcb59b4344503a.)
I do not believe this is actually reachable in BoringSSL, even in the
face of malloc errors. The only field which sets ASN1_TFLG_COMBINE is in
X509_ATTRIBUTE. That field's value is X509_ATTRIBUTE_SET which cannot
fail to initialize. (It is a CHOICE whose initialization consists of
setting the selector to -1 and calling the type's callback which is
unset for this type.)
Change-Id: I29c080f8a4ddc2f3ef9c119d0d90a899d3cb78c5
Reviewed-on: https://boringssl-review.googlesource.com/22365
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
1 << 31 is technically an undefined shift. It should be 1u << 31 to shut
UBSan up. I've also converted the others for consistency.
Change-Id: I1c6fe282f55c7032cea39f5ff1035a7711155f02
Reviewed-on: https://boringssl-review.googlesource.com/22344
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Currently we only check that the underlying EC_METHODs match, which
avoids the points being in different forms, but not that the points are
on the same curves. (We fixed the APIs early on so off-curve EC_POINTs
cannot be created.)
In particular, this comes up with folks implementating Java's crypto
APIs with ECDH_compute_key. These APIs are both unfortunate and should
not be mimicked, as they allow folks to mismatch the groups on the two
multiple EC_POINTs. Instead, ECDH APIs should take the public value as a
byte string.
Thanks also to Java's poor crypto APIs, we must support custom curves,
which makes this particularly gnarly. This CL makes EC_GROUP_cmp work
with custom curves and adds an additional subtle requirement to
EC_GROUP_set_generator.
Annoyingly, this change is additionally subtle because we now have a
reference cycle to hack around.
Change-Id: I2efbc4bd5cb65fee5f66527bd6ccad6b9d5120b9
Reviewed-on: https://boringssl-review.googlesource.com/22245
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
(Credit to libFuzzer for finding this.)
Change-Id: I0353d686d883703d39145c5bdd1e56368a587a35
Reviewed-on: https://boringssl-review.googlesource.com/22324
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Per the comment, it's no longer necessary. macOS i386 does not exist,
but apparently iOS i386 does! We can probably just remove it altogether,
but our cmake_minimum_required is nominally 2.8, so I just put the
version check in.
Bug: 210
Change-Id: I6e0617a3f292a218b2465eee85bd4814bd0e55c7
Reviewed-on: https://boringssl-review.googlesource.com/22304
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Also switch them to accepting a u16 length prefix. We appear not to have
any such tests right now, but RSA-2048 would involve modulus well larger
and primes just a hair larger than a u8 length prefix alows.
Change-Id: Icce8f1d976e159b945302fbba732e72913c7b724
Reviewed-on: https://boringssl-review.googlesource.com/22284
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
I really need to resurrect the CL to make them entirely static
(https://crbug.com/boringssl/20), but, in the meantime, to make
replacing the EC_METHOD pointer in EC_POINT with EC_GROUP not
*completely* insane, make them refcounted.
OpenSSL did not do this because their EC_GROUPs are mutable
(EC_GROUP_set_asn1_flag and EC_GROUP_set_point_conversion_form). Ours
are immutable but for the two-function dance around custom curves (more
of OpenSSL's habit of making their objects too complex), which is good
enough to refcount.
Change-Id: I3650993737a97da0ddcf0e5fb7a15876e724cadc
Reviewed-on: https://boringssl-review.googlesource.com/22244
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This reverts commit 75d43b5785. Chatting
with EKR, there is some reason to believe that doing this might cause
more middlebox issues. Since we're still in the middle of working
towards viable deployment in the first place, revert this.
We can experiment with this later. I should have arranged for this to be
controlled more carefully anyway.
Change-Id: I0c8bf578f9d7364e913894e1bf3c2b8123dfd770
Reviewed-on: https://boringssl-review.googlesource.com/22204
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Both of these changes have stuck in Chrome for quite a while now. Let's
clear them.
Change-Id: I13094451be2584ecaaf6b60eedefb7212b7bcde2
Reviewed-on: https://boringssl-review.googlesource.com/22226
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This is an OpenSSL thing to support platforms where BN_ULONG is not
actually the size it claims to be. We define BN_ULONG to uint32_t and
uint64_t which are guaranteed by C to implement arithemetic modulo 2^32
and 2^64, respectively. Thus there is no need for any of this.
Change-Id: I098cd4cc050a136b9f2c091dfbc28dd83e01f531
Reviewed-on: https://boringssl-review.googlesource.com/21784
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This reverts commit f6942f0d22.
Reason for revert: This doesn't actually work in clang-cl. I
forgot we didn't have the clang-cl try bots enabled! :-( I
believe __asm__ is still okay, but I'll try it by hand
tomorrow.
Original change's description:
> Use uint128_t and __asm__ in clang-cl.
>
> clang-cl does not define __GNUC__ but is still a functioning clang. We
> should be able to use our uint128_t and __asm__ code in it on Windows.
>
> Change-Id: I67310ee68baa0c0c947b2441c265b019ef12af7e
> Reviewed-on: https://boringssl-review.googlesource.com/22184
> Commit-Queue: Adam Langley <agl@google.com>
> Reviewed-by: Adam Langley <agl@google.com>
> CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
TBR=agl@google.com,davidben@google.com
Change-Id: I5c7e0391cd9c2e8cc0dfde37e174edaf5d17db22
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://boringssl-review.googlesource.com/22224
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
clang-cl does not define __GNUC__ but is still a functioning clang. We
should be able to use our uint128_t and __asm__ code in it on Windows.
Change-Id: I67310ee68baa0c0c947b2441c265b019ef12af7e
Reviewed-on: https://boringssl-review.googlesource.com/22184
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
We never implemented psk_ke, so there's no need to define the constant.
Change-Id: I6e52596e1a2cf0b3db5e7cd96db6836f4290bf0b
Reviewed-on: https://boringssl-review.googlesource.com/22144
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
There is also no need to make the struct public. Also tidy up includes a
bit.
Change-Id: I188848dfd8f9ed42925b2c55da8dc4751c29f146
Reviewed-on: https://boringssl-review.googlesource.com/22126
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Some of the complaints seem a bit questionable or their replacements
problematic, but not using strcat, strcpy, and strncpy is easy and
safer.
Change-Id: I64faf24b4f39d1ea410e883f026350094975a9b5
Reviewed-on: https://boringssl-review.googlesource.com/22125
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This doesn't matter in so far as runner is not a real TLS
implementation, but it should enforce what there is to enforce just to
keep BoringSSL honest.
Bug: 80
Change-Id: I68940c33712d34a2437dc4dee31342e7f0f57c23
Reviewed-on: https://boringssl-review.googlesource.com/22069
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This does not affect TLS 1.2 (beyond Channel ID or NPN) but, in TLS 1.3,
we send several encrypted handshake messages in a row. For the server,
this means 66 wasted bytes in TLS 1.3. Since OpenSSL has otherwise used
one record per message since the beginning and unencrypted overhead is
less interesting, leave that behavior as-is for the time being. (This
isn't the most pressing use of the breakage budget.) But TLS 1.3 is new,
so get this tight from the start.
Change-Id: I64dbd590a62469d296e1f10673c14bcd0c62919a
Reviewed-on: https://boringssl-review.googlesource.com/22068
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
We enforce that servers don't send bogus ALPN values, so consumers may
assume that SSL_get0_alpn_selected won't have anything terribly weird.
To maintain that invariant in the face of folks whose ALPN preferences
change (consider a persisted session cache), we should decline to offer
0-RTT if early_alpn would have been rejected by the check anyway.
Change-Id: Ic3a9ba4041d5d4618742eb05e27033525d96ade1
Reviewed-on: https://boringssl-review.googlesource.com/22067
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
This is in preparation for giving DTLS_STATE one.
Change-Id: I3dfeeaad2d20c547d8e65d739bd0ad5bc1acf74a
Reviewed-on: https://boringssl-review.googlesource.com/22065
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
new_*_len can just be computed rather than maintained as state.
Change-Id: If097ee9e68d8791fcfeb69052151faf0134c7c52
Reviewed-on: https://boringssl-review.googlesource.com/21948
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This finally clears most of the SSL_clear special-cases.
Change-Id: I00fc240ccbf13f4290322845f585ca6f5786ad80
Reviewed-on: https://boringssl-review.googlesource.com/21947
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
I've left EVP_set_buggy_rsa_parser as a no-op stub for now, but it
shouldn't need to last very long. (Just waiting for a CL to land in a
consumer.)
Bug: chromium:735616
Change-Id: I6426588f84dd0803661a79c6636a0414f4e98855
Reviewed-on: https://boringssl-review.googlesource.com/22124
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
As with SSLTranscript before, we temporarily need some nastiness in
SSL3_STATE, but this is in preparation of giving SSL3_STATE a
constructor and destructor.
Change-Id: Ifc0ce34fdcd8691d521d8ea03ff5e83dad43b4a3
Reviewed-on: https://boringssl-review.googlesource.com/21944
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Our assembly does not use the GOT to reference symbols, which means
references to visible symbols will often require a TEXTREL. This is
undesirable, so all assembly-referenced symbols should be hidden. CPU
capabilities are the only such symbols defined in C.
These symbols may be hidden by doing at least one of:
1. Build with -fvisibility=hidden
2. __attribute__((visibility("hidden"))) in C.
3. .extern + .hidden in some assembly file referencing the symbol.
We have lots of consumers and can't always rely on (1) happening. We
were doing (3) by way of d216b71f90 and
16e38b2b8f, but missed 32-bit x86 because
it doesn't cause a linker error.
Those two patches are not in upstream. Upstream instead does (3) by way
of x86cpuid.pl and friends, but we have none of these files.
Standardize on doing (2). This avoids accidentally getting TEXTRELs on
some 32-bit x86 build configurations. This also undoes
d216b71f90 and
16e38b2b8f. They are no now longer needed
and reduce the upstream diff.
Change-Id: Ib51c43fce6a7d8292533635e5d85d3c197a93644
Reviewed-on: https://boringssl-review.googlesource.com/22064
Commit-Queue: Matt Braithwaite <mab@google.com>
Reviewed-by: Matt Braithwaite <mab@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
If you specify any --target which refers to a x86_64-apple-darwin
triple, or a more specific variant derived from it, specifying
-stdlib=libc++ is required, otherwise clang falls back to libstdc++
which didn't include c++11, and fails to compile in very obscure ways
(simply failing to find any c++11 symbols).
Change-Id: I58025cea91eaa0c16d9b5831f9965889b75bbc31
Reviewed-on: https://boringssl-review.googlesource.com/21984
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
