foo_init hooks are never implemented. Even upstream never uses them. The
flags member is also never used. We also don't expose paramgen, so
remove it.
Change-Id: I51d9439316c5163520ab7168693c457f33e59417
Reviewed-on: https://boringssl-review.googlesource.com/6846
Reviewed-by: Adam Langley <alangley@gmail.com>
node.js is, effectively, another bindings library. However, it's better
written than most and, with these changes, only a couple of tiny fixes
are needed in node.js. Some of these changes are a little depressing
however so we'll need to push node.js to use APIs where possible.
Changes:
∙ Support verify_recover. This is very obscure and the motivation
appears to be https://github.com/nodejs/node/issues/477 – where it's
not clear that anyone understands what it means :(
∙ Add a few, no-op #defines
∙ Add some members to |SSL_CTX| and |SSL| – node.js needs to not
reach into these structs in the future.
∙ Add EC_get_builtin_curves.
∙ Add EVP_[CIPHER|MD]_do_all_sorted – these functions are limited to
decrepit.
Change-Id: I9a3566054260d6c4db9d430beb7c46cc970a9d46
Reviewed-on: https://boringssl-review.googlesource.com/6952
Reviewed-by: Adam Langley <agl@google.com>
Since the error string logic was rewritten, this hasn't done anything.
Change-Id: Icb73dca65e852bb3c7d04c260d591906ec72c15f
Reviewed-on: https://boringssl-review.googlesource.com/6961
Reviewed-by: Adam Langley <agl@google.com>
(Comment-only change; no functional difference.)
Some code was broken by the |d2i_ECDSA_SIG| change in 87897a8c. It was
passing in a pointer to an existing |ECDSA_SIG| as the first argument
and then simply assuming that the structure would be updated in place.
The comments on the function suggested that this was reasonable.
This change updates the comments that use similar wording to either note
that the function will never update in-place, or else to note that
depending on that is a bad idea for the future.
I've also audited all the uses of these functions that I can find and,
in addition to the one case with |d2i_ECDSA_SIG|, there are several
users of |d2i_PrivateKey| that could become a problem in the future.
I'll try to fix them before it does become an issue.
Change-Id: I769f7b2e0b5308d09ea07dd447e02fc161795071
Reviewed-on: https://boringssl-review.googlesource.com/6902
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
A lot of commented-out code we haven't had to put them back, so these
can go now. Also remove the TODO about OAEP having a weird API. The API
is wrong, but upstream's shipped it with the wrong API, so that's what
it is now.
Change-Id: I7da607cf2d877cbede41ccdada31380f812f6dfa
Reviewed-on: https://boringssl-review.googlesource.com/6763
Reviewed-by: Adam Langley <agl@google.com>
|EC_GROUP_get0_order| doesn't require any heap allocations and never
fails, so it is much more convenient and more efficient for callers to
call.
Change-Id: Ic60f768875e7bc8e74362dacdb5cbbc6957b05a6
Reviewed-on: https://boringssl-review.googlesource.com/6532
Reviewed-by: Adam Langley <agl@google.com>
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
(Imported from upstream's c394a488942387246653833359a5c94b5832674e and test
data from 00456fded43eadd4bb94bf675ae4ea5d158a764f.)
Change-Id: Ic97059d42722fd810973ccb0c26c415c4eaae79a
Reviewed-on: https://boringssl-review.googlesource.com/6617
Reviewed-by: Adam Langley <agl@google.com>
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
(Imported from upstream's cc598f321fbac9c04da5766243ed55d55948637d, with test
from our original report. Verified ASan trips up on the test without the fix.)
Change-Id: I007d93f172b2f16bf6845d685d72717ed840276c
Reviewed-on: https://boringssl-review.googlesource.com/6615
Reviewed-by: Adam Langley <agl@google.com>
Right now your options are:
- Bounce on a reference and deal with cleanup needlessly.
- Manually check the type tag and peek into the union.
We probably have no hope of opaquifying this struct, but for new code, let's
recommend using this function rather than the more error-prone thing.
Change-Id: I9b39ff95fe4264a3f7d1e0d2894db337aa968f6c
Reviewed-on: https://boringssl-review.googlesource.com/6551
Reviewed-by: Adam Langley <agl@google.com>
There's a few things that will be kind of a nuisance and possibly not worth it
(crypto/asn1 dumps a lot of undeclared things, etc.). But it caught some
mistakes. Even without the warning, making sure to include the externs before
defining a function helps catch type mismatches.
Change-Id: I3dab282aaba6023e7cebc94ed7a767a5d7446b08
Reviewed-on: https://boringssl-review.googlesource.com/6484
Reviewed-by: Adam Langley <agl@google.com>
Chromium's toolchains may now assume C++11 library support, so we may freely
use C++11 features. (Chromium's still in the process of deciding what to allow,
but we use Google's style guide directly, toolchain limitations aside.)
Change-Id: I1c7feb92b7f5f51d9091a4c686649fb574ac138d
Reviewed-on: https://boringssl-review.googlesource.com/6465
Reviewed-by: Adam Langley <agl@google.com>
Android is now using Ninja so it doesn't spew so much to the terminal
and thus any warnings in BoringSSL (which builds really early in the
process) and much more obvious.
Thus this change fixes a few warnings that appear in the Android build.
Change-Id: Id255ace90fece772a1c3a718c877559ce920b960
Reviewed-on: https://boringssl-review.googlesource.com/6400
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
This removes a hard link-time dependency on the SHA-1 code. The code
was self-contradictory in whether it defaulted to SHA-1 or refused to
default to SHA-1.
Change-Id: I5ad7949bdd529df568904f87870313e3d8a57e72
Reviewed-on: https://boringssl-review.googlesource.com/5833
Reviewed-by: Adam Langley <alangley@gmail.com>
It's very annoying having to remember the right incant every time I want
to switch around between my build, build-release, build-asan, etc.,
output directories.
Unfortunately, this target is pretty unfriendly without CMake 3.2+ (and
Ninja 1.5+). This combination gives a USES_TERMINAL flag to
add_custom_target which uses Ninja's "console" pool, otherwise the
output buffering gets in the way. Ubuntu LTS is still on an older CMake,
so do a version check in the meantime.
CMake also has its own test mechanism (CTest), but this doesn't use it.
It seems to prefer knowing what all the tests are and then tries to do
its own output management and parallelizing and such. We already have
our own runners. all_tests.go could actually be converted tidily, but
generate_build_files.py also needs to read it, and runner.go has very
specific needs.
Naming the target ninja -C build test would be nice, but CTest squats
that name and CMake grumps when you use a reserved name, so I've gone
with run_tests.
Change-Id: Ibd20ebd50febe1b4e91bb19921f3bbbd9fbcf66c
Reviewed-on: https://boringssl-review.googlesource.com/6270
Reviewed-by: Adam Langley <alangley@gmail.com>
Although the previous commit should ensure this doesn't happen, the
uint8_t** pattern is very error-prone and we're trying to avoid doing
much to the legacy ASN.1 stack. To that end, maintaining the strong
exception guarantee w.r.t. the input pointer-pointer is best effort and
we won't rely on it, so we needn't spend our time chasing down problems.
Change-Id: Ib78974eb94377fe0b0b379f57d9695dc81f344bb
Reviewed-on: https://boringssl-review.googlesource.com/5949
Reviewed-by: Adam Langley <agl@google.com>
(Imported from upstream's 728bcd59d3d41e152aead0d15acc51a8958536d3.)
Actually this one was reported by us, but the commit message doesn't
mention this.
This is slightly modified from upstream's version to fix some problems
noticed in import. Specifically one of d2i_X509_AUX's success paths is
bust and d2i_PrivateKey still updates on one error path. Resolve the
latter by changing both it and d2i_AutoPrivateKey to explicitly hit the
error path on ret == NULL. This lets us remove the NULL check in
d2i_AutoPrivateKey.
We'll want to report the problems back upstream.
Change-Id: Ifcfc965ca6d5ec0a08ac154854bd351cafbaba25
Reviewed-on: https://boringssl-review.googlesource.com/5948
Reviewed-by: Adam Langley <agl@google.com>
Not content with signing negative RSA moduli, still other Estonian IDs have too
many leading zeros. Work around those too.
This workaround will be removed in six months.
BUG=534766
Change-Id: Ica23b1b1499f9dbe39e94cf7b540900860e8e135
Reviewed-on: https://boringssl-review.googlesource.com/5980
Reviewed-by: Adam Langley <agl@google.com>
Target date for removal of the workaround is 6 months.
BUG=532048
Change-Id: I402f75e46736936725575559cd8eb194115ab0df
Reviewed-on: https://boringssl-review.googlesource.com/5910
Reviewed-by: Adam Langley <agl@google.com>
The documentation for |ECDSA_sign| and |ECDSA_verify| says that the
|type| parameter should be zero.
Change-Id: I875d3405455c5443f5a5a5c2960a9a9f486ca5bb
Reviewed-on: https://boringssl-review.googlesource.com/5832
Reviewed-by: Adam Langley <agl@google.com>
Estonian IDs issued between September 2014 to September 2015 are broken and use
negative moduli. They last five years and are common enough that we need to
work around this bug.
Add parallel "buggy" versions of BN_cbs2unsigned and RSA_parse_public_key which
tolerate this mistake, to align with OpenSSL's previous behavior. This code is
currently hooked up to rsa_pub_decode in RSA_ASN1_METHOD so that d2i_X509 is
tolerant. (This isn't a huge deal as the rest of that stack still uses the
legacy ASN.1 code which is overly lenient in many other ways.)
In future, when Chromium isn't using crypto/x509 and has more unified
certificate handling code, we can put client certificates under a slightly
different codepath, so this needn't hold for all certificates forever. Then in
September 2019, when the broken Estonian certificates all expire, we can purge
this codepath altogether.
BUG=532048
Change-Id: Iadb245048c71dba2eec45dd066c4a6e077140751
Reviewed-on: https://boringssl-review.googlesource.com/5894
Reviewed-by: Adam Langley <agl@google.com>
arm_arch.h is included from ARM asm files, but lives in crypto/, not
openssl/include/. Since the asm files are often built from a different
location than their position in the source tree, relative include paths
are unlikely to work so, rather than having crypto/ be a de-facto,
second global include path, this change moves arm_arch.h to
include/openssl/.
It also removes entries from many include paths because they should be
needed as relative includes are always based on the locations of the
source file.
Change-Id: I638ff43d641ca043a4fc06c0d901b11c6ff73542
Reviewed-on: https://boringssl-review.googlesource.com/5746
Reviewed-by: Adam Langley <agl@google.com>
This change makes |EVP_PKEY_asn1_find_str|, which is used by
|PEM_read_bio_PrivateKey|, recognize "DSA" as well as "EC" and "RSA".
Change-Id: I39cce12f600cec6a71df75312a41f8395429af62
Reviewed-on: https://boringssl-review.googlesource.com/5743
Reviewed-by: Adam Langley <agl@google.com>
Some compilers in some configurations warn about this structure member
not being assigned a value. Since it is never used anywhere, just
remove it.
Change-Id: I46064234961bf449fe5fcb88594ddb3ff390e7d7
Reviewed-on: https://boringssl-review.googlesource.com/5621
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
When using CMake to build with MSVC, MSVC complains about unreachable
code in the <xtree> header. This incantation silences that.
Change-Id: I5fc5305dc816a009a4c59501b212fd11e290637d
Reviewed-on: https://boringssl-review.googlesource.com/5552
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
(Imported from upstream's 374fd385c2347b965c3490aa1c10025e1339d265.)
This codepath is only reachable on malloc failure if putting DSA private
keys into a PKCS#8 PrivateKeyInfo.
Change-Id: I88052eab3f477c4cdf5749be525878278d966a69
Reviewed-on: https://boringssl-review.googlesource.com/5543
Reviewed-by: Adam Langley <agl@google.com>
Running make_errors.go every time a function is renamed is incredibly
tedious. Plus we keep getting them wrong.
Instead, sample __func__ (__FUNCTION__ in MSVC) in the OPENSSL_PUT_ERROR macro
and store it alongside file and line number. This doesn't change the format of
ERR_print_errors, however ERR_error_string_n now uses the placeholder
"OPENSSL_internal" rather than an actual function name since that only takes
the uint32_t packed error code as input.
This updates err scripts to not emit the function string table. The
OPENSSL_PUT_ERROR invocations, for now, still include the extra
parameter. That will be removed in a follow-up.
BUG=468039
Change-Id: Iaa2ef56991fb58892fa8a1283b3b8b995fbb308d
Reviewed-on: https://boringssl-review.googlesource.com/5275
Reviewed-by: Adam Langley <agl@google.com>
gyp doesn't allow two files to share the same name to avoid bugs in OS X
libtool, so asn1.c's need to all get a prefix.
Change-Id: I3593597912c49dd02655cae329fb253ed4f6f56d
Reviewed-on: https://boringssl-review.googlesource.com/5431
Reviewed-by: Adam Langley <agl@google.com>
This removes the version field from RSA and instead handles versioning
as part of parsing. (As a bonus, we now correctly limit multi-prime RSA
to version 1 keys.)
Most consumers are also converted. old_rsa_priv_{de,en}code are left
alone for now. Those hooks are passed in parameters which match the old
d2i/i2d pattern (they're only used in d2i_PrivateKey and
i2d_PrivateKey).
Include a test which, among other things, checks that public keys being
serialized as private keys are handled properly.
BUG=499653
Change-Id: Icdd5f0382c4a84f9c8867024f29756e1a306ba08
Reviewed-on: https://boringssl-review.googlesource.com/5273
Reviewed-by: Adam Langley <agl@google.com>
It carefully NULLs all references to the buffer, so the failure cases
still need to call OPENSSL_free.
Change-Id: Ia14341ecea81296f94e467325ab6eff1362e987e
Reviewed-on: https://boringssl-review.googlesource.com/5271
Reviewed-by: Adam Langley <agl@google.com>
This is the first structure to be implemented with the new BIGNUM ASN.1
routines. Object reuse in the legacy d2i/i2d functions is implemented by
releasing whatever was in *out before and setting it to the
newly-allocated object. As with the new d2i_SSL_SESSION, this is a
weaker form of object reuse, but should suffice for reasonable callers.
As ECDSA_SIG is more likely to be parsed alone than as part of another
structure (and using CBB is slightly tedious), add convenient functions
which take byte arrays. For consistency with SSL_SESSION, they are named
to/from_bytes. from_bytes, unlike the CBS variant, rejects trailing
data.
Note this changes some test expectations: BER signatures now push an
error code. That they didn't do this was probably a mistake.
BUG=499653
Change-Id: I9ec74db53e70d9a989412cc9e2b599be0454caec
Reviewed-on: https://boringssl-review.googlesource.com/5269
Reviewed-by: Adam Langley <agl@google.com>
All callers have been moved to EVP_PKEY_up_ref. (Neither spelling exists
upstream so we only had our own callers to move.)
Change-Id: I267f14054780fe3d6dc1170b7b6ae3811a0d1a9a
Reviewed-on: https://boringssl-review.googlesource.com/5291
Reviewed-by: Adam Langley <agl@google.com>
This removes EVP_PKEY_HMAC and all the support code around it. EVP_MD requires
a lot of extra glue to support HMAC. This lets us prune it all away.
As a bonus, it removes a (minor) dependency from EVP to the legacy ASN.1 stack.
Change-Id: I5a9e3e39f518429828dbf13d14647fb37d9dc35a
Reviewed-on: https://boringssl-review.googlesource.com/5120
Reviewed-by: Adam Langley <agl@google.com>
The only flag is EVP_MD_CTX_FLAG_NO_INIT and no good can possibly come of
anyone outside EVP_PKEY_HMAC calling it. (And indeed no one calls it.
EVP_MD_CTX_set_flags has a caller in wpa_supplicant, but it uses
EVP_MD_CTX_FLAG_NON_FIPS_ALLOW which we don't define. The call is guarded by a
pair of ifdefs for some FIPS mode wpa_supplicant.)
Change-Id: I70ab8ffa646f3f75dfa4d37c96b9e82448ff1e40
Reviewed-on: https://boringssl-review.googlesource.com/4971
Reviewed-by: Adam Langley <agl@google.com>
It's never called externally and for good reason; the only flag to set is
EVP_MD_CTX_FLAG_NO_INIT which is an implementation detail of EVP_PKEY_HMAC
(hopefully to be removed eventually). Indeed, only EVP_PKEY_HMAC ever calls
this function. Except there's no need to because the HMAC_CTX has already been
initialized at that point. (And were it not initialized, that call would not
bode well for the poor HMAC_CTX.)
The legacy EVP_PKEY_HMAC API has test coverage and still works after this
change.
Change-Id: I2fb0bede3c24ad1519f9433f957606de15ba86c7
Reviewed-on: https://boringssl-review.googlesource.com/4970
Reviewed-by: Adam Langley <agl@google.com>
Enough code fails to check their return codes anyway. We ought to make
it official.
Change-Id: Ie646360fd7073ea943036f5e21bed13df7e1b77a
Reviewed-on: https://boringssl-review.googlesource.com/4954
Reviewed-by: Adam Langley <agl@google.com>
Currently far from passing and I haven't even tried with a leak checker yet.
Also bn_test is slow.
Change-Id: I4fe2783aa5f7897839ca846062ae7e4a367d2469
Reviewed-on: https://boringssl-review.googlesource.com/4794
Reviewed-by: Adam Langley <agl@google.com>
This change converts the reference counts in crypto/ to use
|CRYPTO_refcount_t|. The reference counts in |X509_PKEY| and |X509_INFO|
were never actually used and so were dropped.
Change-Id: I75d572cdac1f8c1083c482e29c9519282d7fd16c
Reviewed-on: https://boringssl-review.googlesource.com/4772
Reviewed-by: Adam Langley <agl@google.com>
This imports the EVP_PKEY test data of upstream's evptests.txt, but
modified to fit our test framework and with a new test driver. The
remainder of the test data will be imported separately into aead_test
and cipher_test.
Some minor changes to the test format were made to account for test
framework differences. One test has different results since we don't
support RSA signatures with omitted (rather than NULL) parameters.
Otherwise, the biggest difference in test format is that the ad-hoc
result strings are replaced with checking ERR_peek_error.
Change-Id: I758869abbeb843f5f2ac6c1cbd87333baec08ec3
Reviewed-on: https://boringssl-review.googlesource.com/4703
Reviewed-by: Adam Langley <agl@google.com>
This matches how upstream imported that test. evp_test will be used for
the subset of upstream's evp_test which land in our crypto/evp layer.
(Some of crypto/evp is in crypto/cipher for us, so those tests will be
in a ported cipher_test.)
Change-Id: Ic899442794b66350e73a706bb7c77a6ff3d2564b
Reviewed-on: https://boringssl-review.googlesource.com/4702
Reviewed-by: Adam Langley <agl@google.com>
Change |EC_KEY_new_by_curve_name| to report |ERR_R_MALLOC_FAILURE|
itself, so that reporting of |EC_R_UNKNOWN_GROUP| is not confused by
the caller's addition of a spurious |ERR_R_MALLOC_FAILURE|.
Change-Id: Id3f5364f01eb8e3597bcddd6484bc03d5578befb
Reviewed-on: https://boringssl-review.googlesource.com/4690
Reviewed-by: Adam Langley <agl@google.com>
Or rather fix in so far as that call will always fail now, rather than
mix up EC and DH EVP_PKEY. We don't implement EVP_PKEY_DH.
Change-Id: I752978f3440b59d963b5c13f2349284d7d799182
Reviewed-on: https://boringssl-review.googlesource.com/4567
Reviewed-by: Adam Langley <agl@google.com>
This avoids callers having to worry about |CRYPTO_add| and what the
correct lock to use it with is. (Esp since we'll probably change the way
that reference counts work in the future.)
Change-Id: I972bf0cc3be6099e0255e64a0fd50249062d1eb4
Reviewed-on: https://boringssl-review.googlesource.com/4623
Reviewed-by: Adam Langley <agl@google.com>
Beyond generally eliminating unnecessary includes, eliminate as many
includes of headers that declare/define particularly error-prone
functionality like strlen, malloc, and free. crypto/err/internal.h was
added to remove the dependency on openssl/thread.h from the public
openssl/err.h header. The include of <stdlib.h> in openssl/mem.h was
retained since it defines OPENSSL_malloc and friends as macros around
the stdlib.h functions. The public x509.h, x509v3.h, and ssl.h headers
were not changed in order to minimize breakage of source compatibility
with external code.
Change-Id: I0d264b73ad0a720587774430b2ab8f8275960329
Reviewed-on: https://boringssl-review.googlesource.com/4220
Reviewed-by: Adam Langley <agl@google.com>
C4701 is "potentially uninitialized local variable 'buf' used". It
sometimes results in false positives, which can now be suppressed
using the macro OPENSSL_SUPPRESS_POTENTIALLY_UNINITIALIZED_WARNINGS.
Change-Id: I15068b5a48e1c704702e7752982b9ead855e7633
Reviewed-on: https://boringssl-review.googlesource.com/3160
Reviewed-by: Adam Langley <agl@google.com>
A previous change in BoringSSL renamed ERR_print_errors_fp to
BIO_print_errors_fp as part of refactoring the code to improve the
layering of modules within BoringSSL. Rename it back for better
compatibility with code that was using the function under the original
name. Move its definition back to crypto/err using an implementation
that avoids depending on crypto/bio.
Change-Id: Iee7703bb1eb4a3d640aff6485712bea71d7c1052
Reviewed-on: https://boringssl-review.googlesource.com/4310
Reviewed-by: Adam Langley <agl@google.com>
Sadly, it turns out that we have need of this, at least for now. The
code is taken from upstream and changed only as much as needed.
This only imports keys and doesn't know how to actually perform
operations on them for now.
Change-Id: I0db70fb938186cb7a91d03f068b386c59ed90b84
We don't really gain much from this one, but consistency.
Change-Id: I3f830c6d1ad65263bd1cc09372a5b810a8f690c0
Reviewed-on: https://boringssl-review.googlesource.com/4124
Reviewed-by: Adam Langley <agl@google.com>
May as well use this convenience function when we can. A little tidier. Even
fixes a leak on malloc failure in eckey_type2param.
Change-Id: Ie48dd98f2fe03fa9911bd78db4423ab9faefc63d
Reviewed-on: https://boringssl-review.googlesource.com/3772
Reviewed-by: Adam Langley <agl@google.com>
Now that much of EVP has been normalized to 0/1, a lot of code can just use
boolean operators. (As can some code which was already using them...)
Change-Id: I6bb17edfd6f67050bf1706d59d8f37df57535faa
Reviewed-on: https://boringssl-review.googlesource.com/3875
Reviewed-by: Adam Langley <agl@google.com>
No code within BoringSSL or Google (grep for EVP_PKEY_CTX_(ctrl|get|set)) is
sensitive to the various failure cases. Normalize it all to 0/1 for simplicity.
This does carry a slight risk: any new ctrl hooks we import from upstream that,
like EVP_PKEY_CTX_get_rsa_oaep_md, return something other than success/failure
cannot be called directly via EVP_PKEY_CTX_ctrl. They instead need to
internally be routed through a struct like CBS and only called through the
wrappers. To that end, unexport EVP_PKEY_CTX_ctrl and require that callers use
the wrappers. No code in Google uses it directly and, if need be, switching to
the wrapper would be an incredibly upstreamable patch.
Change-Id: I3fd4e5a1a0f3d4d1c4122c52d4c74a5105b99cd5
Reviewed-on: https://boringssl-review.googlesource.com/3874
Reviewed-by: Adam Langley <agl@google.com>
This is the only EVP_PKEY ctrl hook which returns something other than a
boolean.
Change-Id: Ic226aef168abdf72e5d30e8264a559ed5039a055
Reviewed-on: https://boringssl-review.googlesource.com/3873
Reviewed-by: Adam Langley <agl@google.com>
This removes another place where we're internally sensitive to the
success/failure conditions.
Change-Id: I18fecf6457e841ba0afb718397b9b5fd3bbdfe4c
Reviewed-on: https://boringssl-review.googlesource.com/3872
Reviewed-by: Adam Langley <agl@google.com>
All EVP_PKEY types return 1 on that. (It can go away entirely when
EVP_PKEY_HMAC is gone.) This removes a place internally where we're sensitive
to the failure code.
Change-Id: Ic6cda2da9337ba7ef1c66a18e40c5dcc44fcf840
Reviewed-on: https://boringssl-review.googlesource.com/3871
Reviewed-by: Adam Langley <agl@google.com>
(I got this wrong when reading the OpenSSL code.)
Change-Id: Ib289ef41d0ab5a3157ad8b9454d2de96d1f86c22
Reviewed-on: https://boringssl-review.googlesource.com/3620
Reviewed-by: Adam Langley <agl@google.com>
It was a mistake to remove this in the first place.
Change-Id: Icd97b4db01e49151daa41dd892f9da573ddc2842
Reviewed-on: https://boringssl-review.googlesource.com/3541
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Previously, error strings were kept in arrays for each subdirectory and
err.c would iterate over them all and insert them at init time to a hash
table.
This means that, even if you have a shared library and lots of processes
using that, each process has ~30KB of private memory from building that
hash table.
This this change, all the error strings are built into a sorted list and
are thus static data. This means that processes can share the error
information and it actually saves binary space because of all the
pointer overhead in the old scheme. Also it saves the time taken
building the hash table at startup.
This removes support for externally-supplied error string data.
Change-Id: Ifca04f335c673a048e1a3e76ff2b69c7264635be
Including string.h in base.h causes any file that includes a BoringSSL
header to include string.h. Generally this wouldn't be a problem,
although string.h might slow down the compile if it wasn't otherwise
needed. However, it also causes problems for ipsec-tools in Android
because OpenSSL didn't have this behaviour.
This change removes string.h from base.h and, instead, adds it to each
.c file that requires it.
Change-Id: I5968e50b0e230fd3adf9b72dd2836e6f52d6fb37
Reviewed-on: https://boringssl-review.googlesource.com/3200
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Before it was possible to pass a NULL-terminated C-string to the PBKDF2
functions, and indicate the parameter was a C-string by passing a length
of -1.
This is not relied on anywhere in the BoringSSL code, and the API contract is
possible to misuse as it is not the common way of doing things.
(A problem would arise when passing in a large unsigned length that
subsequently gets interpreted as -1).
Change-Id: Ifbd31ff76e183fa74e9fa346908daf4bfb8fc3da
Reviewed-on: https://boringssl-review.googlesource.com/2953
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Removes a bit of unused code. This effectively reverts upstream's
25af7a5dbc05c7359d1d7f472d50d65a9d876b7e. It's new with OpenSSL 1.0.2 so
nothing can be using it yet. We can restore it with tests if we end up wanting
it later.
(Also I think it might be misnamed. The KDF seems to be defined in X9.63, not
X9.62.)
Change-Id: I482daf681e0cf5c3bbdc72c57793f91448deaee8
Reviewed-on: https://boringssl-review.googlesource.com/2846
Reviewed-by: Adam Langley <agl@google.com>
HMAC_CTX_copy's documentation is off. It actually follows the old copy
functions which call FOO_init on dest first. Notably this means that they leak
memory if dest is currently in use.
Add HMAC_CTX_copy_ex as an analog of EVP_MD_CTX_copy and deprecate
HMAC_CTX_copy. (EVP_CIPHER_CTX_copy, in contrast, was correct from the start.)
Change-Id: I48566c858663d3f659bd356200cf862e196576c9
Reviewed-on: https://boringssl-review.googlesource.com/2694
Reviewed-by: Adam Langley <agl@google.com>
This commit fixes a number of crashes caused by malloc failures. They
were found using the -malloc-test=0 option to runner.go which runs tests
many times, causing a different allocation call to fail in each case.
(This test only works on Linux and only looks for crashes caused by
allocation failures, not memory leaks or other errors.)
This is not the complete set of crashes! More can be found by collecting
core dumps from running with -malloc-test=0.
Change-Id: Ia61d19f51e373bccb7bc604642c51e043a74bd83
Reviewed-on: https://boringssl-review.googlesource.com/2320
Reviewed-by: Adam Langley <agl@google.com>
This is intended for TLS client auth with Windows CAPI- and CNG-backed keys
which implement sign over sign_raw and do not support all hash functions. Only
plumbed through RSA for now.
Change-Id: Ica42e7fb026840f817a169da9372dda226f7d6fd
Reviewed-on: https://boringssl-review.googlesource.com/2250
Reviewed-by: Adam Langley <agl@google.com>
This stopped being a sample program a bit ago.
Change-Id: I23301fd71a373f995847dcfd64346bd262811f39
Reviewed-on: https://boringssl-review.googlesource.com/2131
Reviewed-by: Adam Langley <agl@google.com>
This got lost in the initial commit. Add a test for d2i_AutoPrivateKey.
BUG=crbug.com/428671
Change-Id: Ib4f6114b03536edcfe3b1720a513f57f748e81d2
Reviewed-on: https://boringssl-review.googlesource.com/2130
Reviewed-by: Adam Langley <agl@google.com>
The only alias, EVP_PKEY_RSA2, is handled programmatically. ASN1_PKEY_ALIAS and
ASN1_PKEY_DYNAMIC are then unused and unexported and can be removed.
Change-Id: I990650636bac3b802c8b439257c67ce7a3f8bc70
Reviewed-on: https://boringssl-review.googlesource.com/2124
Reviewed-by: Adam Langley <agl@google.com>
Some archaeology: it was added in upstream's
ee1d9ec019a7584482bd95891404f1cad66a4a0a. This seems to come from upstream's
arrangement where an EVP_MD can specify both the signing algorithm and the
message digest. (Most of the usual hash algorithms were tied to RSA.)
The flag is set on EVP_MDs that should use the EVP_PKEY's method table in
EVP_Sign* rather than the one attached to the EVP_MD (there's also
required_pkey_type to filter on EVP_PKEY to prevent a mismatch). Without the
flag, the old codepath is hit where they're tied together.
Interestingly, EVP_md5 does not have the flag, but I suppose this is because no
one would sign ECDSA + MD5. EVP_DigestSign* also postdates this and doesn't use
the legacy mechanism anyway. Upstream also has, e.g., EVP_ecdsa(). Although
those too have since also gained the flag in
bce1af776247fee153223ea156228810779483ce.
Let's get rid of these TODOs. We don't have the old codepath. It's unclear if
upstream really does either at this point.
Note: EVP_PKEY_RSA_method in upstream is actually a macro that expands to three
fields, which is why it's so difficult to figure out what's going on with those
structs.
Change-Id: I1aea4d3f79f1eb1755063bb96c1c65276c6e3643
Reviewed-on: https://boringssl-review.googlesource.com/2122
Reviewed-by: Adam Langley <agl@google.com>
It was already almost there. Just a malloc failure away. now all the
EVP_Digest{Sign,Verify}* functions may be used without worrying about -1 return
values.
Change-Id: I96a9750b300010615979bd5f1522b1d241764665
Reviewed-on: https://boringssl-review.googlesource.com/2064
Reviewed-by: Adam Langley <agl@google.com>
The same library code applies for both the error and the function, so modules
cannot easily report errors from each other. Switch evp/algorithm.c's error
codes to the EVP library. Remove the original error codes so it's obvious some
changes are needed.
- X509_R_DIGEST_AND_KEY_TYPE_NOT_SUPPORTED -> EVP_R_DIGEST_AND_KEY_TYPE_NOT_SUPPORTED
ASN1_R_DIGEST_AND_KEY_TYPE_NOT_SUPPORTED -> EVP_R_DIGEST_AND_KEY_TYPE_NOT_SUPPORTED
(Actually, the X509 version of this error code doesn't exist in OpenSSL. It should
have been ASN1.)
- ASN1_R_UNKNOWN_SIGNATURE_ALGORITHM -> EVP_R_UNKNOWN_SIGNATURE_ALGORITHM
- ASN1_R_WRONG_PUBLIC_KEY_TYPE -> EVP_R_WRONG_PUBLIC_KEY_TYPE
- ASN1_R_UNKNOWN_MESSAGE_DIGEST_ALGORITHM -> EVP_R_UNKNOWN_MESSAGE_DIGEST_ALGORITHM
Change-Id: I05b1a05b465d800c85f7d63ca74588edf40847b9
Reviewed-on: https://boringssl-review.googlesource.com/1940
Reviewed-by: Adam Langley <agl@google.com>
Factor the AlgorithmIdentifier portions of ASN1_item_sign and ASN1_item_verify
out. This makes it possible to initialize a signature context from an
AlgorithmIdentifier without needing the data parsed into an ASN1_ITEM/void*
pair and reserialized.
Change-Id: Idc2e06b1310a3f801aa25de323d39d2b7a44ef50
Reviewed-on: https://boringssl-review.googlesource.com/1916
Reviewed-by: Adam Langley <agl@google.com>
We only ever use the EVP_PKEY case, not the EVP_PKEY_CTX one.
Change-Id: Ibead854f793663da0a9e474599507d9c3ff920cb
Reviewed-on: https://boringssl-review.googlesource.com/1915
Reviewed-by: Adam Langley <agl@google.com>
It should return 0 for failure, not -1; the call site was expecting 0 anyway.
Change-Id: I24ab5d3695b8ac438e40be1a4fd74ecd3b845f5a
Reviewed-on: https://boringssl-review.googlesource.com/1914
Reviewed-by: Adam Langley <agl@google.com>
Chromium does not like static initializers, and the CPU logic uses one to
initialize CPU bits. However, the crypto library lacks an explicit
initialization function, which could complicate (no compile-time errors)
porting existing code which uses crypto/, but not ssl/.
Add an explicit CRYPTO_library_init function, but make it a no-op by default.
It only does anything (and is required) if building with
BORINGSSL_NO_STATIC_INITIALIZER.
Change-Id: I6933bdc3447fb382b1f87c788e5b8142d6f3fe39
Reviewed-on: https://boringssl-review.googlesource.com/1770
Reviewed-by: Adam Langley <agl@google.com>
Thanks to Denis Denisov for running the analysis.
Change-Id: I80810261e013423e746fd8d8afefb3581cffccc0
Reviewed-on: https://boringssl-review.googlesource.com/1701
Reviewed-by: Adam Langley <agl@google.com>
Otherwise, in C, it becomes a K&R function declaration which doesn't actually
type-check the number of arguments.
Change-Id: I0731a9fefca46fb1c266bfb1c33d464cf451a22e
Reviewed-on: https://boringssl-review.googlesource.com/1582
Reviewed-by: Adam Langley <agl@google.com>
Reference counting should be internal to the type, otherwise callers need to
know which lock to use.
Change-Id: If4d805876a321ef6dece115c805e605584ff311e
Reviewed-on: https://boringssl-review.googlesource.com/1414
Reviewed-by: Adam Langley <agl@google.com>
Where possible, functions should return one for success and zero for
error. The use of additional negative values to indicate an error is,
itself, error prone.
This change fixes many EVP functions to remove the possibility of
negative return values. Existing code that is testing for <= 0 will
continue to function, although there is the possibility that some code
was differentiating between negative values (error) and zero (invalid
signature) for the verify functions and will now show the wrong error
message.
Change-Id: I982512596bb18a82df65861394dbd7487783bd3d
Reviewed-on: https://boringssl-review.googlesource.com/1333
Reviewed-by: Adam Langley <agl@google.com>
Custom RSA and ECDSA keys may not expose the key material. Plumb and "opaque"
bit out of the *_METHOD up to EVP_PKEY. Query that in ssl_rsa.c to skip the
sanity checks for certificate and key matching.
Change-Id: I362a2d5116bfd1803560dfca1d69a91153e895fc
Reviewed-on: https://boringssl-review.googlesource.com/1255
Reviewed-by: Adam Langley <agl@google.com>
It's unused with SSLv2 gone. Also, being a decryption padding check, it really
should be constant-time and isn't.
Change-Id: I96be02cb50f9bf0229b9174eccd80fa338bf8e3e
Reviewed-on: https://boringssl-review.googlesource.com/1254
Reviewed-by: Adam Langley <agl@google.com>
Previously, public headers lived next to the respective code and there
were symlinks from include/openssl to them.
This doesn't work on Windows.
This change moves the headers to live in include/openssl. In cases where
some symlinks pointed to the same header, I've added a file that just
includes the intended target. These cases are all for backwards-compat.
Change-Id: I6e285b74caf621c644b5168a4877db226b07fd92
Reviewed-on: https://boringssl-review.googlesource.com/1180
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Match the other EVP_DigestSignFinal implementations. Fix the instances in
ssl/t1_enc.c which were not following the EVP_DigestSignFinal contract; on
entry, *out_len should contain the size of the buffer.
Change-Id: Icd44d97a4c98704dea975798c0101d5a37274d17
Reviewed-on: https://boringssl-review.googlesource.com/1130
Reviewed-by: Adam Langley <agl@google.com>
Converting check_autoarg from a macro to a function lost the behavior. Instead,
just move the logic into p_rsa.c which was the only EVP_PKEY implementation
that even needed the flag.
Also document this behavior on each of the functions. Make note of the out =
NULL case only returning the maximum output size, and not necessarily the
actual size.
For testing, update example_sign to determine the signature size using the NULL
behavior rather than querying the RSA key.
Change-Id: Iec6c2862028a5cfdefe8faa0e8c471755070898a
Reviewed-on: https://boringssl-review.googlesource.com/1121
Reviewed-by: Adam Langley <agl@google.com>
This change adds a new function, BN_bn2bin_padded, that attempts, as
much as possible, to serialise a BIGNUM in constant time.
This is used to avoid some timing leaks in RSA decryption.
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-----
Initial fork from f2d678e6e89b6508147086610e985d4e8416e867 (1.0.2 beta).
(This change contains substantial changes from the original and
effectively starts a new history.)
