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
Found by running malloc tests with -valgrind. Unfortunately, the next one is
deep in crypto/asn1 itself, so I'm going to stop here for now.
Change-Id: I7a33971ee07c6b7b7a98715f2f18e0f29380c0a1
Reviewed-on: https://boringssl-review.googlesource.com/3350
Reviewed-by: Adam Langley <agl@google.com>
The ENGINE code had a concept of a stable-ABI for METHODs, because that
might be a useful thing in the future when people want to have blobs
that wrap PKCS#11 or something.
However, at the moment nobody uses this feature and it didn't work very
well anyway: I hadn't updated |ENGINE_free| to free them all and
|set_method| was copying the methods, but not resetting the |is_static|
flag.
This change removes support for non-static methods. We can always put it
back later if we need.
Change-Id: Ic7401c8cb1cadd46b26a215f85bc48562efe9919
Reviewed-on: https://boringssl-review.googlesource.com/3300
Reviewed-by: Adam Langley <agl@google.com>
We assume that AArch64 systems are modern enough to have getauxval.
Change-Id: I4cc74f04ca5ed50b8ca1cfd00afeaaa01c6caca0
Reviewed-on: https://boringssl-review.googlesource.com/3280
Reviewed-by: Adam Langley <agl@google.com>
Tag number 31 is a long form tag that requires multiple octets. It
cannot be handled by adding a single uint8. Changed CBB_add_asn1()
to return 0 when it is passed in the extension for tag 31.
Change-Id: Ia33936d4f174d1a7176eb11da0b5c7370efb9416
CBB_init_fixed() should not call free because it can lead to use after
free or double free bugs. The caller should be responsible for
creating and destroying the buffer.
In the current code, ssl3_get_v2_client_hello() may free s->init_buf->data
via CBB_init_fixed(). It can also be freed via SSL_free(s) since
ssl3_get_v2_client_hello() doesn't set it to NULL and CBB_init_fixed()
can't set the caller's pointer to NULL.
Change-Id: Ia05a67ae25af7eb4fb04f08f20d50d912b41e38b
When addressing [1], I checked the AEAD code but brain-farted: a key is
aligned in that code, but it's the Poly1305 key, which doesn't matter
here.
It would be nice to align the ChaCha key too, but Android doesn't have
|posix_memalign| in the versions that we care about. It does have
|memalign|, but that's documented as "obsolete" and we don't have a
concept of an Android OS yet and I don't want to add one just for this.
So this change uses the buffer for loading the key again.
(Note that we never used to check for alignment of the |key| before
calling this. We must have gotten it for free somehow when checking the
alignment of |in| and |out|. But there are clearly some paths that don't
have an aligned key:
https://code.google.com/p/chromium/issues/detail?id=454308.)
At least the generation script started paying off immediately ☺.
[1] https://boringssl-review.googlesource.com/#/c/3132/1/crypto/chacha/chacha_vec.c@185
Change-Id: I4f893ba0733440fddd453f9636cc2aeaf05076ed
Reviewed-on: https://boringssl-review.googlesource.com/3270
Reviewed-by: Adam Langley <agl@google.com>
Thanks to an anonymous bug report.
Change-Id: Icdde78c82c8ee13fb64e0124712b240295677f63
Reviewed-on: https://boringssl-review.googlesource.com/3260
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Android didn't have getauxval until Jelly Bean (4.1). This means that
BoringSSL running on older Androids won't be able to detect NEON
support. (This is moot for Chromium because Chromium calls
android_getCpuFeatures and sets the NEON flag itself, but other users of
BoringSSL on Android probably won't do that.)
This change mirrors a little of what upstream does and tries running a
NEON instruction with a handler for SIGILL installed.
Change-Id: I853b85c37ffb049b240582d71fcf07adedc37a30
Reviewed-on: https://boringssl-review.googlesource.com/3190
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
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>
|num_rounds| is neither a parameter nor manifest constant.
Change-Id: I6c1d3a3819731f53fdd01eef6bb4de8a45176a1d
Reviewed-on: https://boringssl-review.googlesource.com/3180
Reviewed-by: Adam Langley <agl@google.com>
Obviously I shouldn't be doing this by hand each time.
Change-Id: I64e3f5ede5c47eddbff3b18172a95becc681b486
Reviewed-on: https://boringssl-review.googlesource.com/3170
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
I put the header back, but missed the #endif at the end of the file.
Regenerating this is clearly too error prone – I'll write a script to do
it for the future.
Change-Id: I06968c9f7a4673f5942725e727c67cb4e01d361a
A handful of latin-1 codepoints existed a trio of files. This change
switches the encoding to UTF-8.
Change-Id: I00309e4d1ee3101e0cc02abc53196eafa17a4fa5
The special-case in HMAC is no longer needed. Test that HMAC_CTX is initialized
with the zero key.
Change-Id: I4ee2b495047760765c7d7fdfb4ccb510723aa263
Reviewed-on: https://boringssl-review.googlesource.com/3121
Reviewed-by: Adam Langley <agl@google.com>
By copying the input and output data via an aligned buffer, the
alignment requirements for the NEON ChaCha implementation on ARM can be
eliminted. This does, however, reduce the speed when aligned buffers are
used. However, updating the GCC version used to generate the ASM more
than makes up for that.
On a SnapDragon 801 (OnePlus One) the aligned speed was 214.6 MB/s and
the unaligned speed was 112.1 MB/s. Now both are 218.4 MB/s. A Nexus 7
also shows a slight speed up.
Change-Id: I68321ba56767fa5354b31a1491a539b299236e9a
Reviewed-on: https://boringssl-review.googlesource.com/3132
Reviewed-by: Adam Langley <agl@google.com>
This avoids a conflict with the Chromium build system, which
defines WIN32_LEAN_AND_MEAN with a different value.
BUG=crbug.com/453196
Change-Id: Ia15ec7c20325c1961af4f32e5208266e5f846f35
Reviewed-on: https://boringssl-review.googlesource.com/3150
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
The variable switches the default type for add_library from STATIC to SHARED.
We can condition additional stuff on that for convenience. (tabtest still
doesn't build.)
BoringSSL as any kind of stable system shared library is still very much
unsupported, but this is probably handy for making sure we don't forget all
those pesky OPENSSL_EXPORTs.
Change-Id: I66ab80bcddbf3724e03e85384141fdf4f4acbc2e
Reviewed-on: https://boringssl-review.googlesource.com/3092
Reviewed-by: Adam Langley <agl@google.com>
This reverts commit cd5c892a87. We'd rather get
rid of crypto/conf altogether, and these tests will require that we
OPENSSL_EXPORT conf.h's functions.
Change-Id: I271511ba321201e60de94e5c79c4b565ce31728f
Reviewed-on: https://boringssl-review.googlesource.com/3120
Reviewed-by: Adam Langley <agl@google.com>
Define WIN32_LEAN_AND_MEAN before including Windows Platform SDK
headers to preempt naming conflicts and to make the build faster. Avoid
including those headers in BoringSSL headers. Document that Platform
SDK 8.1 or later is required on Windows.
Change-Id: I907ada21dc722527ea37e839c71c5157455a7003
Reviewed-on: https://boringssl-review.googlesource.com/3100
Reviewed-by: Adam Langley <agl@google.com>
In an attempt to assign a zero-length HMAC key, consumers might
incorrectly call:
HMAC_Init_ex(key=NULL, key_len=0)
This does not work as expected since |key==NULL| has special semantics.
This bug may consequently result in uninitialized memory being used for
the HMAC key data.
This workaround doesn't fix all the problems associated with this
pattern, however by defaulting to a zero key the results are more
predictable than before.
BUG=http://crbug.com/449409
Change-Id: I777276d57c61f1c0cce80b18e28a9b063784733f
Reviewed-on: https://boringssl-review.googlesource.com/3040
Reviewed-by: Adam Langley <agl@google.com>
This matches the Chromium build.
Change-Id: I6ebd01c6ecb67c79577f98cf468dc204721595ef
Reviewed-on: https://boringssl-review.googlesource.com/3063
Reviewed-by: Adam Langley <agl@google.com>
out2 wasn't sized to account for stateful AEAD open requiring a seal overhead's
worth of scratch space. Also, pass in sizeof(out2) rather than a computed
ciphertext length, so the max_out check would have actually caught this.
Change-Id: Ibe689424f6c8ad550b3a45266699892076e7ba5e
Reviewed-on: https://boringssl-review.googlesource.com/3060
Reviewed-by: Adam Langley <agl@google.com>
Te4 is used in in crypto/aes/aes.c. It's used upstream in an alternate
implementation of AES_set_encrypt_key not included in our version.
Change-Id: I5704dbc714bdb05ef515cbf2aff5e43c3b62c5b2
Reviewed-on: https://boringssl-review.googlesource.com/3061
Reviewed-by: Adam Langley <agl@google.com>
Previously, the data for the common DH parameters was given twice: once
with 64-bit limbs and again with 32-bit limbs. A simple macro can
eliminate this duplication.
Change-Id: I15af008a769616f8146845cc8dd0e6526aa142ba
Reviewed-on: https://boringssl-review.googlesource.com/2950
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
(Imported from upstream's 2747d73c1466c487daf64a1234b6fe2e8a62ac75.)
Also fix up some stylistic issues in conf.c and clarify empty case in
documentation.
Change-Id: Ibacabfab2339d7566d51db4b3ac4579aec0d1fbf
Reviewed-on: https://boringssl-review.googlesource.com/3023
Reviewed-by: Adam Langley <agl@google.com>
(Imported from upstream's 004efdbb41f731d36bf12d251909aaa08704a756.)
The outer algorithm is already printed at the bottom of the function. This
allows any tools which print the X509 this way to determine if there is a
mismatch. This is also the point where the TBSCertificate is printed, not the
Certificate. See upstream's RT #3665.
Change-Id: I89baa4e4b626abf8813545a90eaa4409489ad893
Reviewed-on: https://boringssl-review.googlesource.com/3022
Reviewed-by: Adam Langley <agl@google.com>
(Imported from upstream's b3d7294976c58e0e05d0ee44a0e7c9c3b8515e05.)
May as well avoid diverging.
Change-Id: I3edec4fe15b492dd3bfb3146a8944acc6575f861
Reviewed-on: https://boringssl-review.googlesource.com/3020
Reviewed-by: Adam Langley <agl@google.com>
The bsaes-armv7.S asm has an #if __ARM_ARCH__>=7 around its contents,
i.e. it's not just switched at runtime – it only compiles for >= ARMv7.
I mistakenly regressed e_aes.c in 3e652657 to always expected bsaes
functions to exist on ARM. This change fixes that.
Change-Id: Ifd9111438508909a0627b25aee3e2f11e62e3ee8
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>
Recognize the four most commonly offered safe DH parameter sets when
negotiating multiplicative, ephemeral Diffie-Hellman. These values were
found from a scan of the Alexa common sites.
When a known safe prime is used, reduce the private key size
correspondingly.
Change-Id: I655eb7a5c743c0b389698c0471d16db5a0966652
This change syncs these asm files with upstream's 1.0.2 branch. The
important change is that they contain ARMv8 code that allows 32-bit ARM
code to use the hardware support in ARMv8 when running on such a chip.
Change-Id: Id37cb1ff0cbc98a8e328612df7cf60340ca96064
Reviewed-on: https://boringssl-review.googlesource.com/2921
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Comment fixups and a mismerge in aead_test. Also some buffer was larger than
needed.
Change-Id: I0e158089f42801575833684912f9edb206f61007
Reviewed-on: https://boringssl-review.googlesource.com/2870
Reviewed-by: Adam Langley <agl@google.com>
As feared, 2bca0988 did cause some leak checkers to get upset about the
state_hash pointer getting cleared.
This change makes err_shutdown free all the error queues to try and
avoid this. Hopefully this doesn't upset TSAN in turn.
BUG=448296
Change-Id: I827da63c793dcabc73168ece052cdcd3d3cc64e3
Reviewed-on: https://boringssl-review.googlesource.com/2890
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
The Android SDK version against which Chromium builds is too old to
include sys/auxv.h. This change switches the ARM code to use a weak
pointer for getauxval and to hard code the aux value numbers.
It also switches the license on cpu-arm.c because there's no OpenSSL
left in there now.
Change-Id: I440cb9d533a06d8b245b189d8e5148fa33e29412
Reviewed-on: https://boringssl-review.googlesource.com/2880
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
With GCC 4.9 and -O2 (and only -O2, -O1 and -O3 didn't trigger it), the
Poly1305 code can end up writing to an unaligned address otherwise and
that triggers a bus error on ARM.
Change-Id: Ifbeb7e2066a893d91d6f63c6565bac7d5542ef81
Reviewed-on: https://boringssl-review.googlesource.com/2850
Reviewed-by: Adam Langley <agl@google.com>
This is an initial cut at aarch64 support. I have only qemu to test it
however—hopefully hardware will be coming soon.
This also affects 32-bit ARM in that aarch64 chips can run 32-bit code
and we would like to be able to take advantage of the crypto operations
even in 32-bit mode. AES and GHASH should Just Work in this case: the
-armx.pl files can be built for either 32- or 64-bit mode based on the
flavour argument given to the Perl script.
SHA-1 and SHA-256 don't work like this however because they've never
support for multiple implementations, thus BoringSSL built for 32-bit
won't use the SHA instructions on an aarch64 chip.
No dedicated ChaCha20 or Poly1305 support yet.
Change-Id: Ib275bc4894a365c8ec7c42f4e91af6dba3bd686c
Reviewed-on: https://boringssl-review.googlesource.com/2801
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>
The use in s3_srvr.c doesn't care (it doesn't even have to be in bounds), but
it's good to have the value be initialized and not a function of the input.
(The old uninitialized case wasn't hit in s3_srvr.c because of the earlier
bounds check.)
Change-Id: Ib6b418b3c140aa564f8a46da3d34bb2b69f06195
Reviewed-on: https://boringssl-review.googlesource.com/2845
Reviewed-by: Adam Langley <agl@google.com>
According to X6.90 null, object identifier, boolean, integer and enumerated
types can only have primitive encodings: return an error if any of
these are received with a constructed encoding.
(Imported from upstream's 89f40f369f414b52e00f7230b0e3ce99e430a508.)
Change-Id: Ia5d15eef72e379119f50fdbac4e92c4761bf5eaf
Reviewed-on: https://boringssl-review.googlesource.com/2835
Reviewed-by: Adam Langley <agl@google.com>
Test that SSLv3 accepts arbitrary padding bytes (hello, POODLE) and rejects
non-minimal padding, while TLS accepts non-minimal padding but rejects
arbitrary padding bytes.
Also test what happens when the MAC is correct, but there is no padding. This
is the case that triggers a failing padding_ok check after the MAC check
on padding_len = 0 passes.
Change-Id: Ia1444c526437899fc57ceafcbcef9c8f5cb9a6c5
Reviewed-on: https://boringssl-review.googlesource.com/2702
Reviewed-by: Adam Langley <agl@google.com>
This introduces another knob into SSL_AEAD_CTX to omit the version from the ad
parameter. It also allows us to fold a few more SSL3_ENC_METHOD hooks together.
Change-Id: I6540d410d4722f734093554fb434dab6e5217d4f
Reviewed-on: https://boringssl-review.googlesource.com/2698
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>
CBC modes in SSLv3 are bust already with POODLE and we're moving away from it.
Align all the names from 'ssl3' and 'tls1' to 'tls', to match the names of the
TLS-only AEADs.
Change-Id: If742296a8e2633ef42a484e4d873b4a83558b6aa
Reviewed-on: https://boringssl-review.googlesource.com/2693
Reviewed-by: Adam Langley <agl@google.com>
The EVP_CIPHER codepath should no longer be used with TLS. It still exists for
DTLS and SSLv3. The AEAD construction in TLS does not allow for
variable-overhead AEADs, so stateful AEADs do not include the length in the ad
parameter. Rather the AEADs internally append the unpadded length once it is
known. EVP_aead_rc4_md5_tls is modified to account for this.
Tests are added (and RC4-MD5's regenerated) for each of the new AEADs. The
cipher tests are all moved into crypto/cipher/test because there's now a lot of
them and they clutter the directory listing.
In ssl/, the stateful AEAD logic is also modified to account for stateful AEADs
with a fixed IV component, and for AEADs which use a random nonce (for the
explicit-IV CBC mode ciphers).
The new implementation fixes a bug/quirk in stateless CBC mode ciphers where
the fixed IV portion of the keyblock was generated regardless. This is at the
end, so it's only relevant for EAP-TLS which generates a MSK from the end of
the key block.
Change-Id: I2d8b8aa11deb43bde2fd733f4f90b5d5b8cb1334
Reviewed-on: https://boringssl-review.googlesource.com/2692
Reviewed-by: Adam Langley <agl@google.com>
These helper functions will be used in the implementation of the legacy CBC
mode AEADs. The file is copied as-is and then modified to remove the dependency
on ssl/. Notably explicit IV logic is removed (that's a side effect of how
explicit IVs are currently implemented) and the padding length is returned
directly rather than smuggled into rec->type.
(Diffing tls_cbc.c and s3_cbc.c is probably the easiest for a review.)
The helpers are currently unused.
Change-Id: Ib703f4d3620196c9f2921cb3b8bf985f2d1777db
Reviewed-on: https://boringssl-review.googlesource.com/2691
Reviewed-by: Adam Langley <agl@google.com>
The extra free in ex_data_impl.c is fixing a mistake: when calling
|CRYPTO_cleanup_all_ex_data| the |EX_CLASS_ITEM| itself wouldn't be
freed.
The change in err_impl.c is to free the thread-id hash also. This allows
programs to free absolutely all memory allocated by BoringSSL, which
allows fuzz testing to find any memory leaks.
Change-Id: I1e518adf2b3e0efa7d7f00f7ab4e65e1dc70161e
Reviewed-on: https://boringssl-review.googlesource.com/2670
Reviewed-by: Adam Langley <agl@google.com>
DSA_verify and DSA_check_signature didn't share a codepath, so the fix was only
applied to the former. Implement verify in terms of check_signature and add
tests for bad DER variants.
Change-Id: I6577f96b13b57fc89a5308bd8a7c2318defa7ee1
Reviewed-on: https://boringssl-review.googlesource.com/2820
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>
Write the array literal of all zeros as {0} rather than {}.
Change-Id: If15330d96d019be671d3bcbbdea60c2b3ecc2128
Reviewed-on: https://boringssl-review.googlesource.com/2740
Reviewed-by: Adam Langley <agl@google.com>
I typoed this word and then auto-complete duplicated it all over the
place. This change fixes all the comments.
This change has no semantic effect (comment only).
Change-Id: I8952e9e71302043574757cd74a05e66500008432
Using OPENSSL_WINDOWS for this is inaccurate because it's really a
feature of the compiler, not the platform. I think it's only MSVC that
uses the UI64 suffix.
Change-Id: I4a95961b94e69e72b93f5ed1e0457661b74242c8
Reviewed-on: https://boringssl-review.googlesource.com/2730
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
ERR_get_error returns the least recent error, not the most recent error.
Nothing in err_test was actually asserting on that.
Change-Id: Ia49e29c231de4bbec77d037860ad1ffa8cce4779
Reviewed-on: https://boringssl-review.googlesource.com/2750
Reviewed-by: Adam Langley <agl@google.com>
One about a possible uninitialised variable (incorrect, but it's easier
to keep the compiler happy) and one warning about "const static" being
backwards.
Change-Id: Ic5976a5f0b48f32e09682e31b65d8ea1c27e5b88
Reviewed-on: https://boringssl-review.googlesource.com/2632
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Since this is C89 we need to maintain this ancient practice.
Change-Id: I7223e7c38a35cf551b6e3c9159d2e21ebf7e62be
Reviewed-on: https://boringssl-review.googlesource.com/2631
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
It's a static function anyway so it doesn't affect anything and it's
colliding with a debugging function on one platform.
Change-Id: Iae0595cce7cb2bdd4c56217f6f1de51ff3134a8b
Reviewed-on: https://boringssl-review.googlesource.com/2630
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
The expectation when calling HMAC with key=NULL and keylen=0 is to compute
HMAC on the provided data with a key of length 0 instead of using the
"previous" key, which in the case of HMAC() is whatever bytes happen to be
left on the stack when the HMAC_CTX struct is allocated.
Change-Id: I52a95e262ee4e15f1af3136cb9c07f42f40ce122
Reviewed-on: https://boringssl-review.googlesource.com/2660
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>
Bruce Dawson pointed out that the shadowing of |ret| in |s3_srvr.c|
looked dodgy. It was actually deliberate (we don't want to reset the
default value of the function's |ret| variable with a successful return
from the callback) but it does look dodgy.
This change adds -Wshadow to ban variable shadowing and fixes all
current instances.
Change-Id: I1268f88b9f26245c7d16d6ead5bb9014ea471c01
Reviewed-on: https://boringssl-review.googlesource.com/2520
Reviewed-by: Adam Langley <agl@google.com>
All serialization functions take point format as input, and
asn1_form is never used.
Change-Id: Ib1ede692e815ac0c929e3b589c3a5869adb0dc8b
Reviewed-on: https://boringssl-review.googlesource.com/2511
Reviewed-by: Adam Langley <agl@google.com>
According to rfc5480 and rfc4492 the hybrid format is not allowed
neither in certificates or the tls protocol.
Change-Id: I1d3fb5bef765bc7b58d29bdd60e15247fac4dc7a
Reviewed-on: https://boringssl-review.googlesource.com/2510
Reviewed-by: Adam Langley <agl@google.com>
The files should round-trip now. This corrects some discrepancies between
obj_mac.h and obj_mac.num which were also present in upstream. There seems to
be a mismerge in upstream's eebd5e5dd7dff58297ea52e1c21df8fccd593965.
(The discrepancy is harmless; those OIDs are not in obj_xref.txt.)
Change-Id: I1f6cda016533ec3182750310f9936f7e072b54a0
Reviewed-on: https://boringssl-review.googlesource.com/2474
Reviewed-by: Adam Langley <agl@google.com>
Probably best to keep the original format, trailing whitespace and all.
Change-Id: I81a0ac46fd4ab4bb9d2b03d930b191024971447c
Reviewed-on: https://boringssl-review.googlesource.com/2473
Reviewed-by: Adam Langley <agl@google.com>
This got reset at some point, but not the files generated from it.
obj_mac.num is an input/output parameter to objects.pl and used to keep the
NIDs stable.
Imported from f2d678e6e89b6508147086610e985d4e8416e867, the point at which we
forked.
Change-Id: Ifd52b1aaa55054d37bc1217f2375a93302839e23
Reviewed-on: https://boringssl-review.googlesource.com/2472
Reviewed-by: Adam Langley <agl@google.com>
Make the commands print a short usage summary and add a README file that
explains the dependencies.
Change-Id: I0c3f0713749ecfca23afaa2b536ac70dbdd7db0a
Reviewed-on: https://boringssl-review.googlesource.com/2471
Reviewed-by: Adam Langley <agl@google.com>
Upstream (impressively quickly) fixed the missing intrinsic. Switch Windows
clang back to building the same code as MSVC. Also include the intrin.h header
rather than forward-declare the intrinsic. clang only works if the header is
explicitly included. Chromium forcibly includes it to work around these kinds
of issues, but we shouldn't rely on it.
BUG=crbug.com/438382
Change-Id: I0ff6d48e1a3aa455cff99f8dc4c407e88b84d446
Reviewed-on: https://boringssl-review.googlesource.com/2461
Reviewed-by: Adam Langley <agl@google.com>
Use it in ssl3_cert_verify_hash so signing a pre-TLS-1.2 handshake hash can go
through RSA_sign and be intercepted via RSA_METHOD appropriately. This avoids
Windows needing to intercept sign_raw. (CAPI keys cannot provide sign_raw,
unless the input size happens to be that of NID_md5_sha1.)
Also use it in processing ServerKeyExchange to avoid special-casing RSA.
BUG=crbug.com/437023
Change-Id: Ia07433f468b75fdf7bfc8fa90c9751639b2478e6
Reviewed-on: https://boringssl-review.googlesource.com/2420
Reviewed-by: David Benjamin <davidben@google.com>
Windows clang lacks _umul128, but it has inline assembly so just use
that.
Change-Id: I6ff5d2465edc703a4d47ef0efbcea43d6fcc79fa
Reviewed-on: https://boringssl-review.googlesource.com/2454
Reviewed-by: Adam Langley <agl@google.com>
This probably snuck in when adapting the code from upstream. There's a header
file for it now. (Also it's uint32_t now rather than unsigned int.)
Change-Id: Ie8f45bc7a88988744174182a70512c0eff37cc1c
Reviewed-on: https://boringssl-review.googlesource.com/2441
Reviewed-by: Adam Langley <agl@google.com>
MSVC does not allow pointer arithmetic on void* pointers. Also
fix some style issues around whether * hugs the type or the
variable name.
Change-Id: I40cc1627830b37879fd70e2b688a42df62b6c62a
Reviewed-on: https://boringssl-review.googlesource.com/2452
Reviewed-by: Adam Langley <agl@google.com>
Don't use |BIO_set_foo_buffer_size| when setting the
sizes of the buffers while making buffer pair. Since it
happens in pair.c we know the BIOs are BIO pairs and using
bio_ctrl here complicates setting external buffers. Also
zero out bio_bio_st during construction.
This fixes a problem that would happen if the default buffer
sizes were not set, since buf_externally_allocated was
not yet initialized.
Remove BIO_C_SET_BUFF_SIZE and BIO_CTRL_RESET which are
not used for bio pairs.
Change-Id: I365091d5f44f6f1c5522c325a771bdf03d8fe950
Reviewed-on: https://boringssl-review.googlesource.com/2370
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>
Also add functionality for setting external buffers to give the
caller better control of the buffers. This is typical needed if OS
sockets can outlive the bio pair.
Change-Id: I500f0c522011ce76e9a9bce5d7b43c93d9d11457
No need to include unistd.h. (Though it probably should include string.h for
memcmp and strcmp.)
Change-Id: Ib09d2da4f7079c9d87338df75ec3560f4f203764
Reviewed-on: https://boringssl-review.googlesource.com/2260
Reviewed-by: Adam Langley <agl@google.com>
The error queue should only take ownership of the data if ERR_get_* is called,
not ERR_peek_*. Add a test for ERR_peek_error_line_data.
Change-Id: I976fc90fb54437dff723418ef3afd94f1c967922
Reviewed-on: https://boringssl-review.googlesource.com/2237
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>
One of them was never implemented upstream or downstream. The other no longer
works in BoringSSL. They're not used within BoringSSL (this still compiles),
even in X509_INFO, and do not appear to be used by consumers. If they were, we
would like to know via a compile failure.
This removes the last consumer within BoringSSL of the ASN.1 parsing macros.
Change-Id: Ifb72b1fcd0a4f7b3e6b081486f8638110872334b
Reviewed-on: https://boringssl-review.googlesource.com/2203
Reviewed-by: Adam Langley <agl@google.com>
(Imported form upstream's 455b65dfab0de51c9f67b3c909311770f2b3f801 and
0d6a11a91f4de238ce533c40bd9507fe5d95f288)
Change-Id: Ia195c7fe753cfa3a7f8c91d2d7b2cd40a547be43
Imported from upstream's 9bed73adaa6f834177f29e478d9a2247a6577c04.
Upstream's commit appears to have been based on BoringSSL's commits to
improve the constant-time behaviour of RSA padding checks and thus I've
not tried to import those bits of the change.
Change-Id: I0ea5775b0f1e18741bbbc9f792a6af0d3d2a4caf
When calling X509_set_version to set v1 certificate, that should mean
that the version number field is omitted.
(Imported from upstream's 8c0d19d8577c9a96b65622bfa92d0affd6bbb4ac)
Change-Id: If433fda7b6ccbd899f3379a38581c351cf4a82da
Can't really happen, but the flow of control isn't obvious. Add an
initializer.
(Imported from upstream's fa2ae04c40510262d198131c758acd8aa5a9b4ce)
Change-Id: If393687bca9f505b825feffaf2a63895a0ea5b6a
Pull constant-time methods out to a separate header, add tests.
(Imported from upstream's 9a9b0c0401cae443f115ff19921d347b20aa396b and
27739e92659d38cdefa21e51b7f52b81a7ac3388)
Change-Id: Id570f5c531aca791112929e6258989f43c8a78d7
The old code implicitly relies on the ASN.1 code returning a \0-prefixed
buffer when the buffer length is 0. Change this to verify explicitly
that the ASN.1 string has positive length.
(Imported from upstream's 7f7c05ca638c3cc6d261961fae439cd91e3c1d27)
Change-Id: Icc6c44b874bdcb02374016a36d209830d6162a8a
When d2i_ECPrivateKey reads a private key with a missing (optional)
public key, generate one automatically from the group and private key.
(Imported from upstream's 2083f7c465d07867dd9867b8742bb71c03d1f203)
Change-Id: I9e5090de87cf846ab92e4be5b6bf64e6091d02e4
Remove the existing md5_test and sha1_test. They now are all covered by
digest_test. For good measure, test the one-shot functions too.
Change-Id: I8e144cc563fb8817144e26cbd2e10c15642464ba
Reviewed-on: https://boringssl-review.googlesource.com/2211
Reviewed-by: Adam Langley <agl@google.com>
Two leaks can happen: if idx is -1, the newly allocated entry may not be freed.
Also, for X509_PURPOSE_add, if only one BUF_strdup succeeds, it will leak.
Restructure both so that the allocations happen ahead of time and are properly
cleaned up. This avoids leaving an existing entry in a half-broken state.
Found (sort of) by scan-build; because of all the indirections and DYNAMIC
flags, it doesn't actually realize the leak's been fixed.
Change-Id: I5521889bd14e007b3f62b6a4906d7c346698b48c
Reviewed-on: https://boringssl-review.googlesource.com/2209
Reviewed-by: Adam Langley <agl@google.com>
If it fails to be added to the list, the input should be freed. Found (sort of)
by scan-build; because of all the indirections and DYNAMIC flags, it doesn't
actually realize the leak's been fixed.
Change-Id: Idca10964e1ffb2ace1cea7f88d94693205d70d5b
Reviewed-on: https://boringssl-review.googlesource.com/2208
Reviewed-by: Adam Langley <agl@google.com>
Appease clang scan-build a bit. I'm not sure it's actually worth silencing all
of them because some of them look like preserving invariants between local
variables, but some are clearly pointless or can be restructured slightly.
Change-Id: I0bc81e2589bb402ff3ef0182d7a8921e31b85052
Reviewed-on: https://boringssl-review.googlesource.com/2205
Reviewed-by: Adam Langley <agl@google.com>
(Imported from upstream's ef908777218bd4a362dbe9cebb8e18fa8ab384cf.)
Change-Id: Id9b288d230cc9d8ab308690a18e687e2132e3293
Reviewed-on: https://boringssl-review.googlesource.com/2168
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
(Imported from upstream's 267e6f3cc0ef78dea4e5cf93907a71556a45f008)
Change-Id: I99cfd0196b9625449c50494562c44f57f09fed17
Reviewed-on: https://boringssl-review.googlesource.com/2167
Reviewed-by: David Benjamin <davidben@chromium.org>
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>
It's never used, upstream or downstream. The 64-bit value is wrong anyway for
LLP64 platforms.
Change-Id: I56afc51f4c17ed3f1c30959b574034f181b5b0c7
Reviewed-on: https://boringssl-review.googlesource.com/2123
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>
bl and bh are never used by macros if either BN_UMULT_LOHI or
BN_UMULT_HIGH are defined.
Change-Id: I7fdd45014a6b78cc586b5223396e09bc213608a5
Reviewed-on: https://boringssl-review.googlesource.com/2105
Reviewed-by: Adam Langley <agl@google.com>
Chromium's doesn't have built-in support for ml64.exe. Seems easier to
just build consistently with Yasm on both Win32 and Win64. (This will
require an equivalent change in Chromium's build, but keep upstream
and downstream builds consistent.)
Also don't set CMAKE_ASM_NASM_COMPILER explicitly; cmake's default
ASM_NASM behavior will search for both nasm or yasm in %PATH%. Leave
it unset so it can be overwritten on the command-line to point to
a particular yasm. Update BUILDING accordingly.
Verified the tests still pass.
Change-Id: I7e434be474b5b2d49e3bafbced5b41cc0246bd00
Reviewed-on: https://boringssl-review.googlesource.com/2104
Reviewed-by: Adam Langley <agl@google.com>
generic.c still needs to include generic implementations in Win64.
Those are currently done with inline assembly and won't work on
MSVC.
Change-Id: Ifeb5470872d8c97b2ccffeae6f3ccb5661051de3
Reviewed-on: https://boringssl-review.googlesource.com/2102
Reviewed-by: Adam Langley <agl@google.com>
This is no longer used but, by retaining it, we might miss cases where
code is still testing against it.
Change-Id: I40ed47e41f903aaf2c5e5354d4348f8890021382
Reviewed-on: https://boringssl-review.googlesource.com/2110
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Although x86masm.pl exists, upstream's documentation suggest only x86nasm.pl is
supported. Yasm seems to handle it fine with a small change.
Change-Id: Ia77be57c6b743527225924b2b398f2f07a084a7f
Reviewed-on: https://boringssl-review.googlesource.com/2092
Reviewed-by: Adam Langley <agl@google.com>
We were building the NASM flavor with MASM which is why it didn't work. Get the
MASM output working: cpuid and cmove are not available in MASM unless the file
declares .686. Also work around MASM rejecting a very long line in SHA-256.
The follow-up change will get the NASM flavor working. We should probably use
that one as it's documented as supported upstream. But let's make this one
functional too.
Change-Id: Ica69cc042a7250c7bc9ba9325caab597cd4ce616
Reviewed-on: https://boringssl-review.googlesource.com/2091
Reviewed-by: Adam Langley <agl@google.com>
Win32 still has assembly issues and bssl wants to select() on both sockets and
stdin (doesn't work on Windows). But this is a start.
Change-Id: Iafc5215be281aed836c5ac2dc8b379399848a2c2
Reviewed-on: https://boringssl-review.googlesource.com/2090
Reviewed-by: Adam Langley <agl@google.com>
X509_NAME is one of the symbols that collide with wincrypt.h. Move it to x509.h
so libraries which only use the pure-crypto portions of BoringSSL without X.509
needn't have to resolve the collision.
Change-Id: I057873498e58fe4a4cf264356f9a58d7a15397b7
Reviewed-on: https://boringssl-review.googlesource.com/2080
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>
Do away with all those unreadable macros. Also fix many many memory leaks in
the SSL_SESSION reuse case. Add a number of helper functions in CBS to help
with parsing optional fields.
Change-Id: I2ce8fd0d5b060a1b56e7f99f7780997fabc5ce41
Reviewed-on: https://boringssl-review.googlesource.com/1998
Reviewed-by: Adam Langley <agl@google.com>
Zero is encoded as a single zero octet. Per X.690, 8.3.1:
The encoding of an integer value shall be primitive. The contents octets
shall consist of one or more octets.
Change-Id: If4304a2be5117b71446a3a62a2b8a6124f85a202
Reviewed-on: https://boringssl-review.googlesource.com/2010
Reviewed-by: Adam Langley <agl@google.com>
Companion to CBS_get_asn1_uint64. Also add tests for both the parsing and the
serializing.
Change-Id: Ic5e9a0089c88b300f874712d0e9964cb35a8c40b
Reviewed-on: https://boringssl-review.googlesource.com/1999
Reviewed-by: Adam Langley <agl@google.com>
This has been wrong since the initial rework of e_aes.c.
Change-Id: I91d92b643c151cd38a272a27f805e5f8ba6dc2df
Reviewed-on: https://boringssl-review.googlesource.com/1981
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
BUF_strlcpy still assumes |src| is a NUL-terminated string and will call strlen
on it to determine the actual length. BUF_strndup's input need not be
NUL-terminated.
Change-Id: I9ca95e92533d12f1b0283412249bda4f8cf92433
Reviewed-on: https://boringssl-review.googlesource.com/1997
Reviewed-by: Adam Langley <agl@google.com>
Intended to make parsing ASN.1 structures with OPTIONAL elements easier. (Just
attempting to parse the next tag doesn't distinguish between a malformed CBS
which has now been partially advanced and an optional tag mismatch.)
Change-Id: Idceb3dfd6ec028e87e1bc5aaddcec177b0c32150
Reviewed-on: https://boringssl-review.googlesource.com/1995
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>
Implementations of ENGINEs often don't want to implement every function.
This change adds an error code for those situations.
Change-Id: Id6b7eace36d06ffad7f347f556d942d447d8a2fd
Reviewed-on: https://boringssl-review.googlesource.com/1920
Reviewed-by: Wan-Teh Chang <wtc@chromium.org>
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>
Verified that nothing uses it.
Change-Id: I1755144129e274f3d1680ddb8cb12273070eb078
Reviewed-on: https://boringssl-review.googlesource.com/1912
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>
One ASN1_R_UNKNOWN_FORMAT got mispelled into ASN1_R_UNKOWN_FORMAT and
duplicated.
Change-Id: If123ef848ffe68afa021f5f3e3fb08eac92c5f94
Reviewed-on: https://boringssl-review.googlesource.com/1911
Reviewed-by: Adam Langley <agl@google.com>
ERR_FLAG_MALLOCED is now masked and isn't returned through
ERR_get_error_line_data.
Change-Id: Ida633bf1a5ca01f563c1323dbdfb2433c2ab5159
Reviewed-on: https://boringssl-review.googlesource.com/1910
Reviewed-by: Adam Langley <agl@google.com>
I misunderstood the OpenSSL semantics here. When receiving an error data
pointer via ERR_get_error_line_data and friends, although the error is
cleared, OpenSSL retains ownership of the data pointer. It's kept in the
cleared error until another error overrides it, or the whole error queue
is cleared.
It's pretty odd to have live pointers in empty errors so this change
allows an error queue to retain one data pointer. Thus the pointer
returned from ERR_get_error_line_data is valid until the next call to
ERR_get_error_line_data, or until the queue is freed.
From reviewing uses of the API, this is sufficient for all of them.
Change-Id: I73cb8e9c792452ae3c1a934ac8bbe8b5353b65b2
Reviewed-on: https://boringssl-review.googlesource.com/1880
Reviewed-by: Adam Langley <agl@google.com>
Not that these functions can actually fail. The only codepaths that do so are
user errors.
Change-Id: I9fcbd402ab6574b5423ae22b462a0e1192ef01d7
Reviewed-on: https://boringssl-review.googlesource.com/1900
Reviewed-by: Adam Langley <agl@google.com>
NaCl defines _POSIX_C_SOURCE on the command line for some reason, thus
we have to be defensive about defining it.
Change-Id: Icbc8afcb1ac0e0ca23b788b11ea911c3f55a8b7f
Reviewed-on: https://boringssl-review.googlesource.com/1891
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
_BSD_SOURCE has been deprecated (see bug). The manpage for printf
suggests that any _POSIX_C_SOURCE >= 200112L is also sufficient to bring
in the needed declarations and the bug reporter confirms that it's
sufficient for him.
https://code.google.com/p/chromium/issues/detail?id=419859
Change-Id: Ifc053f11c5aa1df35aae8e952d2c73a7f4599ec2
Reviewed-on: https://boringssl-review.googlesource.com/1890
Reviewed-by: Adam Langley <agl@google.com>
Winsock needs to be initialized. Also, perror doesn't do anything
useful and read/recv aren't interchangeable.
Change-Id: Ic9dfd6907b7b0d396eafe72072a29d027b66bc0c
Reviewed-on: https://boringssl-review.googlesource.com/1871
Reviewed-by: Adam Langley <agl@google.com>
vsnprintf returns -1 on Windows on truncation, not the needed
size.
Change-Id: I0a9f32504127b2fb740244c3b59132e201d14234
Reviewed-on: https://boringssl-review.googlesource.com/1870
Reviewed-by: Adam Langley <agl@google.com>
This resolves a pile of MSVC warnings in Chromium.
Change-Id: Ib9a29cb88d8ed8ec4118d153260f775be059a803
Reviewed-on: https://boringssl-review.googlesource.com/1865
Reviewed-by: Adam Langley <agl@google.com>
This avoids unnecessary differences between LP64 and LLP64. Also
MSVC throws overflow warnings in the big-endian 64-bit codepath,
so use the preprocessor.
Change-Id: I74cef2d631d39f282177e043ed24bc6ecbbcb8fd
Reviewed-on: https://boringssl-review.googlesource.com/1860
Reviewed-by: Adam Langley <agl@google.com>
close is closesocket. Also some of the headers are different
and inet_aton should be inet_pton.
Change-Id: I9eee0880d91833bdd3bcf0f2a352923c9fb1a891
Reviewed-on: https://boringssl-review.googlesource.com/1864
Reviewed-by: Adam Langley <agl@google.com>
It's unnecessary and Windows was unhappy about the signed/unsigned
comparisons.
Change-Id: If2c4a20de48a2cddb0a4e0ca01e84eef91b155db
Reviewed-on: https://boringssl-review.googlesource.com/1863
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>
Windows doesn't have it, and it should have been size_t anyway.
Change-Id: I901b8d78182576eaa52384d3ffef4810ff48cf7b
Reviewed-on: https://boringssl-review.googlesource.com/1861
Reviewed-by: Adam Langley <agl@google.com>
This reduces the delta for getting Android to compile and avoids having
Android carry around diffs to upstream versions of tcpdump.
Change-Id: I7f4cbb22b7a0f246bbebe960ca2139f0f42e14a0
Reviewed-on: https://boringssl-review.googlesource.com/1830
Reviewed-by: Adam Langley <agl@google.com>
Now that symbol visibility is done correctly, this shouldn't be needed.
Change-Id: I608beed1de63c1309358ff17dd28e3191e87dbd4
Reviewed-on: https://boringssl-review.googlesource.com/1810
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
In order to minimise the upstream diffs needed for bits of Android to
build with BoringSSL, this change implements the old style PKCS#12
functions as wrappers around the modern parser.
The function to read all the contents of a BIO could almost be a utility
function but I'll wait until there are two uses for it first.
The important change from the original functions is that these will
always read the complete buffer/BIO/FILE passed in. Based on a survey of
uses of d2i_PKCS12 that I found, this appears to be universally what
callers want anyway.
Change-Id: I3f5b84e710b161d975f91f4d16c83d44371368d1
Reviewed-on: https://boringssl-review.googlesource.com/1791
Reviewed-by: Adam Langley <agl@google.com>
Android requested that the wpa_supplicant go upstream. This change adds
some dummy functions and reinstates DSA_dup_DH in order to make the diff
smaller and easier for upstream.
Change-Id: I77ac271b8652bae5a0bbe16afde51d9096f3dfb5
Reviewed-on: https://boringssl-review.googlesource.com/1740
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>
Change-Id: Id77fb7c904cbfe8172466dff20b6a715d90b806c
Reviewed-on: https://boringssl-review.googlesource.com/1710
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
This isn't a header file that makes sense to export; any compilation unit which
includes it will gain a bunch of static arrays.
Change-Id: Ic698b74bdf758506a53d4eba19ab8b0f49a11ef7
Reviewed-on: https://boringssl-review.googlesource.com/1692
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>
Useful for parsing things when you don't know what tag to expect (e.g. a
CHOICE). Also allow its operands to be NULL, so that it can be used to skip
arbitrary elements.
Delete CBS_ASN1_ANY which wasn't doing anything.
Change-Id: I56413e68b4f2e13860ea3e55373d5830713d7e5c
Reviewed-on: https://boringssl-review.googlesource.com/1661
Reviewed-by: Adam Langley <agl@google.com>
Get all this stuff out of the way.
- OPENSSL_NO_MD5
- OPENSSL_NO_SHA
- OPENSSL_NO_EC
- OPENSSL_NO_ECDSA
- OPENSSL_NO_ECDH
- OPENSSL_NO_NEXTPROTONEG
- OPENSSL_NO_DH
- OPENSSL_NO_SSL3
- OPENSSL_NO_RC4
- OPENSSL_NO_RSA
Also manually removed a couple instances of OPENSSL_NO_DSA that seemed to be
confused anyway. Did some minor manual cleanup. (Removed a few now-pointless
'if (0)'s.)
Change-Id: Id540ba97ee22ff2309ab20ceb24c7eabe766d4c4
Reviewed-on: https://boringssl-review.googlesource.com/1662
Reviewed-by: Adam Langley <agl@google.com>
We don't really want to expose this but the bytestring test calls this
function directly and, when linked against a shared library, thus needs
it to be in the dynamic symbol table.
Change-Id: Ife01da36fe142026a6a3e545cff746512b966ee6
This change removes the previous OpenSSL/NSS hack in PKCS#12 parsing and
limits the hacks purely to the BER->DER conversion function, where they
belong.
PKCS#7 and #12 switch between implicit and explicit tags in different
places and sometimes only implicitly define that they are using implicit
tags. This change fixes a previous confusion where an implicit tag was
thought to be explicit.
Change-Id: Ib68c78cf2a1bfcbf90a296cb98313ab86ed2a1f3
Reviewed-on: https://boringssl-review.googlesource.com/1640
Reviewed-by: Adam Langley <agl@google.com>
wpa_supplicant needs this in order to get the order of the coordinate
field, apparently so that they can hash to a point.
Change-Id: I92d5df7b37b67ace5f497c25f53f16bbe134aced
Reviewed-on: https://boringssl-review.googlesource.com/1622
Reviewed-by: Adam Langley <agl@google.com>
This only applies to RC4, but it is still used by some Android code.
Change-Id: I4cf86269ffb7a230576da1bb2bfef7e1d4f234d6
Reviewed-on: https://boringssl-review.googlesource.com/1621
Reviewed-by: Adam Langley <agl@google.com>
Sadly this is needed by wpa_supplicant for NTLM hashes.
Change-Id: I1c362c676a11ee01f301ff6fbd33d0669396ea23
Reviewed-on: https://boringssl-review.googlesource.com/1620
Reviewed-by: Adam Langley <agl@google.com>
This fixes several of the problems with the old API.
- Padding was completely ignored.
- ='s in the middle of the input were accepted.
- It tries to be helpful and strips leading/trailing whitespace.
Change-Id: I99b9d5e6583f7eaf9bf0b6ee9ca39799811b58dc
Reviewed-on: https://boringssl-review.googlesource.com/1602
Reviewed-by: Adam Langley <agl@google.com>
This is cleaner than the OpenSSL code was, at least, but it's hardly
beautiful due to the "standard" that it's trying to implement. (See
[1].)
The references from the PKCS#8 code to various ciphers have digests have
been made into function pointer references rather than NIDs so that the
linker will be able to drop RC2 code for binaries that don't call PKCS#8
or #12 functions.
A bug that crashed OpenSSL/BoringSSL when parsing a malformed PKCS#8
structure has been fixed too.
See https://www.cs.auckland.ac.nz/~pgut001/pubs/pfx.html
Change-Id: Iaa1039e04ed7877b90792835e8ce3ebc3b29f89e
Reviewed-on: https://boringssl-review.googlesource.com/1592
Reviewed-by: Adam Langley <agl@google.com>
Previously, the ASN.1 functions in bytestring were capable of processing
indefinite length elements when the _ber functions were used. That works
well enough for PKCS#3, but NSS goes a bit crazy with BER encoding and
PKCS#12. Rather than complicate the core bytestring functions further,
the BER support is removed from them and moved to a separate function
that converts from BER to DER (if needed).
Change-Id: I2212b28e99bab9fab8c61f80d2012d3e5a3cc2f0
Reviewed-on: https://boringssl-review.googlesource.com/1591
Reviewed-by: Adam Langley <agl@google.com>
As useless as it might seem, the certificates in PKCS#12 files appear to
always be encrypted with 40-bit RC2. OpenSSL, NSS and Windows are all
the same on this point. Thus, in order to be able to import PKCS#12
files we need RC2 support.
RC2 has deliberately not been added to EVP_get_cipherbynid so that the
linker can drop the RC2 code unless the PKCS#12 functions are actually
called.
Change-Id: I5b2062fdf78cb622a8038c326da01aac8fb58962
Reviewed-on: https://boringssl-review.googlesource.com/1590
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>
This change removes the old ASN.1 functions (ASN1_seq_unpack and
ASN1_seq_pack) which have always been disabled in BoringSSL.
It also removes code enabled by OPENSSL_EXPORT_VAR_AS_FUNCTION, which
we have never used.
Change-Id: I1fe323abf945a8a5828a04cc195c072e100a5095
Reviewed-on: https://boringssl-review.googlesource.com/1556
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
This change extracts two, common parts of RSA_decrypt and RSA_sign into
a function called |private_transform|. It also allows this to be
overridden in a method, which is convenient for opaque keys that only
expose the raw RSA transform as it means that the padding code from
BoringSSL can be easily reimplemented.
One significant change here is that short RSA ciphertexts will no longer
be accepted. I think this is correct and OpenSSL has a comment about PGP
mistakenly stripping leading zeros. However, these is the possibility
that it could break something.
Change-Id: I258c5cbbf21314cc9b6e8d2a2b898fd9a440cd40
Reviewed-on: https://boringssl-review.googlesource.com/1554
Reviewed-by: Adam Langley <agl@google.com>
This lets us put the SSL_CIPHER table in the data section. For type-checking,
make STACK_OF(SSL_CIPHER) cast everything to const SSL_CIPHER*.
Note that this will require some changes in consumers which weren't using a
const SSL_CIPHER *.
Change-Id: Iff734ac0e36f9e5c4a0f3c8411c7f727b820469c
Reviewed-on: https://boringssl-review.googlesource.com/1541
Reviewed-by: Adam Langley <agl@google.com>
Android uses these for some conversions from Java formats. The code is
sufficiently bespoke that putting the conversion functions into
BoringSSL doesn't make a lot of sense, but the alternative is to expose
these ones.
Change-Id: If1362bc4a5c44cba4023c909e2ba6488ae019ddb
Several callers of EVP_EncodeBlock are doing ad-hoc versions of this
function without any overflow checks.
Change-Id: I4d0cad2347ea8c44b42465e8b14b2783db69ee8f
Reviewed-on: https://boringssl-review.googlesource.com/1511
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Android needs it. These functions were removed in the move to BoringSSL.
Change-Id: Ice24a0a1c390930cf07dbd00f72a3e12e6c241f9
Reviewed-on: https://boringssl-review.googlesource.com/1510
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
ssl23_get_client_hello has lots of remnants of SSLv2 support and remnants of an
even older SSL_OP_NON_EXPORT_FIRST option (see upstream's
d92f0bb6e9ed94ac0c3aa0c939f2565f2ed95935) which complicates the logic.
Split it into three states and move V2ClientHello parsing into its own
function. Port it to CBS and CBB to give bounds checks on the V2ClientHello
parse.
This fixes a minor bug where, if the SSL_accept call in ssl23_get_client_hello
failed, cb would not be NULL'd and SSL_CB_ACCEPT_LOOP would get reported an
extra time.
It also unbreaks the invariant between s->packet, s->packet_length,
s->s3->rbuf.buf, and s->s3->rbuf.offset at the point the switch, although this
was of no consequence because the first ssl3_read_n call passes extend = 0
which resets s->packet and s->packet_length.
It also makes us tolerant to major version bumps in the ClientHello. Add tests
for TLS tolerance of both minor and major version bumps as well as the HTTP
request error codes.
Change-Id: I948337f4dc483f4ebe1742d3eba53b045b260257
Reviewed-on: https://boringssl-review.googlesource.com/1455
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: I908d207ccd3d529ec09c687effc2aeb4631127d9
Reviewed-on: https://boringssl-review.googlesource.com/1470
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Windows doesn't have ssize_t, sadly. There's SSIZE_T, but defining an
OPENSSL_SSIZE_T seems worse than just using an int.
Change-Id: I09bb5aa03f96da78b619e551f92ed52ce24d9f3f
Reviewed-on: https://boringssl-review.googlesource.com/1352
Reviewed-by: Adam Langley <agl@google.com>
Clang's integrated as accepts unified ARM syntax only. This change
updates the GHASH ARM asm to use that syntax and thus be compatible.
Patch from Nico Weber.
https://code.google.com/p/chromium/issues/detail?id=124610
Change-Id: Ie6f3de4e37286f0af39196fad33905f7dee7402e
- Upon parsing, reject OIDs with invalid base-128 encoding.
- Always NUL-terminate the destination buffer in OBJ_obj2txt printing
function.
CVE-2014-3508
(Imported from upstream's c01618dd822cc724c05eeb52455874ad068ec6a5)
Change-Id: I12bdeeaa700183195e4c2f474f964f8ae7a04549
Reviewed-on: https://boringssl-review.googlesource.com/1440
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
This function serialises a PKCS#7 structure containing a number of
certificates.
Change-Id: Iaf15887e1060d5d201d5a3dd3dca8d51105ee6d6
Reviewed-on: https://boringssl-review.googlesource.com/1431
Reviewed-by: Adam Langley <agl@google.com>
When shifting data because extra ASN.1 length bytes were needed, the
data was moved from the start of the ASN.1 length, not the start of the
ASN.1 data.
Change-Id: Ib13d5e4e878774df2af0505c0297eff6cf781728
Reviewed-on: https://boringssl-review.googlesource.com/1430
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Avoid needing to manually increment the reference count and using the right
lock, both here and in Chromium.
Change-Id: If116ebc224cfb1c4711f7e2c06f1fd2c97af21dd
Reviewed-on: https://boringssl-review.googlesource.com/1415
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>
bn_get_bits5 always reads two bytes, even when it doesn't need to. For some
sizes of |p|, this can result in reading just past the edge of the array.
Unroll the first iteration of the loop and avoid reading out of bounds.
Replace bn_get_bits5 altogether in C as it's not doing anything interesting.
Change-Id: Ibcc8cea7d9c644a2639445396455da47fe869a5c
Reviewed-on: https://boringssl-review.googlesource.com/1393
Reviewed-by: Adam Langley <agl@google.com>
Replace the tree-like structure by a linear approach, with fewer special
cases to handle value 0.
(Imported from upstream's d5213519c0ed87c71136084e7e843a4125ecc024.)
Change-Id: Icdd4815066bdbab0d2c0020db6a8cacc49b3d82a
Reviewed-on: https://boringssl-review.googlesource.com/1400
Reviewed-by: Adam Langley <agl@google.com>
A single va_list may not be used twice. Nothing calls BIO_vprintf and it just
(v)snprintfs into a buffer anyway, so remove it. If it's actually needed, we
can fiddle with va_copy and the lack of it in C89 later, but anything that
actually cares can just assemble the output externally.
Add a test in bio_test.c.
BUG=399546
Change-Id: Ia40a68b31cb5984d817e9c55351f49d9d6c964c1
Reviewed-on: https://boringssl-review.googlesource.com/1391
Reviewed-by: Adam Langley <agl@google.com>
PEDANTIC was not closed, but rather the compiler being used.
Change-Id: I743118f1481adddcd163406be72926fff6c87338
Reviewed-on: https://boringssl-review.googlesource.com/1388
Reviewed-by: Adam Langley <agl@google.com>
OPENSSL_FIPS was removed in 64f4c91b89,
but these definitions in crypto/pem remained.
Change-Id: Ia85dd3fd7161f0b33b471b17643767b2b33fdda6
Reviewed-on: https://boringssl-review.googlesource.com/1381
Reviewed-by: Adam Langley <agl@google.com>
The original functions do an ascii_to_ucs2 transformation on the password.
Deprecate them in favor of making that encoding the caller's problem.
ascii_to_ucs2 doesn't handle, say, UTF-8 anyway. And with the original OpenSSL
function, some ciphers would do the transformation, and some wouldn't making
the text-string/bytes-string confusion even messier.
BUG=399121
Change-Id: I7d1cea20a260f21eec2e8ffb7cd6be239fe92873
Reviewed-on: https://boringssl-review.googlesource.com/1347
Reviewed-by: Adam Langley <agl@google.com>
PNaCl builds BoringSSL with OPENSSL_NO_ASM, but the new OPENSSL_cleanse
was using inline assembly anyway. It appears that even though the inline
asm was empty, it still breaks the PNaCl build:
disallowed: inline assembly: call void asm sideeffect "", "r,~{memory}"(i8* %.asptr319), !dbg !96986
With this change, we don't have any compiler scarecrows for
OPENSSL_cleanse any longer when using OPENSSL_NO_ASM :( Maybe, one day,
we'll get memset_s in our base platform.
Change-Id: Ia359f6bcc2000be18a6f15de10fc683452151741
Reviewed-on: https://boringssl-review.googlesource.com/1353
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Some phones have a buggy NEON unit and the Poly1305 NEON code fails on
them, even though other NEON code appears to work fine.
This change:
1) Fixes a bug where NEON was assumed even when the code wasn't compiled
in NEON mode.
2) Adds a second NEON control bit that can be disabled in order to run
NEON code, but not the Poly1305 NEON code.
https://code.google.com/p/chromium/issues/detail?id=341598
Change-Id: Icb121bf8dba47c7a46c7667f676ff7a4bc973625
Reviewed-on: https://boringssl-review.googlesource.com/1351
Reviewed-by: Adam Langley <agl@google.com>
This change marks public symbols as dynamically exported. This means
that it becomes viable to build a shared library of libcrypto and libssl
with -fvisibility=hidden.
On Windows, one not only needs to mark functions for export in a
component, but also for import when using them from a different
component. Because of this we have to build with
|BORINGSSL_IMPLEMENTATION| defined when building the code. Other
components, when including our headers, won't have that defined and then
the |OPENSSL_EXPORT| tag becomes an import tag instead. See the #defines
in base.h
In the asm code, symbols are now hidden by default and those that need
to be exported are wrapped by a C function.
In order to support Chromium, a couple of libssl functions were moved to
ssl.h from ssl_locl.h: ssl_get_new_session and ssl_update_cache.
Change-Id: Ib4b76e2f1983ee066e7806c24721e8626d08a261
Reviewed-on: https://boringssl-review.googlesource.com/1350
Reviewed-by: Adam Langley <agl@google.com>
Compilers have a bad habit of removing "superfluous" memset calls that
are trying to zero memory. For example, when memset()ing a buffer and
then free()ing it, the compiler might decide that the memset is
unobservable and thus can be removed.
Previously we tried to stop this by a) implementing memset in assembly
on x86 and b) putting the function in its own file for other platforms.
This change removes those tricks in favour of using asm directives to
scare the compiler away. As best as our compiler folks can tell, this is
sufficient and will continue to be so.
Change-Id: I40e0a62c3043038bafd8c63a91814a75a3c59269
Reviewed-on: https://boringssl-review.googlesource.com/1339
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Windows complains when the declaration of a function doesn't match the
definition. In this case, the |bits| argument (not a pointer, just an
unsigned) was marked as const in the definition only.
Normally const isn't used for non-pointer arguments so I've removed it
in this case to make Windows compile.
https://code.google.com/p/chromium/issues/detail?id=398960
Change-Id: If7386cf61f9dfbf6b32bfada1a49d5742fe94396
Reviewed-on: https://boringssl-review.googlesource.com/1338
Reviewed-by: Adam Langley <agl@google.com>
Caught by clang scan-build. (The allocation was larger than it should have
been.)
Change-Id: Ideb800118f65aaba1ee85b7611c8a705671025a8
Reviewed-on: https://boringssl-review.googlesource.com/1340
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>
PR#3272
(Imported from upstream's 14183e50e75f54c44df6be69670180860ac19550 and
802fdcda1ebc4241a8e02af0046ba2f5264f71f6)
Change-Id: Ied6183d938e320f953a18f6616890d88b74def3f
(Imported from upstream's 912f08dd5ed4f68fb275f3b2db828349fcffba14,
52f856526c46ee80ef4c8c37844f084423a3eff7 and
377551b9c4e12aa7846f4d80cf3604f2e396c964)
Change-Id: Ic2bf93371f6d246818729810e7a45b3f0021845a
OIDs with one component don't have an encoding.
PR#2556 (Bug#1)
(Imported from upstream's ff4cfc4c588c41d5e8d2d530231bc36cbc525add and
65e4dca40cb15f3acc878e26d734ec93bd367dca)
Change-Id: I55b54f23e891abc2c1e0b2976531fba1f16070bb
This ensures high performance is situations when assembler supports
AVX2, but not AD*X.
(Imported from upstream's 82a9dafe32e1e39b5adff18f9061e43d8df3d3c5)
Change-Id: Ie67f49a1c5467807139b6a8a0d4e62162d8a974f
Although the PKCS#1 padding check is internally constant-time, it is not
constant time at the crypto/ ssl/ API boundary. Expose a constant-time
RSA_message_index_PKCS1_type_2 function and integrate it into the
timing-sensitive portion of the RSA key exchange logic.
Change-Id: I6fa64ddc9d65564d05529d9b2985da7650d058c3
Reviewed-on: https://boringssl-review.googlesource.com/1301
Reviewed-by: Adam Langley <agl@google.com>
Due to merging our patched 1.0.1 code with the 1.0.2 code, some parts of
upstream's 25f93585a70fb05bb9f911884ab95e560f662a5d didn't make it into
the code.
https://code.google.com/p/chromium/issues/detail?id=397333
Change-Id: Iceb13e63a7ac91474fd39e7faad11fa52c56185d
Reviewed-on: https://boringssl-review.googlesource.com/1310
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Some EC ASN.1 structures are using a named curve, but include the full
parameters anyway. With this change, BoringSSL will recognise the order
of the curve.
Change-Id: Iff057178453f9fdc98c8c03bcabbccef89709887
Reviewed-on: https://boringssl-review.googlesource.com/1270
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>
More signed/unsigned issues, and some other missing checks.
Change-Id: Ib64429a609ca2d64b74a4744092aac67ad0af4e5
Reviewed-on: https://boringssl-review.googlesource.com/1252
Reviewed-by: Adam Langley <agl@google.com>
On OS X, the length must be the length of the address and not of
sockaddr_storage.
Change-Id: Id962f2f3268f07327724b9867a83c15ec50cb9fd
Reviewed-on: https://boringssl-review.googlesource.com/1251
Reviewed-by: Adam Langley <agl@google.com>
Fix base64_test.c to account for this.
Change-Id: I0b3e8062a2130fb01a7e6f175968484769c406f9
Reviewed-on: https://boringssl-review.googlesource.com/1250
Reviewed-by: Adam Langley <agl@google.com>
Appease the Chromium build on OS X.
Change-Id: Idb7466b4d3e4cc9161cd09066b2f79a6290838b1
Reviewed-on: https://boringssl-review.googlesource.com/1240
Reviewed-by: Adam Langley <agl@google.com>
Another signedness error. Leave a TODO to possibly resolve EVP_DecodeBlock's
ignoring padding. Document some of the Init/Update/Finish versions' behavior.
Change-Id: I78a72c3163f8543172a7008b2d09fb10e003d957
Reviewed-on: https://boringssl-review.googlesource.com/1230
Reviewed-by: Adam Langley <agl@google.com>
When I switched the base64 code to use size_t, I missed that one of the
loops was counting down, not up, and depended on the loop variable going
negative.
Additionally this change fixes a bug in NETSCAPE_SPKI_b64_encode where
the size of the result buffer was incorrectly calculated and a possible
memory leak.
Change-Id: Ibdf644244291274f50b314f3bb13a61b46858ca1
Reviewed-on: https://boringssl-review.googlesource.com/1220
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
These were omitted, but are needed by Chromium now.
Change-Id: I17e1672674311c8dc2ede21539c82b8e2e50f376
Reviewed-on: https://boringssl-review.googlesource.com/1201
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>
Also fix some DTLS cookie bugs. rcvd_cookie is never referenced after being
saved (and the length isn't saved, so it couldn't be used anyway), and the
cookie verification failed to check the length.
For convenience, add a CBS_mem_equal helper function. Saves a bit of
repetition.
Change-Id: I187137733b069f0ac8d8b1bf151eeb80d388b971
Reviewed-on: https://boringssl-review.googlesource.com/1174
Reviewed-by: Adam Langley <agl@google.com>
Also tidy up some variable names and update RSA_verify call for it no longer
returning -1. Add CBS helper functions for dealing with C strings.
Change-Id: Ibc398d27714744f5d99d4f94ae38210cbc89471a
Reviewed-on: https://boringssl-review.googlesource.com/1164
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>
PNaCl needs OPENSSL_NO_ASM to work and a couple of cases were missing
because it hasn't previously been tested.
Additionally, it defined _BSD_SOURCE and others on the command line,
causing duplicate definition errors when defined in source code.
It's missing readdir_r.
It uses newlib, which appears to use u_short in socket.h without ever
defining it.
Change-Id: Ieccfc7365723d0521f6327eebe9f44a2afc57406
Reviewed-on: https://boringssl-review.googlesource.com/1140
Reviewed-by: Adam Langley <agl@google.com>
This is to avoid having to copy over the RSA modulus in all of Chromium's
platform-specific keys.
Change-Id: I20bf22446a5cfb633b900c3b392b7a1da81a5431
Reviewed-on: https://boringssl-review.googlesource.com/1151
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>
Building without RSA support is unreasonable. Changes were made by
running
find . -type f -name *.c | xargs unifdef -m -U OPENSSL_NO_RSA
find . -type f -name *.h | xargs unifdef -m -U OPENSSL_NO_RSA
using unifdef 2.10 and some newlines were removed manually.
Change-Id: Iea559e2d4b3d1053f28a4a9cc2f7a3d1f6cabd61
Reviewed-on: https://boringssl-review.googlesource.com/1095
Reviewed-by: Adam Langley <agl@google.com>
Since crypto/ebcdic.{c,h} are not present in BoringSSL, remove the #ifdefs
Changes were made by running
find . -type f -name *.c | xargs unifdef -m -U CHARSET_EBCDIC
find . -type f -name *.h | xargs unifdef -m -U CHARSET_EBCDIC
using unifdef 2.10.
An additional two ifdefs (CHARSET_EBCDIC_not) were removed manually.
Change-Id: Ie174bb00782cc44c63b0f9fab69619b3a9f66d42
Reviewed-on: https://boringssl-review.googlesource.com/1093
Reviewed-by: Adam Langley <agl@google.com>
(This appears to be the case with upstream too, it's not that BoringSSL
is missing optimisations from what I can see.)
Change-Id: I0e54762ef0d09e60994ec82c5cca1ff0b3b23ea4
Reviewed-on: https://boringssl-review.googlesource.com/1080
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
It was removed in the fork but it turned out to need it.
Change-Id: I21030c8d5befecb63f2c40a59963bec1da1d96fb
Reviewed-on: https://boringssl-review.googlesource.com/1081
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
I have confirmed with Dr. Stephen Henson (the author) that the file is
licensed under the OpenSSL license.
Change-Id: I97dc4c74b363184e1b36e5835bad684d66696d54
This change adds the stitched RC4-MD5 code from upstream OpenSSL but
exposes it as an AEAD. It's not a normal AEAD (it's stateful thus
doesn't take an nonce) but forcing pre-AEAD cipher suites in the AEAD
interface is less painful than forcing AEADs into the EVP_CIPHER
interface. Over time, more and more cipher suites will be exposed as
TLS-specific AEADs and then ssl/ can drop support for EVP_CIPHER.
See original code from upstream:
https://github.com/openssl/openssl/blob/master/crypto/evp/e_rc4_hmac_md5.c
Change-Id: Ia9267b224747f02be6b934ea0b2b50e1f529fab9
Reviewed-on: https://boringssl-review.googlesource.com/1043
Reviewed-by: Adam Langley <agl@google.com>
Fixes one comment that mentioned the wrong function name. Also causes
two BN random functions to fail when the output is NULL. Previously they
would silently do nothing.
Change-Id: I89796ab855ea32787765c301a478352287e61190
This gives us systematic bounds-checking on all the parses. Also adds a
convenience function, CBS_memdup, for saving the current contents of a CBS.
Change-Id: I17dad74575f03121aee3f771037b8806ff99d0c3
Reviewed-on: https://boringssl-review.googlesource.com/1031
Reviewed-by: Adam Langley <agl@google.com>
Since all AEAD ciphers now go through EVP_AEAD interface, the code which
uses EVP_Cipher interface no longer needs any of AEAD handling logic.
This also removes EVP_CTRL_AEAD_TLS1_AAD from GCM interface, which was
duplicating non-TLS-specific GCM logic and is not used anymore.
Change-Id: I5ddae880e7bc921337f9149a0acfdd00c9a478c3
aead_test reads test vectors from a file but used blank lines to
indicate the end of a test case. If the file ended without a blank line
to terminate the final test case, it would previously have been skipped.
Change-Id: Id8dd34e86f0b912596dfb33234a894f8d9aa0235
Apart from the obvious little issues, this also works around a
(seeming) libtool/linker:
a.c defines a symbol:
int kFoo;
b.c uses it:
extern int kFoo;
int f() {
return kFoo;
}
compile them:
$ gcc -c a.c
$ gcc -c b.c
and create a dummy main in order to run it, main.c:
int f();
int main() {
return f();
}
this works as expected:
$ gcc main.c a.o b.o
but, if we make an archive:
$ ar q lib.a a.o b.o
and use that:
$ gcc main.c lib.a
Undefined symbols for architecture x86_64
"_kFoo", referenced from:
_f in lib.a(b.o)
(It doesn't matter what order the .o files are put into the .a)
Linux and Windows don't seem to have this problem.
nm on a.o shows that the symbol is of type "C", which is a "common symbol"[1].
Basically the linker will merge multiple common symbol definitions together.
If ones makes a.c read:
int kFoo = 0;
Then one gets a type "D" symbol - a "data section symbol" and everything works
just fine.
This might actually be a libtool bug instead of an ld bug: Looking at `xxd
lib.a | less`, the __.SYMDEF SORTED index at the beginning of the archive
doesn't contain an entry for kFoo unless initialised.
Change-Id: I4cdad9ba46e9919221c3cbd79637508959359427
Now that the consuming code in ssl/ is removed, there is no need for this.
Leave SSL_COMP and STACK_OF(SSL_COMP) for now so as not to break any code which
manipulates the output of SSL_COMP_get_compression_methods to disable
compression.
Change-Id: Idf0a5debd96589ef6e7e56acf5d9259412b7d7a1
Previously we generated a number that was 8 bytes too large and used a
modular reduction, which has a (tiny, tiny) bias towards zero.
Out of an excess of caution, instead truncate the generated nonce and
try again if it's out of range.
Change-Id: Ia9a7a57dd6d3e5f13d0b881b3e9b2e986d46e4ca
The function was hard-coded to 20 rounds already so the argument was
already useless. Thanks to Huzaifa Sidhpurwala for noticing.
Change-Id: I5f9d6ca6d46c6ab769b19820f8f918349544846d
Ensure the library can find the right files under /etc/ssl/certs when
running on older systems.
There are many symbolic links under /etc/ssl/certs created by using
hash of the PEM certificates in order for OpenSSL to find those
certificates. Openssl has a tool to help you create hash symbolic
links (tools/c_rehash). However newer versions of the library changed
the hash algorithm, which makes it unable to run properly on systems
that use the old /etc/ssl/certs layout (e.g. Ubuntu Lucid).
This patch gives a way to find a certificate according to its hash by
using both the old and new algorithms. http://crbug.com/111045 is used
to track this issue.
(Imported from Chromium:
http://src.chromium.org/viewvc/chrome/trunk/deps/third_party/openssl/patches.chromium/0003-x509_hash_name_algorithm_change.patch?revision=231571)
Change-Id: Idbc27aba7685c991f8b94cfea38cf4f3f4b38adc
Windows has different names for these functions and also doesn't have
the strings.h header in which they appear.
This change adds tiny wrapper functions for Windows.
A client reference identity of ".example.com" matches a server certificate
presented identity that is any sub-domain of "example.com" (e.g.
"www.sub.example.com).
With the X509_CHECK_FLAG_SINGLE_LABEL_SUBDOMAINS flag, it matches only direct
child sub-domains (e.g. "www.sub.example.com"). (cherry picked from commit
e52c52f10bb8e34aaf8f28f3e5b56939e8f6b357)
(Imported from upstream's 3cc8a3f2343cda796de90c127b9e907ca3ec2da5)
Fixes to host checking wild card support and add support for setting
host checking flags when verifying a certificate chain.
(Imported from upstream's a2219f6be36d12f02b6420dd95f819cf364baf1d)
(Imported from upstream's 4ceb430a468e8226175aa3f169c0e746877c17e1,
4f7236edc7d5c384bdb148faf7b23f887cf18f69 and
ed693e43329383c0d68455d83778cdc9748a074d)
The lazy-initialisation of BN_MONT_CTX was serialising all threads, as noted by
Daniel Sands and co at Sandia. This was to handle the case that 2 or more
threads race to lazy-init the same context, but stunted all scalability in the
case where 2 or more threads are doing unrelated things! We favour the latter
case by punishing the former. The init work gets done by each thread that finds
the context to be uninitialised, and we then lock the "set" logic after that
work is done - the winning thread's work gets used, the losing threads throw
away what they've done.
(Imported from upstream's bf43446835bfd3f9abf1898a99ae20f2285320f3)
It's not clear whether this inconsistency could lead to an actual
computation error, but it involved a BIGNUM being passed around the
montgomery logic in an inconsistent state. This was found using flags
-DBN_DEBUG -DBN_DEBUG_RAND, and working backwards from this assertion
in 'ectest';
ectest: bn_mul.c:960: BN_mul: Assertion `(_bnum2->top == 0) ||
(_bnum2->d[_bnum2->top - 1] != 0)' failed
(Imported from upstream's 3cc546a3bbcbf26cd14fc45fb133d36820ed0a75)
Câmara, D.; Gouvêa, C. P. L.; López, J. & Dahab, R.: Fast Software
Polynomial Multiplication on ARM Processors using the NEON Engine.
http://conradoplg.cryptoland.net/files/2010/12/mocrysen13.pdf
(Imported from upstream's 0fb3d5b4fdc76b8d4a4700d03480cda135c6c117)
When looking for an extension we need to set the last found
position to -1 to properly search all extensions.
PR#3309
(Imported from upstream's 5cd5e0219d2e9a8c1f2fec3d867f38179c3a86af)
Improve CBC decrypt and CTR by ~13/16%, which adds up to ~25/33%
improvement over "pre-Silvermont" version. [Add performance table to
aesni-x86.pl].
(Imported from upstream's b347341c75656cf8bc039bd0ea5e3571c9299687)
Include self-signed flag in certificates by checking SKID/AKID as well as
issuer and subject names. Although this is an incompatible change it should
have little impact in pratice because self-issued certificates that are not
self-signed are rarely encountered.
(Imported from upstream's c00f8d697aed17edbd002e2f6c989d8fbd7c4ecf)
When a chain is complete and ends in a trusted root checks are also performed
on the TA and the callback notified with ok==1. For consistency do the same for
chains where the TA is not self signed.
(Imported from upstream's b07e4f2f46fc286c306353d5e362cbc22c8547fb)
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.
Fix a bug in handling of 128 byte long PSK identity in
psk_client_callback.
OpenSSL supports PSK identities of up to (and including) 128 bytes in
length. PSK identity is obtained via the psk_client_callback,
implementors of which are expected to provide a NULL-terminated
identity. However, the callback is invoked with only 128 bytes of
storage thus making it impossible to return a 128 byte long identity and
the required additional NULL byte.
This CL fixes the issue by passing in a 129 byte long buffer into the
psk_client_callback. As a safety precaution, this CL also zeroes out the
buffer before passing it into the callback, uses strnlen for obtaining
the length of the identity returned by the callback, and aborts the
handshake if the identity (without the NULL terminator) is longer than
128 bytes.
https://rt.openssl.org/Ticket/Display.html?id=2608
Previously, this input to the base64 code:
================================================================================-
Would cause the output length of EVP_DecodeUpdate to be negative. When
that happened in the base64 BIO, it would crash. In PEM decoding, the
ASN.1 code actually maintains signed lengths and manages to simply error
out!
Fix handling of points at infinity in ec_GFp_simple_points_make_affine.
When inverting an array of Z coordinates, the algorithm is supposed to
treat any 0 essentially like a 1 to remain in the multiplicative group;
however, for one of the cases, we incorrectly multiplied by 0 and thus
ended up with garbage.
This change adjusts the stack pointer during CBC decryption. The code
was previously using the red zone across function calls and valgrind
thinks that the "unused" stack is undefined after a function call.
Some RSA private keys are specified with only n, e and d. Although we
can use these keys directly, it's nice to have a uniform representation
that includes the precomputed CRT values. This change adds a function
that can recover the primes from a minimal private key of that form.
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-----
Ensure that, when generating small primes, the result is actually of the
requested size. Fixes OpenSSL #2701.
This change does not address the cases of generating safe primes, or
where the |add| parameter is non-NULL.
(The issue was reported by Shay Gueron.)
The final reduction in Montgomery multiplication computes if (X >= m) then X =
X - m else X = X
In OpenSSL, this was done by computing T = X - m, doing a constant-time
selection of the *addresses* of X and T, and loading from the resulting
address. But this is not cache-neutral.
This patch changes the behaviour by loading both X and T into registers, and
doing a constant-time selection of the *values*.
TODO(fork): only some of the fixes from the original patch still apply to
the 1.0.2 code.
Initial fork from f2d678e6e89b6508147086610e985d4e8416e867 (1.0.2 beta).
(This change contains substantial changes from the original and
effectively starts a new history.)
