Welcome to mirror list, hosted at ThFree Co, Russian Federation.

github.com/mono/boringssl.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2016-07-12Change |EVP_PKEY_up_ref| to return int.Adam Langley
Upstream have added |EVP_PKEY_up_ref|, but their version returns an int. Having this function with a different signature like that is dangerous so this change aligns BoringSSL with upstream. Users of this function in Chromium and internally should already have been updated. Change-Id: I0a7aeaf1a1ca3b0f0c635e2ee3826aa100b18157 Reviewed-on: https://boringssl-review.googlesource.com/8736 Reviewed-by: David Benjamin <davidben@google.com>
2016-07-12Revert "Move C++ helpers into |bssl| namespace."Adam Langley
This reverts commit 09feb0f3d95a2bc58ce0faaf14256d3bd30f52a4. (In order to make WebRTC happy this also needs to be reverted.)
2016-07-12Revert scoped_types.h change.Adam Langley
This reverts commits: 8d79ed67407e433d80ebc8d3aa080e8ce16e6eb7 19fdcb523402ed13ab798cf811fb0119e3e7b104 8d79ed67407e433d80ebc8d3aa080e8ce16e6eb7 Because WebRTC (at least) includes our headers in an extern "C" block, which precludes having any C++ in them. Change-Id: Ia849f43795a40034cbd45b22ea680b51aab28b2d
2016-07-12Remove scoped_types.h.Adam Langley
This change scatters the contents of the two scoped_types.h files into the headers for each of the areas of the code. The types are now in the |bssl| namespace. Change-Id: I802b8de68fba4786b6a0ac1bacd11d81d5842423 Reviewed-on: https://boringssl-review.googlesource.com/8731 Reviewed-by: Adam Langley <agl@google.com>
2016-07-12Move C++ helpers into |bssl| namespace.Adam Langley
We currently have the situation where the |tool| and |bssl_shim| code includes scoped_types.h from crypto/test and ssl/test. That's weird and shouldn't happen. Also, our C++ consumers might quite like to have access to the scoped types. Thus this change moves some of the template code to base.h and puts it all in a |bssl| namespace to prepare for scattering these types into their respective headers. In order that all the existing test code be able to access these types, it's all moved into the same namespace. Change-Id: I3207e29474dc5fcc344ace43119df26dae04eabb Reviewed-on: https://boringssl-review.googlesource.com/8730 Reviewed-by: David Benjamin <davidben@google.com>
2016-06-16Unwind X509_LU_RETRY and fix a lot of type confusion.David Benjamin
(This change will be sent upstream. Since the legacy X.509 stack is just kept around for compatibility, if they decide to fix it in a different way, we may wish to revert this and apply their fix.) Dating back to SSLeay, X509_LOOKUP_METHOD had this X509_LU_RETRY machinery. But it's not documented and it appears to have never worked. Problems with the existing logic: - X509_LU_* is not sure whether it is a type enum (to be passed into X509_LOOKUP_by_*) or a return enum (to be retained by those same functions). - X509_LOOKUP_by_* is not sure whether it returns 0/1 or an X509_LU_* value. Looking at the functions themselves, one might think it's the latter, but for X509_LOOKUP_by_subject returning both 0 and X509_LU_FAIL. But looking at the call sites, some expect 0/1 (such as X509_STORE_get1_certs) while others expect an X509_LU_* enum (such as X509_STORE_CTX_get1_issuer). It is very fortunate that FAIL happens to be 0 and X509 happens to be 1. These functions primarily call to X509_LOOKUP_METHOD hooks. Looking through OpenSSL itself and code checked into Google, I found no evidence that any hooks have been implemented except for get_by_subject in by_dir.c. We take that one as definitive and observe it believes it returns 0/1. Notably, it returns 1 on success even if asked for a type other than X509_LU_X509. (X509_LU_X509 = 1. Others are different.) I found another piece of third-party software which corroborates this worldview. - X509_STORE_get_by_subject's handling of X509_LU_RETRY (it's the j < 0 check) is broken. It saves j into vs->current_method where it probably meant to save i. (This bug has existed since SSLeay.) It also returns j (supposedly X509_LU_RETRY) while all callers of X509_STORE_get_by_subject expect it to return 0/1 by checking with ! instead of <= 0. (Note that all other codepaths return 0 and 1 so this function did not actually believe it returned X509_LU_* most of the time.) This, in turn, gives us a free of uninitialized pointers in X509_STORE_get1_certs and other functions which expect that *ret is filled in if X509_STORE_get_by_subject returns success. GCC 4.9 with optimizations from the Android NDK noticed this, which trigged this saga. (It's only reachable if any X509_LOOKUP_METHOD returned X509_LU_RETRY.) - Although the code which expects X509_STORE_get_by_subject return 0/1 does not date to SSLeay, the X509_STORE_get_by_subject call in X509_STORE_CTX_get1_issuer *does* (though, at the time, it was inline in X509_verify_cert. That code believes X509_STORE_get_by_subject returns an X509_LU_* enum, but it doesn't work either! It believes *ret is filled in on X509_LU_RETRY, thus freeing another uninitialized pointer (GCC noticed this too). Since this "retry" code has clearly never worked, from SSLeay onwards, unwind it completely rather than attempt to fix it. No X509_LOOKUP_METHOD can possibly have depended on it. Matching all non-broken codepaths X509_LOOKUP_by_* now returns 0/1 and X509_STORE_get_by_subject returns 0/1. X509_LU_* is purely a type enum with X509_LU_{REJECT,FAIL} being legacy constants to keep old code compiling. (Upstream is recommended to remove those values altogether for 1.1.0.) On the off chance any get_by_* X509_LOOKUP_METHOD implementations did not return 0/1 (I have found no evidence anywhere of this, and I believe it wouldn't have worked anyway), the X509_LOOKUP_by_* wrapper functions will coerce the return values back to 0/1 before passing up to the callers which want 0/1. This both avoids the error-prone -1/0/1 calling convention and, more importantly, avoids problems with third-party callers which expect a X509_LU_* return code. 0/1 collide with FAIL/X509 while -1 will collide with RETRY and might confuse things. Change-Id: I98ecf6fa7342866b9124dc6f0b422cb9ce4a1ae7 Reviewed-on: https://boringssl-review.googlesource.com/8303 Reviewed-by: Adam Langley <agl@google.com>
2016-06-14Remove ASN.1 print hooks.David Benjamin
These functions are never instantiated. (They're a remnant of the PKCS#7 and CMS bits.) Next time upstream touches this code, we don't have to puzzle through the diff and import it. Change-Id: I67c2102ae13e8e0527d858e1c63637dd442a4ffb Reviewed-on: https://boringssl-review.googlesource.com/8242 Reviewed-by: David Benjamin <davidben@google.com>
2016-06-14Avoid illegal pointers in asn1_string_canon.David Benjamin
(Imported from upstream's 3892b95750b6aa5ed4328a287068f7cdfb9e55bc.) More reasonable would have been to drop |to| altogether and act on from[len-1], but I suppose this works. Change-Id: I280b4991042b4d330ba034f6a631f8421ddb2643 Reviewed-on: https://boringssl-review.googlesource.com/8241 Reviewed-by: Adam Langley <agl@google.com>
2016-06-09Ensure verify error is set when X509_verify_cert() fails.David Benjamin
Set ctx->error = X509_V_ERR_OUT_OF_MEM when verification cannot continue due to malloc failure. Similarly for issuer lookup failures and caller errors (bad parameters or invalid state). Also, when X509_verify_cert() returns <= 0 make sure that the verification status does not remain X509_V_OK, as a last resort set it it to X509_V_ERR_UNSPECIFIED, just in case some code path returns an error without setting an appropriate value of ctx->error. Add new and some missing error codes to X509 error -> SSL alert switch. (Imported from upstream's 5553a12735e11bc9aa28727afe721e7236788aab.) Change-Id: I3231a6b2e72a3914cb9316b8e90ebaee009a1c5f Reviewed-on: https://boringssl-review.googlesource.com/8170 Reviewed-by: David Benjamin <davidben@google.com>
2016-06-01Split unlock functions into read/write variants.David Benjamin
Windows SRWLOCK requires you call different functions here. Split them up in preparation for switching Windows from CRITICAL_SECTION. BUG=37 Change-Id: I7b5c6a98eab9ae5bb0734b805cfa1ff334918f35 Reviewed-on: https://boringssl-review.googlesource.com/8080 Reviewed-by: Adam Langley <agl@google.com>
2016-05-13Make i2d_X509_AUX work if *pp = NULL.David Benjamin
When *pp is NULL, don't write garbage, return an unexpected pointer or leak memory on error. (Imported from upstream's 36c37944909496a123e2656ad1f651769a7cc72f.) This calling convention... Change-Id: Ic733092cfb942a3e1d3ceda6797222901ad55bef Reviewed-on: https://boringssl-review.googlesource.com/7944 Reviewed-by: Adam Langley <agl@google.com>
2016-05-03Add size limit to X509_NAME structure.David Benjamin
This adds an explicit limit to the size of an X509_NAME structure. Some part of OpenSSL (e.g. TLS) already effectively limit the size due to restrictions on certificate size. See also upstream's 65cb92f4da37a3895437f0c9940ee0bcf9f28c8a, although this is different from upstream's. Upstream's version bounds both the X509_NAME *and* any data after it in the immediately containing structure. While adding a bound on all of crypto/asn1 is almost certainly a good idea (will look into that for a follow-up), it seems bizarre and unnecessary to have X509_NAME affect its parent. Change-Id: Ica2136bcd1455d7c501ccc6ef2a19bc5ed042501 Reviewed-on: https://boringssl-review.googlesource.com/7846 Reviewed-by: Adam Langley <agl@google.com>
2016-05-03Sync with upstream on i2d_X509_AUX.David Benjamin
Upstream decided to reset *pp on error and to later fix up the other i2d functions to behave similarly. See upstream's c5e603ee182b40ede7713c6e229c15a8f3fdb58a. Change-Id: I01f82b578464060d0f2be5460fe4c1b969124c8e Reviewed-on: https://boringssl-review.googlesource.com/7844 Reviewed-by: Adam Langley <agl@google.com>
2016-05-03Add checks to X509_NAME_oneline()David Benjamin
Sanity check field lengths and sums to avoid potential overflows and reject excessively large X509_NAME structures. Issue reported by Guido Vranken. (Imported from upstream's 9b08619cb45e75541809b1154c90e1a00450e537.) Change-Id: Ib2e1e7cd086f9c3f0d689d61947f8ec3e9220049 Reviewed-on: https://boringssl-review.googlesource.com/7842 Reviewed-by: Adam Langley <agl@google.com>
2016-05-03Sanity check buffer length.David Benjamin
Reject zero length buffers passed to X509_NAME_oneline(). Issue reported by Guido Vranken. (Imported from upstream's 66e731ab09f2c652d0e179df3df10d069b407604.) Tweaked slightly to use <= 0 instead of == 0 since the length is signed. Change-Id: I5ee54d77170845e4699fda7df5e94538c8e55ed9 Reviewed-on: https://boringssl-review.googlesource.com/7841 Reviewed-by: Adam Langley <agl@google.com>
2016-05-02Start assuming MSVC 2015.David Benjamin
BUG=43 Change-Id: I46ad1ca62b8921a03fae51f5d7bbe1c68fc0b170 Reviewed-on: https://boringssl-review.googlesource.com/7821 Reviewed-by: Steven Valdez <svaldez@google.com> Reviewed-by: David Benjamin <davidben@google.com>
2016-04-29Fix d2i_X509_AUX.David Benjamin
The logic to reset *pp doesn't actually work if pp is NULL. (It also doesn't work if *pp is NULL, but that didn't work before either.) Don't bother resetting it. This is consistent with the template-based i2d functions which do not appear to leave *pp alone on error. Will send this upstream. Change-Id: I9fb5753e5d36fc1d490535720b8aa6116de69a70 Reviewed-on: https://boringssl-review.googlesource.com/7812 Reviewed-by: Adam Langley <agl@google.com>
2016-04-26Ensure we check i2d_X509 return valSteven Valdez
The i2d_X509() function can return a negative value on error. Therefore we should make sure we check it. Issue reported by Yuan Jochen Kang. (Imported from upstream's 8f43c80bfac15544820739bf035df946eeb603e8) Change-Id: If247d5bf1d792eb7c6dc179b606ed21ea0ccdbb8 Reviewed-on: https://boringssl-review.googlesource.com/7743 Reviewed-by: David Benjamin <davidben@google.com>
2016-03-28Fix some malloc test failures.David Benjamin
These only affect the tests. Change-Id: If22d047dc98023501c771787b485276ece92d4a2 Reviewed-on: https://boringssl-review.googlesource.com/7573 Reviewed-by: Steven Valdez <svaldez@google.com> Reviewed-by: David Benjamin <davidben@google.com>
2016-03-26Fix build when using Visual Studio 2015 Update 1.Brian Smith
Many of the compatibility issues are described at https://msdn.microsoft.com/en-us/library/mt612856.aspx. The macros that suppressed warnings on a per-function basis no longer work in Update 1, so replace them with #pragmas. Update 1 warns when |size_t| arguments to |printf| are casted, so stop doing that casting. Unfortunately, this requires an ugly hack to continue working in MSVC 2013 as MSVC 2013 doesn't support "%zu". Finally, Update 1 has new warnings, some of which need to be suppressed. --- Updated by davidben to give up on suppressing warnings in crypto/x509 and crypto/x509v3 as those directories aren't changed much from upstream. In each of these cases, upstream opted just blindly initialize the variable, so do the same. Also switch C4265 to level 4, per Microsoft's recommendation and work around a bug in limits.h that happens to get fixed by Google include order style. (limits.h is sensitive to whether corecrt.h, pulled in by stddef.h and some other headers, is included before it. The reason it affected just one file is we often put the file's header first, which means base.h is pulling in stddef.h. Relying on this is ugly, but it's no worse than what everything else is doing and this doesn't seem worth making something as tame as limits.h so messy to use.) Change-Id: I02d1f935356899f424d3525d03eca401bfa3e6cd Reviewed-on: https://boringssl-review.googlesource.com/7480 Reviewed-by: David Benjamin <davidben@google.com>
2016-03-17Don't shift serial number into sign bitDavid Benjamin
(Imported from upstream's 01c32b5e448f6d42a23ff16bdc6bb0605287fa6f.) Change-Id: Ib52278dbbac1ed1ad5c80f0ad69e34584d411cec Reviewed-on: https://boringssl-review.googlesource.com/7461 Reviewed-by: Steven Valdez <svaldez@google.com> Reviewed-by: David Benjamin <davidben@google.com>
2016-03-15Align with upstream's error strings, take two.David Benjamin
I messed up a few of these. ASN1_R_UNSUPPORTED_ALGORITHM doesn't exist. X509_R_UNSUPPORTED_ALGORITHM does exist as part of X509_PUBKEY_set, but the SPKI parser doesn't emit this. (I don't mind the legacy code having really weird errors, but since EVP is now limited to things we like, let's try to keep that clean.) To avoid churn in Conscrypt, we'll keep defining X509_R_UNSUPPORTED_ALGORITHM, but not actually do anything with it anymore. Conscrypt was already aware of EVP_R_UNSUPPORTED_ALGORITHM, so this should be fine. (I don't expect EVP_R_UNSUPPORTED_ALGORITHM to go away. The SPKI parsers we like live in EVP now.) A few other ASN1_R_* values didn't quite match upstream, so make those match again. Finally, I got some of the rsa_pss.c values wrong. Each of those corresponds to an (overly specific) RSA_R_* value in upstream. However, those were gone in BoringSSL since even the initial commit. We placed the RSA <-> EVP glue in crypto/evp (so crypto/rsa wouldn't depend on crypto/evp) while upstream placed them in crypto/rsa. Since no one seemed to notice the loss of RSA_R_INVALID_SALT_LENGTH, let's undo all the cross-module errors inserted in crypto/rsa. Instead, since that kind of specificity is not useful, funnel it all into X509_R_INVALID_PSS_PARAMETERS (formerly EVP_R_INVALID_PSS_PARAMETERS, formerly RSA_R_INVALID_PSS_PARAMETERS). Reset the error codes for all affected modules. (That our error code story means error codes are not stable across this kind of refactoring is kind of a problem. Hopefully this will be the last of it.) Change-Id: Ibfb3a0ac340bfc777bc7de6980ef3ddf0a8c84bc Reviewed-on: https://boringssl-review.googlesource.com/7458 Reviewed-by: Emily Stark (Dunn) <estark@google.com> Reviewed-by: David Benjamin <davidben@google.com>
2016-03-12Match upstream's error codes for the old sigalg code.David Benjamin
People seem to condition on these a lot. Since this code has now been moved twice, just make them all cross-module errors rather than leave a trail of renamed error codes in our wake. Change-Id: Iea18ab3d320f03cf29a64a27acca119768c4115c Reviewed-on: https://boringssl-review.googlesource.com/7431 Reviewed-by: Emily Stark (Dunn) <estark@google.com> Reviewed-by: David Benjamin <davidben@google.com>
2016-03-02Bring back |verify_store|.Adam Langley
This was dropped in d27441a9cb55b02149d7f1236de94f3a40dd1692 due to lack of use, but node.js now needs it. Change-Id: I1e207d4b46fc746cfae309a0ea7bbbc04ea785e8 Reviewed-on: https://boringssl-review.googlesource.com/7270 Reviewed-by: David Benjamin <davidben@google.com>
2016-02-27Move all signature algorithm code to crypto/x509.David Benjamin
All the signature algorithm logic depends on X509_ALGOR. This also removes the X509_ALGOR-based EVP functions which are no longer used externally. I think those APIs were a mistake on my part. The use in Chromium was unnecessary (and has since been removed anyway). The new X.509 stack will want to process the signatureAlgorithm itself to be able to enforce policies on it. This also moves the RSA_PSS_PARAMS bits to crypto/x509 from crypto/rsa. That struct is also tied to crypto/x509. Any new RSA-PSS code would have to use something else anyway. BUG=499653 Change-Id: I6c4b4573b2800a2e0f863d35df94d048864b7c41 Reviewed-on: https://boringssl-review.googlesource.com/7025 Reviewed-by: Adam Langley <agl@google.com>
2016-02-27Move X.509 signature algorithm tests to the crypto/x509 layer.David Benjamin
This is in preparation for moving the logic itself to crypto/x509, so the lower-level functions will not be as readily available. Change-Id: I6507b895317df831ab11d0588c5b09bbb2aa2c24 Reviewed-on: https://boringssl-review.googlesource.com/7023 Reviewed-by: Adam Langley <agl@google.com>
2016-02-25Fix missing ok=0 with cert verification.Steven Valdez
Also avoid using "i" in X509_cert_verify as a loop counter, trust outcome and as an error ordinal. (Imported from upstream's a3baa171053547488475709c7197592c66e427cf) Change-Id: I4b0b542ffacf7fa861c93c8124b334c0aacc3c17 Reviewed-on: https://boringssl-review.googlesource.com/7222 Reviewed-by: David Benjamin <davidben@google.com>
2016-02-25Revert "Fix missing ok=0 with cert verification."David Benjamin
This reverts commit b0576889fa4c86a8e9cb7e978e7160904fa2c5b4. This broke x509_test. Change-Id: Idbb60df9ca0a8ce727931f8e35e99bbd0f08c3c1 Reviewed-on: https://boringssl-review.googlesource.com/7221 Reviewed-by: David Benjamin <davidben@google.com>
2016-02-25Fix missing ok=0 with cert verification.Steven Valdez
Also avoid using "i" in X509_cert_verify as a loop counter, trust outcome and as an error ordinal. (Imported from upstream's a3baa171053547488475709c7197592c66e427cf) Change-Id: I492afdbaa5017bcf00a0412033cf99fca3fe9401 Reviewed-on: https://boringssl-review.googlesource.com/7218 Reviewed-by: David Benjamin <davidben@google.com>
2016-02-24BIO_new_mem_buf should take const void *Steven Valdez
BIO_FLAGS_MEM_RDONLY keeps the invariant. (Imported from upstream's a38a159bfcbc94214dda00e0e6b1fc6454a23b78) Change-Id: I4cb35615d76b77929915e370dbb7fec1455da069 Reviewed-on: https://boringssl-review.googlesource.com/7214 Reviewed-by: David Benjamin <davidben@google.com>
2016-02-17Slightly simplify and deprecate i2d_{Public,Private}Key.David Benjamin
There are all the type-specific serializations rather than something tagged with a type. i2d_PrivateKey's PKCS#8 codepath was unreachable because every EVP_PKEY type has an old_priv_encode function. To prune EVP_PKEY_ASN1_METHOD further, replace i2d_PrivateKey into a switch case so we don't need to keep old_priv_encode around. This cuts down on a case of outside modules reaching into crypto/evp method tables. Change-Id: I30db2eed836d560056ba9d1425b960d0602c3cf2 Reviewed-on: https://boringssl-review.googlesource.com/6865 Reviewed-by: Adam Langley <agl@google.com>
2016-02-17Implement new SPKI parsers.David Benjamin
Many consumers need SPKI support (X.509, TLS, QUIC, WebCrypto), each with different ways to set signature parameters. SPKIs themselves can get complex with id-RSASSA-PSS keys which come with various constraints in the key parameters. This suggests we want a common in-library representation of an SPKI. This adds two new functions EVP_parse_public_key and EVP_marshal_public_key which converts EVP_PKEY to and from SPKI and implements X509_PUBKEY functions with them. EVP_PKEY seems to have been intended to be able to express the supported SPKI types with full-fidelity, so these APIs will continue this. This means future support for id-RSASSA-PSS would *not* repurpose EVP_PKEY_RSA. I'm worried about code assuming EVP_PKEY_RSA implies acting on the RSA* is legal. Instead, it'd add an EVP_PKEY_RSA_PSS and the data pointer would be some (exposed, so the caller may still check key size, etc.) RSA_PSS_KEY struct. Internally, the EVP_PKEY_CTX implementation would enforce the key constraints. If RSA_PSS_KEY would later need its own API, that code would move there, but that seems unlikely. Ideally we'd have a 1:1 correspondence with key OID, although we may have to fudge things if mistakes happen in standardization. (Whether or not X.509 reuses id-ecPublicKey for Ed25519, we'll give it a separate EVP_PKEY type.) DSA parsing hooks are still implemented, missing parameters and all for now. This isn't any worse than before. Decoupling from the giant crypto/obj OID table will be a later task. BUG=522228 Change-Id: I0e3964edf20cb795a18b0991d17e5ca8bce3e28c Reviewed-on: https://boringssl-review.googlesource.com/6861 Reviewed-by: Adam Langley <agl@google.com>
2016-01-21Fix build of x509_test.David Benjamin
Some combination of Chromium's copy of clang and Chromium's Linux sysroot doesn't like syntax. It complains that "chosen constructor is explicit in copy-initialization". Change-Id: Ied6bc17b19421998f926483742510c81f732566b Reviewed-on: https://boringssl-review.googlesource.com/6930 Reviewed-by: Adam Langley <agl@google.com>
2016-01-19Import “altchains” support.Adam Langley
This change imports the following changes from upstream: 6281abc79623419eae6a64768c478272d5d3a426 dfd3322d72a2d49f597b86dab6f37a8cf0f26dbf f34b095fab1569d093b639bfcc9a77d6020148ff 21376d8ae310cf0455ca2b73c8e9f77cafeb28dd 25efcb44ac88ab34f60047e16a96c9462fad39c1 56353962e7da7e385c3d577581ccc3015ed6d1dc 39c76ceb2d3e51eaff95e04d6e4448f685718f8d a3d74afcae435c549de8dbaa219fcb30491c1bfb These contain the “altchains” functionality which allows OpenSSL to backtrack when chain building. Change-Id: I8d4bc2ac67b90091f9d46e7355cae878b4ccf37d Reviewed-on: https://boringssl-review.googlesource.com/6905 Reviewed-by: Adam Langley <agl@google.com>
2016-01-19OpenSSL reformat x509/, x509v3/, pem/ and asn1/.Adam Langley
OpenSSL upstream did a bulk reformat. We still have some files that have the old OpenSSL style and this makes applying patches to them more manual, and thus more error-prone, than it should be. This change is the result of running util/openssl-format-source -v -c . in the enumerated directories. A few files were in BoringSSL style and have not been touched. This change should be formatting only; no semantic difference. Change-Id: I75ced2970ae22b9facb930a79798350a09c5111e Reviewed-on: https://boringssl-review.googlesource.com/6904 Reviewed-by: David Benjamin <davidben@chromium.org> Reviewed-by: Adam Langley <agl@google.com>
2015-12-22Resolve a few old TODOs.David Benjamin
A lot of commented-out code we haven't had to put them back, so these can go now. Also remove the TODO about OAEP having a weird API. The API is wrong, but upstream's shipped it with the wrong API, so that's what it is now. Change-Id: I7da607cf2d877cbede41ccdada31380f812f6dfa Reviewed-on: https://boringssl-review.googlesource.com/6763 Reviewed-by: Adam Langley <agl@google.com>
2015-12-22Reject calls to X509_verify_cert that have not been reinitialisedDavid Benjamin
The function X509_verify_cert checks the value of |ctx->chain| at the beginning, and if it is NULL then it initialises it, along with the value of |ctx->untrusted|. The normal way to use X509_verify_cert() is to first call X509_STORE_CTX_init(); then set up various parameters etc; then call X509_verify_cert(); then check the results; and finally call X509_STORE_CTX_cleanup(). The initial call to X509_STORE_CTX_init() sets |ctx->chain| to NULL. The only place in the OpenSSL codebase where |ctx->chain| is set to anything other than a non NULL value is in X509_verify_cert itself. Therefore the only ways that |ctx->chain| could be non NULL on entry to X509_verify_cert is if one of the following occurs: 1) An application calls X509_verify_cert() twice without re-initialising in between. 2) An application reaches inside the X509_STORE_CTX structure and changes the value of |ctx->chain| directly. With regards to the second of these, we should discount this - it should not be supported to allow this. With regards to the first of these, the documentation is not exactly crystal clear, but the implication is that you must call X509_STORE_CTX_init() before each call to X509_verify_cert(). If you fail to do this then, at best, the results would be undefined. Calling X509_verify_cert() with |ctx->chain| set to a non NULL value is likely to have unexpected results, and could be dangerous. This commit changes the behaviour of X509_verify_cert() so that it causes an error if |ctx->chain| is anything other than NULL (because this indicates that we have not been initialised properly). It also clarifies the associated documentation. (Imported from upstream's 692f07c3e0c04180b56febc2feb57cd94395a7a2.) Change-Id: I971f1a305f12bbf9f4ae955313d5557368f0d374 Reviewed-on: https://boringssl-review.googlesource.com/6760 Reviewed-by: Adam Langley <agl@google.com>
2015-12-16Remove the CRYPTO_EX_new callback.David Benjamin
This callback is never used. The one caller I've ever seen is in Android code which isn't built with BoringSSL and it was a no-op. It also doesn't actually make much sense. A callback cannot reasonably assume that it sees every, say, SSL_CTX created because the index may be registered after the first SSL_CTX is created. Nor is there any point in an EX_DATA consumer in one file knowing about an SSL_CTX created in completely unrelated code. Replace all the pointers with a typedef to int*. This will ensure code which passes NULL or 0 continues to compile while breaking code which passes an actual function. This simplifies some object creation functions which now needn't worry about CRYPTO_new_ex_data failing. (Also avoids bouncing on the lock, but it's taking a read lock, so this doesn't really matter.) BUG=391192 Change-Id: I02893883c6fa8693682075b7b130aa538a0a1437 Reviewed-on: https://boringssl-review.googlesource.com/6625 Reviewed-by: Adam Langley <agl@google.com>
2015-11-17Check for overflow when parsing a CBS with d2i_*.David Benjamin
Until we've done away with the d2i_* stack completely, boundaries need to be mindful of the type mismatch. d2i_* takes a long, not a size_t. Change-Id: If02f9ca2cfde02d0929ac18275d09bf5df400f3a Reviewed-on: https://boringssl-review.googlesource.com/6491 Reviewed-by: Adam Langley <agl@google.com>
2015-11-12Become partially -Wmissing-variable-declarations-clean.David Benjamin
There's a few things that will be kind of a nuisance and possibly not worth it (crypto/asn1 dumps a lot of undeclared things, etc.). But it caught some mistakes. Even without the warning, making sure to include the externs before defining a function helps catch type mismatches. Change-Id: I3dab282aaba6023e7cebc94ed7a767a5d7446b08 Reviewed-on: https://boringssl-review.googlesource.com/6484 Reviewed-by: Adam Langley <agl@google.com>
2015-10-28Fix all sign/unsigned warnings with Clang and GCC.Adam Langley
Change-Id: If2a83698236f7b0dcd46701ccd257a85463d6ce5 Reviewed-on: https://boringssl-review.googlesource.com/4992 Reviewed-by: Adam Langley <agl@google.com>
2015-10-26Add a run_tests target to run all tests.David Benjamin
It's very annoying having to remember the right incant every time I want to switch around between my build, build-release, build-asan, etc., output directories. Unfortunately, this target is pretty unfriendly without CMake 3.2+ (and Ninja 1.5+). This combination gives a USES_TERMINAL flag to add_custom_target which uses Ninja's "console" pool, otherwise the output buffering gets in the way. Ubuntu LTS is still on an older CMake, so do a version check in the meantime. CMake also has its own test mechanism (CTest), but this doesn't use it. It seems to prefer knowing what all the tests are and then tries to do its own output management and parallelizing and such. We already have our own runners. all_tests.go could actually be converted tidily, but generate_build_files.py also needs to read it, and runner.go has very specific needs. Naming the target ninja -C build test would be nice, but CTest squats that name and CMake grumps when you use a reserved name, so I've gone with run_tests. Change-Id: Ibd20ebd50febe1b4e91bb19921f3bbbd9fbcf66c Reviewed-on: https://boringssl-review.googlesource.com/6270 Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26Avoid sticking -1 into a size_t.David Benjamin
There's still a size_t/int cast due to the mass of legacy code, but at least avoid the most egregious case. Change-Id: Icc1741366e09190216e762ca7ef42ecfc3215edc Reviewed-on: https://boringssl-review.googlesource.com/6345 Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26Remove dead code in x509_lu.c.David Benjamin
See also upstream's b62a2f8a373d1889672599834acf95161f2883ce, though upstream left the lock calls in by accident. Otherwise, the change appears to be correct. I see no side effects of x509_object_idx_cnt beyond the return value and *pnmatch, both of which are discarded. Change-Id: Ic2124a733a61591bd1b264164726ce6c69ce10c9 Reviewed-on: https://boringssl-review.googlesource.com/6347 Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26Fix various malloc failure codepaths.David Benjamin
CRYPTO_MUTEX_init needs a CRYPTO_MUTEX_cleanup. Also a pile of problems with x509_lu.c I noticed trying to import some upstream change. Change-Id: I029a65cd2d30aa31f4832e8fbfe5b2ea0dbc66fe Reviewed-on: https://boringssl-review.googlesource.com/6346 Reviewed-by: Adam Langley <alangley@gmail.com>
2015-09-29d2i: don't update input pointer on failureDavid Benjamin
(Imported from upstream's 728bcd59d3d41e152aead0d15acc51a8958536d3.) Actually this one was reported by us, but the commit message doesn't mention this. This is slightly modified from upstream's version to fix some problems noticed in import. Specifically one of d2i_X509_AUX's success paths is bust and d2i_PrivateKey still updates on one error path. Resolve the latter by changing both it and d2i_AutoPrivateKey to explicitly hit the error path on ret == NULL. This lets us remove the NULL check in d2i_AutoPrivateKey. We'll want to report the problems back upstream. Change-Id: Ifcfc965ca6d5ec0a08ac154854bd351cafbaba25 Reviewed-on: https://boringssl-review.googlesource.com/5948 Reviewed-by: Adam Langley <agl@google.com>
2015-09-01Add X509_CRL_up_ref.David Benjamin
(Imported from upstream's 65cbf983ca4f69b8954f949c2edaaa48824481b3.) Change-Id: I1e5d26ed8da5a44f68d22385b31d413628229c50 Reviewed-on: https://boringssl-review.googlesource.com/5784 Reviewed-by: Adam Langley <agl@google.com>
2015-08-31Fix memory leaks on error in x_x509a.c.David Benjamin
See also upstream's c8491de393639dbc4508306b7dbedb3872b74293. Change-Id: I017fb137d6d93b6abb82fdb03f02be8292963d0d Reviewed-on: https://boringssl-review.googlesource.com/5767 Reviewed-by: Adam Langley <agl@google.com>
2015-08-26Move arm_arch.h and fix up lots of include paths.Adam Langley
arm_arch.h is included from ARM asm files, but lives in crypto/, not openssl/include/. Since the asm files are often built from a different location than their position in the source tree, relative include paths are unlikely to work so, rather than having crypto/ be a de-facto, second global include path, this change moves arm_arch.h to include/openssl/. It also removes entries from many include paths because they should be needed as relative includes are always based on the locations of the source file. Change-Id: I638ff43d641ca043a4fc06c0d901b11c6ff73542 Reviewed-on: https://boringssl-review.googlesource.com/5746 Reviewed-by: Adam Langley <agl@google.com>
2015-08-17Fix a couple other leaks on failure in X509_verify_cert.David Benjamin
If get_issuer fails, some of these calls return rather than jumping to common cleanup code. Change-Id: Iacd59747fb11e9bfaae86f2eeed88798ee08203e Reviewed-on: https://boringssl-review.googlesource.com/5711 Reviewed-by: Adam Langley <agl@google.com>