This is horrible, but everything else I tried was worse. The goal with
this CL is to take the extern "C" out of ssl/internal.h and move most
symbols to namespace bssl, so we can start using C++ helpers and
destructors without worry.
Complications:
- Public API functions must be extern "C" and match their declaration in
ssl.h, which is unnamespaced. C++ really does not want you to
interleave namespaced and unnamespaced things. One can actually write
a namespaced extern "C" function, but this means, from C++'s
perspective, the function is namespaced. Trying to namespace the
public header would worked but ended up too deep a rabbithole.
- Our STACK_OF macros do not work right in namespaces.
- The typedefs for our exposed but opaque types are visible in the
header files and copied into consuming projects as forward
declarations. We ultimately want to give SSL a destructor, but
clobbering an unnamespaced ssl_st::~ssl_st seems bad manners.
- MSVC complains about ambiguous names if one typedefs SSL to bssl::SSL.
This CL opts for:
- ssl/*.cc must begin with #define BORINGSSL_INTERNAL_CXX_TYPES. This
informs the public headers to create forward declarations which are
compatible with our namespaces.
- For now, C++-defined type FOO ends up at bssl::FOO with a typedef
outside. Later I imagine we'll rename many of them.
- Internal functions get namespace bssl, so we stop worrying about
stomping the tls1_prf symbol. Exported C functions are stuck as they
are. Rather than try anything weird, bite the bullet and reorder files
which have a mix of public and private functions. I expect that over
time, the public functions will become fairly small as we move logic
to more idiomatic C++.
Files without any public C functions can just be written normally.
- To avoid MSVC troubles, some bssl types are renamed to CPlusPlusStyle
in advance of them being made idiomatic C++.
Bug: 132
Change-Id: Ic931895e117c38b14ff8d6e5a273e868796c7581
Reviewed-on: https://boringssl-review.googlesource.com/18124
Reviewed-by: David Benjamin <davidben@google.com>
This is needed to switch Chromium's SSLServerSocket and parts of
Conscrypt to CRYPTO_BUFFER.
Bug: 54
Change-Id: Iacd417970607bc1a162057676b576956a3bdfa3f
Reviewed-on: https://boringssl-review.googlesource.com/17965
Reviewed-by: Adam Langley <agl@google.com>
This also serves as a certificate verification callback for
CRYPTO_BUFFER-based consumers. Remove the silly
SSL_CTX_i_promise_to_verify_certs_after_the_handshake placeholder.
Bug: 54, chromium:347402
Change-Id: I4c6b445cb9cd7204218acb2e5d1625e6f37aff6f
Reviewed-on: https://boringssl-review.googlesource.com/17964
Reviewed-by: David Benjamin <davidben@google.com>
This would also have fixed the Windows clang issues. Those kicked in
because Windows clang defines __clang__ and not __GNUC__, but
OPENSSL_UNUSED accounts for this. It's also shorter.
Change-Id: I75bc17bbb789c5b78a7a369c43194e146739f574
Reviewed-on: https://boringssl-review.googlesource.com/18004
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
This implements PR #1051
(https://github.com/tlswg/tls13-spec/pull/1051).
Local experiments were not able to replicate the claims in the PR, but
implement this anyway for comparison purposes.
Change-Id: Ic9baf5e671f9a44565020466a553dd08f5ec0f1b
Reviewed-on: https://boringssl-review.googlesource.com/17844
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
The EC_POINT munging is sufficiently heavy on the goto err that I went
ahead and tidied it up.
Bug: 132
Change-Id: I7a3b3b3f166e39e4559acec834dd8e1ea9ac8620
Reviewed-on: https://boringssl-review.googlesource.com/17747
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>
http://i1.kym-cdn.com/photos/images/original/000/242/631/382.gif
In the first step, switch C files to C++ individually, keeping
everything in internal.h C-compatible. We'll make minimal changes needed
to get things compiling (notably a lot of goto errs will need to turn to
bssl::UniquePtr right away), but more aggressive changes will happen in
later steps.
(To avoid a rebase, I'm intentionally avoiding files that would conflict
with CLs in flight right now.)
Bug: 132
Change-Id: Id4cfd722e7b57d1df11f27236b4658b5d39b5fd2
Reviewed-on: https://boringssl-review.googlesource.com/17667
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>
As of 958346a5e7942093ce629972adba95f64d2a51ee, the callback is called
multiple times.
Change-Id: I40dafeb9f14de7d016644313ef137a0c85f0a24d
Reviewed-on: https://boringssl-review.googlesource.com/17725
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
Change-Id: I3de3c48a1de59c2b8de348253ce62a648aa6d6eb
Reviewed-on: https://boringssl-review.googlesource.com/17724
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>
TLS 1.3 deployment is currently blocked by buggy middleboxes
throughout the ecosystem. As an experiment to better understand these bugs
and the problems they are causing, implement TLS 1.3 variants with
alternate encodings. These are still the same protocol, only encoded
slightly differently. We will use what we learn from these experiments to
guide the TLS 1.3 deployment strategy and proposals to the IETF, if any.
These experiments only target the basic 1-RTT TLS 1.3 handshake. Based on
what we learn from this experiment, we may try future variations to
explore 0-RTT and HelloRetryRequest.
When enabled, the server supports all TLS 1.3 variants while the client
is configured to use a particular variant.
Change-Id: I532411d1abc41314dc76acce0246879b754b4c61
Reviewed-on: https://boringssl-review.googlesource.com/17327
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>
This is a bit verbose, but this API is goofy and causes a lot of
confusion. This may be clearer.
Change-Id: I9affff99b838958058e56ee3062521421c9accc5
Reviewed-on: https://boringssl-review.googlesource.com/17645
Reviewed-by: David Benjamin <davidben@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
This has come up a few times and our docs aren't great. This hopefully
describes the sharp edges better.
Change-Id: I5d4044449f74ec116838fd1bba629cd90dc0d1ac
Reviewed-on: https://boringssl-review.googlesource.com/17504
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: I819a5b565e4380f3d816a2e4a68572935c612eae
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Reviewed-on: https://boringssl-review.googlesource.com/17564
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>
WatchGuard's bug is very distinctive. Report a dedicated error code out
of BoringSSL so we can better track this.
Bug: chromium:733223
Change-Id: Ia42abd8654e7987b1d43c63a4f454f35f6aa873b
Reviewed-on: https://boringssl-review.googlesource.com/17328
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>
Consumers should now all be using a pattern that allows us to remove
unset fields from the struct.
Change-Id: Ia3cf4941589d624cf25c5173501bedeab73fb7b8
Reviewed-on: https://boringssl-review.googlesource.com/17326
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>
Both Conscrypt and Netty have a lot of logic to map between the two
kinds of names. WebRTC needed an SSL_CIPHER_get_rfc_name for something.
Just have both in the library. Also deprecate SSL_CIPHER_get_rfc_name
in favor of SSL_CIPHER_standard_name, which matches upstream if built
with enable-ssl-trace. And, unlike SSL_CIPHER_get_rfc_name, this does
not require dealing with the malloc.
(Strangely this decreases bssl's binary size, even though we're carrying
more strings around. It seems the old SSL_CIPHER_get_rfc_name was
somewhat large in comparison. Regardless, a consumer that disliked 30
short strings probably also disliked the OpenSSL names. That would be
better solved by opaquifying SSL_CIPHER and adding a less stringy API
for configuring cipher lists. That's something we can explore later if
needed.)
I also made the command-line tool print out the standard names since
they're more standard. May as well push folks towards those going
forward.
Change-Id: Ieeb3d63e67ef4da87458e68d130166a4c1090596
Reviewed-on: https://boringssl-review.googlesource.com/17324
Reviewed-by: Robert Sloan <varomodt@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
These are not the true version filters due to SSL_OP_NO_* filters.
Change-Id: I4c2db967d885f7c1875a3e052c5b02ea8d612fe1
Reviewed-on: https://boringssl-review.googlesource.com/17266
Reviewed-by: Steven Valdez <svaldez@google.com>
The original motivation behind the sign/complete split was to avoid
needlessly hashing the input on each pass through the state machine, but
we're payload-based now and, in all cases, the payload is either cheap
to compute or readily available. (Even the hashing worry was probably
unnecessary.)
Tweak ssl_private_key_{sign,decrypt} to automatically call
ssl_private_key_complete as needed and take advantage of this in the
handshake state machines:
- TLS 1.3 signing now computes the payload each pass. The payload is
small and we're already allocating a comparable-sized buffer each
iteration to hold the signature. This shouldn't be a big deal.
- TLS 1.2 decryption code still needs two states due to reading the
message (fixed in new state machine style), but otherwise it just
performs cheap idempotent tasks again. The PSK code is reshuffled to
guarantee the callback is not called twice (though this was impossible
anyway because we don't support RSA_PSK).
- TLS 1.2 CertificateVerify signing is easy as the transcript is readily
available. The buffer is released very slightly later, but it
shouldn't matter.
- TLS 1.2 ServerKeyExchange signing required some reshuffling.
Assembling the ServerKeyExchange parameters is moved to the previous
state. The signing payload has some randoms prepended. This is cheap
enough, but a nuisance in C. Pre-prepend the randoms in
hs->server_params.
With this change, we are *nearly* rid of the A/B => same function
pattern.
BUG=128
Change-Id: Iec4fe0be7cfc88a6de027ba2760fae70794ea810
Reviewed-on: https://boringssl-review.googlesource.com/17265
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
Change-Id: Ic2eae037d50de4af67f6cbe888e16d507ab674d8
Reviewed-on: https://boringssl-review.googlesource.com/17224
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
It does not appear removing support for these is feasible right now. :-(
Change-Id: I99521ba6c141855b5140d98bce445d7e62415661
Reviewed-on: https://boringssl-review.googlesource.com/17251
Reviewed-by: David Benjamin <davidben@google.com>
We've got an asynchronous ServerKeyExchange state in the middle that
complicates things a bit, but this is still a little tighter.
BUG=128
Change-Id: I4ee2e3b85e677c9555d2fbddd387c12d41ab2b54
Reviewed-on: https://boringssl-review.googlesource.com/17250
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 can take advantage of our flight-by-flight model.
BUG=128
Change-Id: If27a5b6d88055da71199ef672d9c71969925aca9
Reviewed-on: https://boringssl-review.googlesource.com/17249
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Free code coverage. Also rename things in SSL_select_next_proto so it
works for NPN and ALPN. (I found some code which uses it for ALPN.)
Change-Id: I8d06b768f9484dc3eda1a20506ec84ec3ddbc883
Reviewed-on: https://boringssl-review.googlesource.com/17206
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>
BUG=76
Change-Id: If58a73da38e46549fd55f84a9104e2dfebfda43f
Reviewed-on: https://boringssl-review.googlesource.com/14164
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: 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 imports upstream's scrypt implementation, though it's been heavily
revised. I lost track of words vs. blocks vs. bigger blocks too many
times in the original code and introduced a typedef for the fixed-width
Salsa20 blocks. The downside is going from bytes to blocks is a bit
trickier, so I took advantage of our little-endian assumption.
This also adds an missing check for N < 2^32. Upstream's code is making
this assumption in Integerify. I'll send that change back upstream. I've
also removed the weird edge case where a NULL out_key parameter means to
validate N/r/p against max_mem and nothing else. That's just in there to
get a different error code out of their PKCS#12 code.
Performance-wise, the cleanup appears to be the same (up to what little
precision I was able to get here), but an optimization to use bitwise
AND rather than modulus makes us measurably faster. Though scrypt isn't
a fast operation to begin with, so hopefully it isn't anyone's
bottleneck.
This CL does not route scrypt up to the PKCS#12 code, though we could
write our own version of that if we need to later.
BUG=chromium:731993
Change-Id: Ib2f43344017ed37b6bafd85a2c2b103d695020b8
Reviewed-on: https://boringssl-review.googlesource.com/17084
Reviewed-by: Adam Langley <agl@google.com>
Rather than adding a new mode to EVP_PKEY_CTX, upstream chose to tie
single-shot signing to EVP_MD_CTX, adding functions which combine
EVP_Digest*Update and EVP_Digest*Final. This adds a weird vestigial
EVP_MD_CTX and makes the signing digest parameter non-uniform, slightly
complicating things. But it means APIs like X509_sign_ctx can work
without modification.
Align with upstream's APIs. This required a bit of fiddling around
evp_test.cc. For consistency and to avoid baking details of parameter
input order, I made it eagerly read all inputs before calling
SetupContext. Otherwise which attributes are present depend a lot on the
shape of the API we use---notably the NO_DEFAULT_DIGEST tests for RSA
switch to failing before consuming an input, which is odd.
(This only matters because we have some tests which expect the operation
to abort the operation early with parameter errors and match against
Error. Those probably should not use FileTest to begin with, but I'll
tease that apart a later time.)
Upstream also named NID_Ed25519 as NID_ED25519, even though the
algorithm is normally stylized as "Ed25519". Switch it to match.
Change-Id: Id6c8f5715930038e754de50338924d044e908045
Reviewed-on: https://boringssl-review.googlesource.com/17044
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>
These behave like EVP_AEAD_CTX_{seal,open} respectively, but receive
ciphertext and authentication tag as separate arguments, rather than one
contiguous out or in buffer.
Change-Id: Ia4f1b83424bc7067c55dd9e5a68f18061dab4d07
Reviewed-on: https://boringssl-review.googlesource.com/16924
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
These are never referenced within the library or externally. Some of the
constants have been unused since SSLeay.
Change-Id: I597511208dab1ab3816e5f730fcadaea9a733dff
Reviewed-on: https://boringssl-review.googlesource.com/17025
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
None of these declarations are ever defined or constants used.
Change-Id: Id71ed5f02f9972d375845eacd9ce290a64b1c525
Reviewed-on: https://boringssl-review.googlesource.com/17024
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>
Originally we had some confusion around whether the features could be
toggled individually or not. Per the ARM C Language Extensions doc[1],
__ARM_FEATURE_CRYPTO implies the "crypto extension" which encompasses
all of them. The runtime CPUID equivalent can report the features
individually, but it seems no one separates them in practice, for now.
(If they ever do, probably there'll be a new set of #defines.)
[1] http://infocenter.arm.com/help/topic/com.arm.doc.ihi0053c/IHI0053C_acle_2_0.pdf
Change-Id: I12915dfc308f58fb005286db75e50d8328eeb3ea
Reviewed-on: https://boringssl-review.googlesource.com/16991
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Most importantly, this version of delocate works for ppc64le. It should
also work for x86-64, but will need significant testing to make sure
that it covers all the cases that the previous delocate.go covered.
It's less stringtastic than the old code, however the parser isn't as
nice as I would have liked. I thought that the reason we put up with
AT&T syntax with Intel is so that assembly syntax could be somewhat
consistent across platforms. At least for ppc64le, that does not appear
to be the case.
Change-Id: Ic7e3c6acc3803d19f2c3ff5620c5e39703d74212
Reviewed-on: https://boringssl-review.googlesource.com/16464
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Windows Clang needs this in the stack case too, but it doesn't define
__GNUC__ since it's emulating MSVC.
Change-Id: I646550ca95240e80822adddc2b53c3b58c2ec4a6
Reviewed-on: https://boringssl-review.googlesource.com/16644
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>
The only place it is used is EC_KEY_{dup,copy} and no one calls that
function on an EC_KEY with ex_data. This aligns with functions like
RSAPublicKey_dup which do not copy ex_data. The logic is also somewhat
subtle in the face of malloc errors (upstream's PR 3323).
In fact, we'd even changed the function pointer signature from upstream,
so BoringSSL-only code is needed to pass this pointer in anyway. (I
haven't switched it to CRYPTO_EX_unused because there are some callers
which pass in an implementation anyway.)
Note, in upstream, the dup hook is also used for SSL_SESSIONs when those
are duplicated (for TLS 1.2 ticket renewal or TLS 1.3 resumption). Our
interpretation is that callers should treat those SSL_SESSIONs
equivalently to newly-established ones. This avoids every consumer
providing a dup hook and simplifies the interface.
(I've gone ahead and removed the TODO(fork). I don't think we'll be able
to change this API. Maybe introduce a new one, but it may not be worth
it? Then again, this API is atrocious... I've never seen anyone use argl
and argp even.)
BUG=21
Change-Id: I6c9e9d5a02347cb229d4c084c1e85125bd741d2b
Reviewed-on: https://boringssl-review.googlesource.com/16344
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Instead of a script which generates macros, emit static inlines in
individual header (or C files). This solves a few issues with the
original setup:
- The documentation was off. We match the documentation now.
- The stack macros did not check constness; see some of the fixes in
crypto/x509.
- Type errors did not look like usual type errors.
- Any type which participated in STACK_OF had to be made partially
public. This allows stack types to be defined an internal header or
even an individual file.
- One could not pass sk_FOO_free into something which expects a function
pointer.
Thanks to upstream's 411abf2dd3 for the
idea.
Change-Id: Ie5431390ccad761c17596b0e93941b0d7a68f904
Reviewed-on: https://boringssl-review.googlesource.com/16087
Reviewed-by: Adam Langley <agl@google.com>
We returned the wrong type, but with a typedef which made it void*. In
C++, void* to T* doesn't implicitly convert, so it doesn't quite work
right. Notably, Node passes it into sk_SSL_COMP_zero. The sk_* macros
only weakly typecheck right now, but a pending CL converts them to
proper functions.
Change-Id: I635d1e39e4f4f11b2b7bf350115a7f1b1be30e4f
Reviewed-on: https://boringssl-review.googlesource.com/16447
Reviewed-by: Adam Langley <agl@google.com>
Saves having it in several places.
Change-Id: I329e1bf4dd4a7f51396e36e2604280fcca32b58c
Reviewed-on: https://boringssl-review.googlesource.com/16026
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>
Change-Id: I7d8f9098038a82b29ab0eff8a3258975d8804a68
Reviewed-on: https://boringssl-review.googlesource.com/16264
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>
It's about time we got rid of this. As a first step, introduce a flag,
so that some consumers may stage this change in appropriately.
BUG=chromium:534766,chromium:532048
Change-Id: Id53f0bacf5bdbf85dd71d1262d9f3a9ce3c4111f
Reviewed-on: https://boringssl-review.googlesource.com/16104
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>
I think I've finally cleared this out. Everything should be using
upstream's longer 'proto' names now.
Change-Id: I6ab283dca845fdc184f3764223d027acba59ca91
Reviewed-on: https://boringssl-review.googlesource.com/16086
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>
EVP_AEAD_CTX is otherwise a pain to use from C++ when you need to keep
it around.
Change-Id: I1dff926b33a3246680be21b89b69dfb336d25cd5
Reviewed-on: https://boringssl-review.googlesource.com/15965
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
FIPS 186-4 wants d = e^-1 (mod lcm(p-1, q-1)), not (p-1)*(q-1).
Note this means the size of d might reveal information about p-1 and
q-1. However, we do operations with Chinese Remainder Theorem, so we
only use d (mod p-1) and d (mod q-1) as exponents. Using a minimal
totient does not affect those two values.
This removes RSA_recover_crt_params. Using a minimal d breaks (or rather
reveals an existing bug in) the function.
While I'm here, rename those ridiculous variable names.
Change-Id: Iaf623271d49cd664ba0eca24aa25a393f5666fac
Reviewed-on: https://boringssl-review.googlesource.com/15944
Commit-Queue: David Benjamin <davidben@google.com>
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>
Nothing is using them. For encrypt, there's generally no need to swap
out public key operations. keygen seems especially pointless as one
could just as easily call the other function directly.
The one behavior change is RSA_encrypt now gracefully detects if called
on an empty RSA, to match the other un-RSA_METHOD-ed functions which had
similar treatments. (Conscrypt was filling in the encrypt function
purely to provide a non-crashing no-op function. They leave the public
bits blank and pass their custom keys through sufficiently many layers
of Java crypto goo that it's not obvious whether this is reachable.)
We still can't take the function pointers out, but once
96bbe03dfd
trickles back into everything, we can finally prune RSA_METHOD.
Bump BORINGSSL_API_VERSION as a convenience so I can land the
corresponding removal in Conscrypt immediately.
Change-Id: Ia2ef4780a5dfcb869b224e1ff632daab8d378b2e
Reviewed-on: https://boringssl-review.googlesource.com/15864
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This allows us to implement RSA-PSS in the FIPS module without pulling
in EVP_PKEY. It also allows people to use RSA-PSS on an RSA*.
Empirically folks seem to use the low-level padding functions a lot,
which is unfortunate.
This allows us to remove a now redundant length check in p_rsa.c.
Change-Id: I5270e01c6999d462d378865db2b858103c335485
Reviewed-on: https://boringssl-review.googlesource.com/15825
Reviewed-by: Adam Langley <agl@google.com>
Thanks to Alex Gaynor for catching this.
Change-Id: I00e86f90a6ecb845393c0f4f9f8177a053645e70
Reviewed-on: https://boringssl-review.googlesource.com/15784
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>
David Benjamin
Steven Valdez
Piotr Sikora
Martin Kreichgauer
Adam Langley
Adam Langley
Alessandro Ghedini
