The documentation for |BN_CTX_get| states: "Once |BN_CTX_get| has
returned NULL, all future calls will also return NULL until
|BN_CTX_end| is called." Some code takes advantage of that guarantee
by only checking the return value of the last call to |BN_CTX_get| in a
series of calls. That is correct and the most efficient way of doing
it. However, that pattern is inconsistent with most of the other uses
of |BN_CTX_get|. Also, static analysis tools like Coverity cannot
understand that pattern. This commit removes the instances of that
pattern that Coverity complained about when scanning *ring*.
Change-Id: Ie36d0223ea1caee460c7979547cf5bfd5fb16f93
Reviewed-on: https://boringssl-review.googlesource.com/5611
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>
Also implement it without reference to crypto/asn1 or fake ASN1_INTEGERs and
add a test. Some platform crypto APIs only give back the key size, and not the
encoded signature length. No sense in implementing it twice.
BUG=347404,499653
Change-Id: I9aa27d52674375f8b036e57bb5850f091c9b25dd
Reviewed-on: https://boringssl-review.googlesource.com/5080
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 one really needs to get C++'d.
Change-Id: I088f3b77867af0cfc2da8324ba87954ef4cbba74
Reviewed-on: https://boringssl-review.googlesource.com/4350
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>
The only dependency the low-level crypto modules have on code in
crypto/obj is their use of OBJ_nid2sn, which is trivial to avoid.
This facilitates future simplification of crypto/obj, including
possibly the removal of functions like OBJ_nid2sn and the complex
build infrastructure that supports them.
This change also removes EVP_CIPHER_name and EVP_MD_name.
Change-Id: I34ce7dc7e58d5c08b52f95d25eba3963590cf2f7
Reviewed-on: https://boringssl-review.googlesource.com/3932
Reviewed-by: Adam Langley <agl@google.com>
Limiting uses of crypto/bio to code that really need to it by avoiding
the use of BIO just to write to stdout/stderr.
Change-Id: I34e0f773161aeec073691e439ac353fb7b1785f3
Reviewed-on: https://boringssl-review.googlesource.com/3930
Reviewed-by: Adam Langley <agl@google.com>
Implement ECDSA_SIG_new and ECDSA_SIG_free manually in preparation for
removing all crypto/asn1 dependencies from ECDSA signature verification.
Change-Id: I0e84d74fa8e757af0cfb09daef03d59f428143cc
Reviewed-on: https://boringssl-review.googlesource.com/4153
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>
By using non-DER or invalid encodings outside the signed portion of a
certificate the fingerprint can be changed without breaking the signature.
Although no details of the signed portion of the certificate can be changed
this can cause problems with some applications: e.g. those using the
certificate fingerprint for blacklists.
1. Reject signatures with non zero unused bits.
If the BIT STRING containing the signature has non zero unused bits reject the
signature. All current signature algorithms require zero unused bits.
2. Check certificate algorithm consistency.
Check the AlgorithmIdentifier inside TBS matches the one in the certificate
signature. NB: this will result in signature failure errors for some broken
certificates.
3. Check DSA/ECDSA signatures use DER.
Reencode DSA/ECDSA signatures and compare with the original received signature.
Return an error if there is a mismatch.
This will reject various cases including garbage after signature (thanks to
Antti Karjalainen and Tuomo Untinen from the Codenomicon CROSS program for
discovering this case) and use of BER or invalid ASN.1 INTEGERs (negative or
with leading zeroes).
CVE-2014-8275
(Imported from upstream's 85cfc188c06bd046420ae70dd6e302f9efe022a9 and
4c52816d35681c0533c25fdd3abb4b7c6962302d)
Change-Id: Ic901aea8ea6457df27dc542a11c30464561e322b
Reviewed-on: https://boringssl-review.googlesource.com/2783
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
RAND_pseudo_bytes just calls RAND_bytes now and only returns 0 or 1. Switch all
callers within the library call the new one and use the simpler failure check.
This fixes a few error checks that no longer work (< 0) and some missing ones.
Change-Id: Id51c79deec80075949f73fa1fbd7b76aac5570c6
Reviewed-on: https://boringssl-review.googlesource.com/2621
Reviewed-by: Adam Langley <agl@google.com>
Saves doing it ad-hoc all the time.
Change-Id: Ic1a1180f56eec37c19799649bb8f18237bd617f8
Reviewed-on: https://boringssl-review.googlesource.com/2241
Reviewed-by: Adam Langley <agl@google.com>
r and s are scalars, not EC coordinates.
Change-Id: I46a20215d3c602559c18c74a1da9a91543ea73ca
Reviewed-on: https://boringssl-review.googlesource.com/2240
Reviewed-by: Adam Langley <agl@google.com>
Windows is much pickier about dllimport/dllexport. Declare it on
the declaration, not the definition. Also ensure that the declaration
precedes the definition. Finally, remove a stray OPENSSL_EXPORT.
Change-Id: Id50b9de5acbe5adf1b15b22dd60b7a5c13a80cce
Reviewed-on: https://boringssl-review.googlesource.com/1862
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>
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>
The |size| method was documented to return the same as |ECDSA_size| -
the max size of an ECDSA signature. However, this involves some ASN.1
calculations which is best done once. What custom implementations want
to give is the size of the group order on which the ASN.1 computations
are based.
This change switches the |size| method to allow that.
Change-Id: I95b6e0c2b52bfcd0d74850c2c4e9bc01269255e2
Reviewed-on: https://boringssl-review.googlesource.com/1200
Reviewed-by: David Benjamin <davidben@chromium.org>
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>
Initial fork from f2d678e6e89b6508147086610e985d4e8416e867 (1.0.2 beta).
(This change contains substantial changes from the original and
effectively starts a new history.)
