Write some templated functions for the d2i/i2d convention We open-coded a lot of these because we didn't have C++ templates in the beginning. Now we do and we can map calling conventions much more straightforwardly. Bug: 42290417 Change-Id: I7e5ba9e1bf2bd556bbff614b11ac15eb6e155f49 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/81773 Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/crypto/asn1/a_bool.cc b/crypto/asn1/a_bool.cc index 7869e69..d068343 100644 --- a/crypto/asn1/a_bool.cc +++ b/crypto/asn1/a_bool.cc
@@ -21,13 +21,10 @@ int i2d_ASN1_BOOLEAN(ASN1_BOOLEAN a, unsigned char **outp) { - CBB cbb; - if (!CBB_init(&cbb, 3) || // - !CBB_add_asn1_bool(&cbb, a != ASN1_BOOLEAN_FALSE)) { - CBB_cleanup(&cbb); - return -1; - } - return CBB_finish_i2d(&cbb, outp); + return bssl::I2DFromCBB( + /*initial_capacity=*/3, outp, [&](CBB *cbb) -> bool { + return CBB_add_asn1_bool(cbb, a != ASN1_BOOLEAN_FALSE); + }); } ASN1_BOOLEAN d2i_ASN1_BOOLEAN(ASN1_BOOLEAN *out, const unsigned char **inp,
diff --git a/crypto/asn1/a_object.cc b/crypto/asn1/a_object.cc index bb4bc4a..102a9f0 100644 --- a/crypto/asn1/a_object.cc +++ b/crypto/asn1/a_object.cc
@@ -43,13 +43,11 @@ } int i2d_ASN1_OBJECT(const ASN1_OBJECT *in, unsigned char **outp) { - bssl::ScopedCBB cbb; - if (!CBB_init(cbb.get(), static_cast<size_t>(in->length) + 2) || - !asn1_marshal_object(cbb.get(), in, /*tag=*/0)) { - return -1; - } - - return CBB_finish_i2d(cbb.get(), outp); + return bssl::I2DFromCBB( + /*initial_capacity=*/static_cast<size_t>(in->length) + 2, outp, + [&](CBB *cbb) -> bool { + return asn1_marshal_object(cbb, in, /*tag=*/0); + }); } int i2t_ASN1_OBJECT(char *buf, int buf_len, const ASN1_OBJECT *a) { @@ -93,53 +91,33 @@ ASN1_OBJECT *d2i_ASN1_OBJECT(ASN1_OBJECT **out, const unsigned char **inp, long len) { - if (len < 0) { - return NULL; - } - - CBS cbs, child; - CBS_init(&cbs, *inp, (size_t)len); - if (!CBS_get_asn1(&cbs, &child, CBS_ASN1_OBJECT)) { - OPENSSL_PUT_ERROR(ASN1, ASN1_R_DECODE_ERROR); - return NULL; - } - - const uint8_t *contents = CBS_data(&child); - ASN1_OBJECT *ret = c2i_ASN1_OBJECT(out, &contents, CBS_len(&child)); - if (ret != NULL) { - // |c2i_ASN1_OBJECT| should have consumed the entire input. - assert(CBS_data(&cbs) == contents); - *inp = CBS_data(&cbs); - } - return ret; + return bssl::D2IFromCBS(out, inp, len, [](CBS *cbs) -> ASN1_OBJECT * { + CBS child; + if (!CBS_get_asn1(cbs, &child, CBS_ASN1_OBJECT)) { + OPENSSL_PUT_ERROR(ASN1, ASN1_R_DECODE_ERROR); + return nullptr; + } + const uint8_t *contents = CBS_data(&child); + return c2i_ASN1_OBJECT(nullptr, &contents, CBS_len(&child)); + }); } ASN1_OBJECT *c2i_ASN1_OBJECT(ASN1_OBJECT **out, const unsigned char **inp, long len) { - if (len < 0) { - OPENSSL_PUT_ERROR(ASN1, ASN1_R_INVALID_OBJECT_ENCODING); - return NULL; - } - - CBS cbs; - CBS_init(&cbs, *inp, (size_t)len); - if (!CBS_is_valid_asn1_oid(&cbs)) { - OPENSSL_PUT_ERROR(ASN1, ASN1_R_INVALID_OBJECT_ENCODING); - return NULL; - } - - ASN1_OBJECT *ret = ASN1_OBJECT_create(NID_undef, *inp, (size_t)len, - /*sn=*/NULL, /*ln=*/NULL); - if (ret == NULL) { - return NULL; - } - - if (out != NULL) { - ASN1_OBJECT_free(*out); - *out = ret; - } - *inp += len; // All bytes were consumed. - return ret; + return bssl::D2IFromCBS(out, inp, len, [](CBS *cbs) -> ASN1_OBJECT * { + if (!CBS_is_valid_asn1_oid(cbs)) { + OPENSSL_PUT_ERROR(ASN1, ASN1_R_INVALID_OBJECT_ENCODING); + return nullptr; + } + ASN1_OBJECT *ret = + ASN1_OBJECT_create(NID_undef, CBS_data(cbs), CBS_len(cbs), + /*sn=*/nullptr, /*ln=*/nullptr); + if (ret != nullptr) { + // |c2i_ASN1_OBJECT| consumes its whole input on success. + BSSL_CHECK(CBS_skip(cbs, CBS_len(cbs))); + } + return ret; + }); } ASN1_OBJECT *asn1_parse_object(CBS *cbs, CBS_ASN1_TAG tag) {
diff --git a/crypto/bytestring/internal.h b/crypto/bytestring/internal.h index 110fb95..0ec59e2 100644 --- a/crypto/bytestring/internal.h +++ b/crypto/bytestring/internal.h
@@ -15,12 +15,14 @@ #ifndef OPENSSL_HEADER_CRYPTO_BYTESTRING_INTERNAL_H #define OPENSSL_HEADER_CRYPTO_BYTESTRING_INTERNAL_H -#include <openssl/base.h> +#include <openssl/asn1.h> +#include <openssl/bytestring.h> +#include <openssl/err.h> -#if defined(__cplusplus) +#include <type_traits> + + extern "C" { -#endif - // CBS_asn1_ber_to_der reads a BER element from |in|. If it finds // indefinite-length elements or constructed strings then it converts the BER @@ -66,9 +68,53 @@ // This function may be used to help implement legacy i2d ASN.1 functions. int CBB_finish_i2d(CBB *cbb, uint8_t **outp); - -#if defined(__cplusplus) } // extern C -#endif + +BSSL_NAMESPACE_BEGIN + +// D2IFromCBS takes a functor of type |Unique<T>(CBS*)| and implements the d2i +// calling convention. For compatibility with functions that don't tag their +// return value (e.g. public APIs), |T*(CBS)| is also accepted. The callback can +// assume that the |CBS|'s length fits in |long|. The callback should not access +// |out|, |inp|, or |len| directly. +template <typename T, typename CBSFunc> +inline T *D2IFromCBS(T **out, const uint8_t **inp, long len, CBSFunc func) { + static_assert(std::is_invocable_v<CBSFunc, CBS *>); + static_assert( + std::is_same_v<std::invoke_result_t<CBSFunc, CBS *>, UniquePtr<T>> || + std::is_same_v<std::invoke_result_t<CBSFunc, CBS *>, T *>); + if (len < 0) { + OPENSSL_PUT_ERROR(ASN1, ASN1_R_BUFFER_TOO_SMALL); + return nullptr; + } + CBS cbs; + CBS_init(&cbs, *inp, len); + UniquePtr<T> ret(func(&cbs)); + if (ret == nullptr) { + return nullptr; + } + if (out != nullptr) { + UniquePtr<T> free_out(*out); + *out = ret.get(); + } + *inp = CBS_data(&cbs); + return ret.release(); +} + +// I2DFromCBB takes a functor of type |bool(CBB*)| and implements the i2d +// calling convention. It internally makes a |CBB| with the specified initial +// capacity. The callback should not access |outp| directly. +template <typename CBBFunc> +inline int I2DFromCBB(size_t initial_capacity, uint8_t **outp, CBBFunc func) { + static_assert(std::is_invocable_v<CBBFunc, CBB *>); + static_assert(std::is_same_v<std::invoke_result_t<CBBFunc, CBB *>, bool>); + ScopedCBB cbb; + if (!CBB_init(cbb.get(), initial_capacity) || !func(cbb.get())) { + return -1; + } + return CBB_finish_i2d(cbb.get(), outp); +} + +BSSL_NAMESPACE_END #endif // OPENSSL_HEADER_CRYPTO_BYTESTRING_INTERNAL_H
diff --git a/crypto/dh/dh_asn1.cc b/crypto/dh/dh_asn1.cc index 9295e14..8b1ec11 100644 --- a/crypto/dh/dh_asn1.cc +++ b/crypto/dh/dh_asn1.cc
@@ -95,29 +95,11 @@ } DH *d2i_DHparams(DH **out, const uint8_t **inp, long len) { - if (len < 0) { - return NULL; - } - CBS cbs; - CBS_init(&cbs, *inp, (size_t)len); - DH *ret = DH_parse_parameters(&cbs); - if (ret == NULL) { - return NULL; - } - if (out != NULL) { - DH_free(*out); - *out = ret; - } - *inp = CBS_data(&cbs); - return ret; + return bssl::D2IFromCBS(out, inp, len, DH_parse_parameters); } int i2d_DHparams(const DH *in, uint8_t **outp) { - CBB cbb; - if (!CBB_init(&cbb, 0) || - !DH_marshal_parameters(&cbb, in)) { - CBB_cleanup(&cbb); - return -1; - } - return CBB_finish_i2d(&cbb, outp); + return bssl::I2DFromCBB( + /*initial_capacity=*/256, outp, + [&](CBB *cbb) -> bool { return DH_marshal_parameters(cbb, in); }); }
diff --git a/crypto/dsa/dsa_asn1.cc b/crypto/dsa/dsa_asn1.cc index af5ccad..456c03b 100644 --- a/crypto/dsa/dsa_asn1.cc +++ b/crypto/dsa/dsa_asn1.cc
@@ -255,113 +255,41 @@ } DSA_SIG *d2i_DSA_SIG(DSA_SIG **out_sig, const uint8_t **inp, long len) { - if (len < 0) { - return NULL; - } - CBS cbs; - CBS_init(&cbs, *inp, (size_t)len); - DSA_SIG *ret = DSA_SIG_parse(&cbs); - if (ret == NULL) { - return NULL; - } - if (out_sig != NULL) { - DSA_SIG_free(*out_sig); - *out_sig = ret; - } - *inp = CBS_data(&cbs); - return ret; + return bssl::D2IFromCBS(out_sig, inp, len, DSA_SIG_parse); } int i2d_DSA_SIG(const DSA_SIG *in, uint8_t **outp) { - CBB cbb; - if (!CBB_init(&cbb, 0) || - !DSA_SIG_marshal(&cbb, in)) { - CBB_cleanup(&cbb); - return -1; - } - return CBB_finish_i2d(&cbb, outp); + return bssl::I2DFromCBB( + /*initial_capacity=*/256, outp, + [&](CBB *cbb) -> bool { return DSA_SIG_marshal(cbb, in); }); } DSA *d2i_DSAPublicKey(DSA **out, const uint8_t **inp, long len) { - if (len < 0) { - return NULL; - } - CBS cbs; - CBS_init(&cbs, *inp, (size_t)len); - DSA *ret = DSA_parse_public_key(&cbs); - if (ret == NULL) { - return NULL; - } - if (out != NULL) { - DSA_free(*out); - *out = ret; - } - *inp = CBS_data(&cbs); - return ret; + return bssl::D2IFromCBS(out, inp, len, DSA_parse_public_key); } int i2d_DSAPublicKey(const DSA *in, uint8_t **outp) { - CBB cbb; - if (!CBB_init(&cbb, 0) || - !DSA_marshal_public_key(&cbb, in)) { - CBB_cleanup(&cbb); - return -1; - } - return CBB_finish_i2d(&cbb, outp); + return bssl::I2DFromCBB( + /*initial_capacity=*/256, outp, + [&](CBB *cbb) -> bool { return DSA_marshal_public_key(cbb, in); }); } DSA *d2i_DSAPrivateKey(DSA **out, const uint8_t **inp, long len) { - if (len < 0) { - return NULL; - } - CBS cbs; - CBS_init(&cbs, *inp, (size_t)len); - DSA *ret = DSA_parse_private_key(&cbs); - if (ret == NULL) { - return NULL; - } - if (out != NULL) { - DSA_free(*out); - *out = ret; - } - *inp = CBS_data(&cbs); - return ret; + return bssl::D2IFromCBS(out, inp, len, DSA_parse_private_key); } int i2d_DSAPrivateKey(const DSA *in, uint8_t **outp) { - CBB cbb; - if (!CBB_init(&cbb, 0) || - !DSA_marshal_private_key(&cbb, in)) { - CBB_cleanup(&cbb); - return -1; - } - return CBB_finish_i2d(&cbb, outp); + return bssl::I2DFromCBB( + /*initial_capacity=*/256, outp, + [&](CBB *cbb) -> bool { return DSA_marshal_private_key(cbb, in); }); } DSA *d2i_DSAparams(DSA **out, const uint8_t **inp, long len) { - if (len < 0) { - return NULL; - } - CBS cbs; - CBS_init(&cbs, *inp, (size_t)len); - DSA *ret = DSA_parse_parameters(&cbs); - if (ret == NULL) { - return NULL; - } - if (out != NULL) { - DSA_free(*out); - *out = ret; - } - *inp = CBS_data(&cbs); - return ret; + return bssl::D2IFromCBS(out, inp, len, DSA_parse_parameters); } int i2d_DSAparams(const DSA *in, uint8_t **outp) { - CBB cbb; - if (!CBB_init(&cbb, 0) || - !DSA_marshal_parameters(&cbb, in)) { - CBB_cleanup(&cbb); - return -1; - } - return CBB_finish_i2d(&cbb, outp); + return bssl::I2DFromCBB( + /*initial_capacity=*/256, outp, + [&](CBB *cbb) -> bool { return DSA_marshal_parameters(cbb, in); }); }
diff --git a/crypto/ec/ec_asn1.cc b/crypto/ec/ec_asn1.cc index 3d54e11..c28da4a 100644 --- a/crypto/ec/ec_asn1.cc +++ b/crypto/ec/ec_asn1.cc
@@ -432,52 +432,20 @@ group = EC_KEY_get0_group(*out); } - if (len < 0) { - OPENSSL_PUT_ERROR(EC, EC_R_DECODE_ERROR); - return NULL; - } - CBS cbs; - CBS_init(&cbs, *inp, (size_t)len); - EC_KEY *ret = EC_KEY_parse_private_key(&cbs, group); - if (ret == NULL) { - return NULL; - } - if (out != NULL) { - EC_KEY_free(*out); - *out = ret; - } - *inp = CBS_data(&cbs); - return ret; + return bssl::D2IFromCBS(out, inp, len, [&](CBS *cbs) { + return EC_KEY_parse_private_key(cbs, group); + }); } int i2d_ECPrivateKey(const EC_KEY *key, uint8_t **outp) { - CBB cbb; - if (!CBB_init(&cbb, 0) || - !EC_KEY_marshal_private_key(&cbb, key, EC_KEY_get_enc_flags(key))) { - CBB_cleanup(&cbb); - return -1; - } - return CBB_finish_i2d(&cbb, outp); + return bssl::I2DFromCBB( + /*initial_capacity=*/64, outp, [&](CBB *cbb) -> bool { + return EC_KEY_marshal_private_key(cbb, key, EC_KEY_get_enc_flags(key)); + }); } EC_GROUP *d2i_ECPKParameters(EC_GROUP **out, const uint8_t **inp, long len) { - if (len < 0) { - return NULL; - } - - CBS cbs; - CBS_init(&cbs, *inp, (size_t)len); - EC_GROUP *ret = EC_KEY_parse_parameters(&cbs); - if (ret == NULL) { - return NULL; - } - - if (out != NULL) { - EC_GROUP_free(*out); - *out = ret; - } - *inp = CBS_data(&cbs); - return ret; + return bssl::D2IFromCBS(out, inp, len, EC_KEY_parse_parameters); } int i2d_ECPKParameters(const EC_GROUP *group, uint8_t **outp) { @@ -485,40 +453,24 @@ OPENSSL_PUT_ERROR(EC, ERR_R_PASSED_NULL_PARAMETER); return -1; } - - CBB cbb; - if (!CBB_init(&cbb, 0) || // - !EC_KEY_marshal_curve_name(&cbb, group)) { - CBB_cleanup(&cbb); - return -1; - } - return CBB_finish_i2d(&cbb, outp); + return bssl::I2DFromCBB( + /*initial_capacity=*/16, outp, + [&](CBB *cbb) -> bool { return EC_KEY_marshal_curve_name(cbb, group); }); } EC_KEY *d2i_ECParameters(EC_KEY **out_key, const uint8_t **inp, long len) { - if (len < 0) { - return NULL; - } - - CBS cbs; - CBS_init(&cbs, *inp, (size_t)len); - const EC_GROUP *group = EC_KEY_parse_parameters(&cbs); - if (group == NULL) { - return NULL; - } - - EC_KEY *ret = EC_KEY_new(); - if (ret == NULL || !EC_KEY_set_group(ret, group)) { - EC_KEY_free(ret); - return NULL; - } - - if (out_key != NULL) { - EC_KEY_free(*out_key); - *out_key = ret; - } - *inp = CBS_data(&cbs); - return ret; + return bssl::D2IFromCBS( + out_key, inp, len, [](CBS *cbs) -> bssl::UniquePtr<EC_KEY> { + const EC_GROUP *group = EC_KEY_parse_parameters(cbs); + if (group == nullptr) { + return nullptr; + } + bssl::UniquePtr<EC_KEY> ret(EC_KEY_new()); + if (ret == nullptr || !EC_KEY_set_group(ret.get(), group)) { + return nullptr; + } + return ret; + }); } int i2d_ECParameters(const EC_KEY *key, uint8_t **outp) { @@ -526,14 +478,10 @@ OPENSSL_PUT_ERROR(EC, ERR_R_PASSED_NULL_PARAMETER); return -1; } - - CBB cbb; - if (!CBB_init(&cbb, 0) || // - !EC_KEY_marshal_curve_name(&cbb, key->group)) { - CBB_cleanup(&cbb); - return -1; - } - return CBB_finish_i2d(&cbb, outp); + return bssl::I2DFromCBB( + /*initial_capacity=*/16, outp, [&](CBB *cbb) -> bool { + return EC_KEY_marshal_curve_name(cbb, key->group); + }); } EC_KEY *o2i_ECPublicKey(EC_KEY **keyp, const uint8_t **inp, long len) { @@ -563,14 +511,13 @@ OPENSSL_PUT_ERROR(EC, ERR_R_PASSED_NULL_PARAMETER); return 0; } - CBB cbb; - if (!CBB_init(&cbb, 0) || // - !EC_POINT_point2cbb(&cbb, key->group, key->pub_key, key->conv_form, - NULL)) { - CBB_cleanup(&cbb); - return -1; - } - int ret = CBB_finish_i2d(&cbb, outp); + // No initial capacity because |EC_POINT_point2cbb| will internally reserve + // the right size in one shot, so it's best to leave this at zero. + int ret = bssl::I2DFromCBB( + /*initial_capacity=*/0, outp, [&](CBB *cbb) -> bool { + return EC_POINT_point2cbb(cbb, key->group, key->pub_key, key->conv_form, + nullptr); + }); // Historically, this function used the wrong return value on error. return ret > 0 ? ret : 0; }
diff --git a/crypto/ecdsa/ecdsa_asn1.cc b/crypto/ecdsa/ecdsa_asn1.cc index 28d5036..cf932fb 100644 --- a/crypto/ecdsa/ecdsa_asn1.cc +++ b/crypto/ecdsa/ecdsa_asn1.cc
@@ -324,28 +324,11 @@ } ECDSA_SIG *d2i_ECDSA_SIG(ECDSA_SIG **out, const uint8_t **inp, long len) { - if (len < 0) { - return NULL; - } - CBS cbs; - CBS_init(&cbs, *inp, (size_t)len); - ECDSA_SIG *ret = ECDSA_SIG_parse(&cbs); - if (ret == NULL) { - return NULL; - } - if (out != NULL) { - ECDSA_SIG_free(*out); - *out = ret; - } - *inp = CBS_data(&cbs); - return ret; + return bssl::D2IFromCBS(out, inp, len, ECDSA_SIG_parse); } int i2d_ECDSA_SIG(const ECDSA_SIG *sig, uint8_t **outp) { - CBB cbb; - if (!CBB_init(&cbb, 0) || !ECDSA_SIG_marshal(&cbb, sig)) { - CBB_cleanup(&cbb); - return -1; - } - return CBB_finish_i2d(&cbb, outp); + return bssl::I2DFromCBB( + /*initial_capacity=*/64, outp, + [&](CBB *cbb) -> bool { return ECDSA_SIG_marshal(cbb, sig); }); }
diff --git a/crypto/evp/evp_asn1.cc b/crypto/evp/evp_asn1.cc index a135b0a..c6f3870 100644 --- a/crypto/evp/evp_asn1.cc +++ b/crypto/evp/evp_asn1.cc
@@ -221,35 +221,26 @@ EVP_PKEY *d2i_PrivateKey(int type, EVP_PKEY **out, const uint8_t **inp, long len) { - if (len < 0) { - OPENSSL_PUT_ERROR(EVP, EVP_R_DECODE_ERROR); - return nullptr; - } - - // Parse with the legacy format. - CBS cbs; - CBS_init(&cbs, *inp, (size_t)len); - bssl::UniquePtr<EVP_PKEY> ret = old_priv_decode(&cbs, type); - if (ret == nullptr) { - // Try again with PKCS#8. - ERR_clear_error(); - CBS_init(&cbs, *inp, (size_t)len); - ret.reset(EVP_parse_private_key(&cbs)); - if (ret == nullptr) { - return nullptr; - } - if (EVP_PKEY_id(ret.get()) != type) { - OPENSSL_PUT_ERROR(EVP, EVP_R_DIFFERENT_KEY_TYPES); - return nullptr; - } - } - - if (out != nullptr) { - EVP_PKEY_free(*out); - *out = ret.get(); - } - *inp = CBS_data(&cbs); - return ret.release(); + return bssl::D2IFromCBS( + out, inp, len, [&](CBS *cbs) -> bssl::UniquePtr<EVP_PKEY> { + // Parse with the legacy format. + CBS copy = *cbs; + bssl::UniquePtr<EVP_PKEY> ret = old_priv_decode(cbs, type); + if (ret == nullptr) { + // Try again with PKCS#8. + ERR_clear_error(); + *cbs = copy; + ret.reset(EVP_parse_private_key(cbs)); + if (ret == nullptr) { + return nullptr; + } + if (EVP_PKEY_id(ret.get()) != type) { + OPENSSL_PUT_ERROR(EVP, EVP_R_DIFFERENT_KEY_TYPES); + return nullptr; + } + } + return ret; + }); } // num_elements parses one SEQUENCE from |in| and returns the number of elements @@ -323,71 +314,45 @@ EVP_PKEY *d2i_PublicKey(int type, EVP_PKEY **out, const uint8_t **inp, long len) { - bssl::UniquePtr<EVP_PKEY> ret(EVP_PKEY_new()); - if (ret == nullptr) { - return nullptr; - } + return bssl::D2IFromCBS( + out, inp, len, [&](CBS *cbs) -> bssl::UniquePtr<EVP_PKEY> { + bssl::UniquePtr<EVP_PKEY> ret(EVP_PKEY_new()); + if (ret == nullptr) { + return nullptr; + } + switch (type) { + case EVP_PKEY_RSA: { + bssl::UniquePtr<RSA> rsa(RSA_parse_public_key(cbs)); + if (rsa == nullptr) { + return nullptr; + } + EVP_PKEY_assign_RSA(ret.get(), rsa.release()); + return ret; + } - CBS cbs; - CBS_init(&cbs, *inp, len < 0 ? 0 : (size_t)len); - switch (type) { - case EVP_PKEY_RSA: { - bssl::UniquePtr<RSA> rsa(RSA_parse_public_key(&cbs)); - if (rsa == nullptr) { - return nullptr; - } - EVP_PKEY_assign_RSA(ret.get(), rsa.release()); - break; - } - - // Unlike OpenSSL, we do not support EC keys with this API. The raw EC - // public key serialization requires knowing the group. In OpenSSL, calling - // this function with |EVP_PKEY_EC| and setting |out| to nullptr does not - // work. It requires |*out| to include a partially-initialized |EVP_PKEY| to - // extract the group. - default: - OPENSSL_PUT_ERROR(EVP, EVP_R_UNSUPPORTED_PUBLIC_KEY_TYPE); - return nullptr; - } - - *inp = CBS_data(&cbs); - if (out != nullptr) { - EVP_PKEY_free(*out); - *out = ret.get(); - } - return ret.release(); + // Unlike OpenSSL, we do not support EC keys with this API. The raw EC + // public key serialization requires knowing the group. In OpenSSL, + // calling this function with |EVP_PKEY_EC| and setting |out| to + // nullptr does not work. It requires |*out| to include a + // partially-initialized |EVP_PKEY| to extract the group. + default: + OPENSSL_PUT_ERROR(EVP, EVP_R_UNSUPPORTED_PUBLIC_KEY_TYPE); + return nullptr; + } + }); } EVP_PKEY *d2i_PUBKEY(EVP_PKEY **out, const uint8_t **inp, long len) { - if (len < 0) { - return nullptr; - } - CBS cbs; - CBS_init(&cbs, *inp, (size_t)len); - bssl::UniquePtr<EVP_PKEY> ret(EVP_parse_public_key(&cbs)); - if (ret == nullptr) { - return nullptr; - } - if (out != nullptr) { - EVP_PKEY_free(*out); - *out = ret.get(); - } - *inp = CBS_data(&cbs); - return ret.release(); + return bssl::D2IFromCBS(out, inp, len, EVP_parse_public_key); } int i2d_PUBKEY(const EVP_PKEY *pkey, uint8_t **outp) { - if (pkey == NULL) { + if (pkey == nullptr) { return 0; } - - CBB cbb; - if (!CBB_init(&cbb, 128) || - !EVP_marshal_public_key(&cbb, pkey)) { - CBB_cleanup(&cbb); - return -1; - } - return CBB_finish_i2d(&cbb, outp); + return bssl::I2DFromCBB( + /*initial_capacity=*/128, outp, + [&](CBB *cbb) -> bool { return EVP_marshal_public_key(cbb, pkey); }); } static bssl::UniquePtr<EVP_PKEY> parse_spki( @@ -406,25 +371,13 @@ } RSA *d2i_RSA_PUBKEY(RSA **out, const uint8_t **inp, long len) { - if (len < 0) { - return nullptr; - } - CBS cbs; - CBS_init(&cbs, *inp, (size_t)len); - bssl::UniquePtr<EVP_PKEY> pkey = parse_spki(&cbs, EVP_pkey_rsa()); - if (pkey == nullptr) { - return nullptr; - } - bssl::UniquePtr<RSA> rsa(EVP_PKEY_get1_RSA(pkey.get())); - if (rsa == nullptr) { - return nullptr; - } - if (out != nullptr) { - RSA_free(*out); - *out = rsa.get(); - } - *inp = CBS_data(&cbs); - return rsa.release(); + return bssl::D2IFromCBS(out, inp, len, [](CBS *cbs) -> bssl::UniquePtr<RSA> { + bssl::UniquePtr<EVP_PKEY> pkey = parse_spki(cbs, EVP_pkey_rsa()); + if (pkey == nullptr) { + return nullptr; + } + return bssl::UniquePtr<RSA>(EVP_PKEY_get1_RSA(pkey.get())); + }); } int i2d_RSA_PUBKEY(const RSA *rsa, uint8_t **outp) { @@ -442,25 +395,13 @@ } DSA *d2i_DSA_PUBKEY(DSA **out, const uint8_t **inp, long len) { - if (len < 0) { - return nullptr; - } - CBS cbs; - CBS_init(&cbs, *inp, (size_t)len); - bssl::UniquePtr<EVP_PKEY> pkey = parse_spki(&cbs, EVP_pkey_dsa()); - if (pkey == nullptr) { - return nullptr; - } - bssl::UniquePtr<DSA> dsa(EVP_PKEY_get1_DSA(pkey.get())); - if (dsa == nullptr) { - return nullptr; - } - if (out != nullptr) { - DSA_free(*out); - *out = dsa.get(); - } - *inp = CBS_data(&cbs); - return dsa.release(); + return bssl::D2IFromCBS(out, inp, len, [](CBS *cbs) -> bssl::UniquePtr<DSA> { + bssl::UniquePtr<EVP_PKEY> pkey = parse_spki(cbs, EVP_pkey_dsa()); + if (pkey == nullptr) { + return nullptr; + } + return bssl::UniquePtr<DSA>(EVP_PKEY_get1_DSA(pkey.get())); + }); } int i2d_DSA_PUBKEY(const DSA *dsa, uint8_t **outp) { @@ -478,27 +419,17 @@ } EC_KEY *d2i_EC_PUBKEY(EC_KEY **out, const uint8_t **inp, long len) { - if (len < 0) { - return nullptr; - } - CBS cbs; - CBS_init(&cbs, *inp, (size_t)len); - const EVP_PKEY_ALG *const algs[] = {EVP_pkey_ec_p224(), EVP_pkey_ec_p256(), - EVP_pkey_ec_p384(), EVP_pkey_ec_p521()}; - bssl::UniquePtr<EVP_PKEY> pkey = parse_spki(&cbs, algs); - if (pkey == nullptr) { - return nullptr; - } - bssl::UniquePtr<EC_KEY> ec_key(EVP_PKEY_get1_EC_KEY(pkey.get())); - if (ec_key == nullptr) { - return nullptr; - } - if (out != nullptr) { - EC_KEY_free(*out); - *out = ec_key.get(); - } - *inp = CBS_data(&cbs); - return ec_key.release(); + return bssl::D2IFromCBS( + out, inp, len, [](CBS *cbs) -> bssl::UniquePtr<EC_KEY> { + const EVP_PKEY_ALG *const algs[] = { + EVP_pkey_ec_p224(), EVP_pkey_ec_p256(), EVP_pkey_ec_p384(), + EVP_pkey_ec_p521()}; + bssl::UniquePtr<EVP_PKEY> pkey = parse_spki(cbs, algs); + if (pkey == nullptr) { + return nullptr; + } + return bssl::UniquePtr<EC_KEY>(EVP_PKEY_get1_EC_KEY(pkey.get())); + }); } int i2d_EC_PUBKEY(const EC_KEY *ec_key, uint8_t **outp) {
diff --git a/crypto/rsa/rsa_asn1.cc b/crypto/rsa/rsa_asn1.cc index d22e584..a57ff0e 100644 --- a/crypto/rsa/rsa_asn1.cc +++ b/crypto/rsa/rsa_asn1.cc
@@ -210,59 +210,23 @@ } RSA *d2i_RSAPublicKey(RSA **out, const uint8_t **inp, long len) { - if (len < 0) { - return NULL; - } - CBS cbs; - CBS_init(&cbs, *inp, (size_t)len); - RSA *ret = RSA_parse_public_key(&cbs); - if (ret == NULL) { - return NULL; - } - if (out != NULL) { - RSA_free(*out); - *out = ret; - } - *inp = CBS_data(&cbs); - return ret; + return bssl::D2IFromCBS(out, inp, len, RSA_parse_public_key); } int i2d_RSAPublicKey(const RSA *in, uint8_t **outp) { - CBB cbb; - if (!CBB_init(&cbb, 0) || - !RSA_marshal_public_key(&cbb, in)) { - CBB_cleanup(&cbb); - return -1; - } - return CBB_finish_i2d(&cbb, outp); + return bssl::I2DFromCBB( + /*initial_capacity=*/256, outp, + [&](CBB *cbb) -> bool { return RSA_marshal_public_key(cbb, in); }); } RSA *d2i_RSAPrivateKey(RSA **out, const uint8_t **inp, long len) { - if (len < 0) { - return NULL; - } - CBS cbs; - CBS_init(&cbs, *inp, (size_t)len); - RSA *ret = RSA_parse_private_key(&cbs); - if (ret == NULL) { - return NULL; - } - if (out != NULL) { - RSA_free(*out); - *out = ret; - } - *inp = CBS_data(&cbs); - return ret; + return bssl::D2IFromCBS(out, inp, len, RSA_parse_private_key); } int i2d_RSAPrivateKey(const RSA *in, uint8_t **outp) { - CBB cbb; - if (!CBB_init(&cbb, 0) || - !RSA_marshal_private_key(&cbb, in)) { - CBB_cleanup(&cbb); - return -1; - } - return CBB_finish_i2d(&cbb, outp); + return bssl::I2DFromCBB( + /*initial_capacity=*/512, outp, + [&](CBB *cbb) -> bool { return RSA_marshal_private_key(cbb, in); }); } RSA *RSAPublicKey_dup(const RSA *rsa) {
diff --git a/crypto/x509/x_x509.cc b/crypto/x509/x_x509.cc index e2869ac..066a978 100644 --- a/crypto/x509/x_x509.cc +++ b/crypto/x509/x_x509.cc
@@ -171,27 +171,8 @@ } X509 *d2i_X509(X509 **out, const uint8_t **inp, long len) { - X509 *ret = NULL; - if (len < 0) { - OPENSSL_PUT_ERROR(ASN1, ASN1_R_BUFFER_TOO_SMALL); - goto err; - } - - CBS cbs; - CBS_init(&cbs, *inp, (size_t)len); - ret = x509_parse(&cbs, NULL); - if (ret == NULL) { - goto err; - } - - *inp = CBS_data(&cbs); - -err: - if (out != NULL) { - X509_free(*out); - *out = ret; - } - return ret; + return bssl::D2IFromCBS(out, inp, len, + [](CBS *cbs) { return x509_parse(cbs, nullptr); }); } int i2d_X509(X509 *x509, uint8_t **outp) { @@ -200,26 +181,28 @@ return -1; } - bssl::ScopedCBB cbb; - CBB cert; - if (!CBB_init(cbb.get(), 64) || // - !CBB_add_asn1(cbb.get(), &cert, CBS_ASN1_SEQUENCE)) { - return -1; - } + return bssl::I2DFromCBB( + /*initial_capacity=*/64, outp, [&](CBB *cbb) -> bool { + CBB cert; + if (!CBB_add_asn1(cbb, &cert, CBS_ASN1_SEQUENCE)) { + return false; + } - // TODO(crbug.com/boringssl/443): When the rest of the library is decoupled - // from the tasn_*.c implementation, replace this with |CBS|-based functions. - uint8_t *out; - int len = i2d_X509_CINF(x509->cert_info, NULL); - if (len < 0 || // - !CBB_add_space(&cert, &out, static_cast<size_t>(len)) || - i2d_X509_CINF(x509->cert_info, &out) != len || - !x509_marshal_algorithm(&cert, x509->sig_alg) || - !asn1_marshal_bit_string(&cert, &x509->signature, /*tag=*/0)) { - return -1; - } - - return CBB_finish_i2d(cbb.get(), outp); + // TODO(crbug.com/boringssl/443): When the rest of the library is + // decoupled from the tasn_*.c implementation, replace this with + // |CBB|-based functions. + uint8_t *out; + int len = i2d_X509_CINF(x509->cert_info, NULL); + if (len < 0 || // + !CBB_add_space(&cert, &out, static_cast<size_t>(len)) || + i2d_X509_CINF(x509->cert_info, &out) != len || + !x509_marshal_algorithm(&cert, x509->sig_alg) || + !asn1_marshal_bit_string(&cert, &x509->signature, /*tag=*/0) || + !CBB_flush(cbb)) { + return false; + } + return true; + }); } static int x509_new_cb(ASN1_VALUE **pval, const ASN1_ITEM *it) {
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index 51417d4..9ee7ec1 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h
@@ -5354,12 +5354,12 @@ // Use |SSL_SESSION_to_bytes| instead. OPENSSL_EXPORT int i2d_SSL_SESSION(SSL_SESSION *in, uint8_t **pp); -// d2i_SSL_SESSION parses a serialized session from the |length| bytes pointed -// to by |*pp|, as described in |d2i_SAMPLE|. +// d2i_SSL_SESSION parses a serialized session from the |len| bytes pointed to +// by |*inp|, as described in |d2i_SAMPLE|. // // Use |SSL_SESSION_from_bytes| instead. -OPENSSL_EXPORT SSL_SESSION *d2i_SSL_SESSION(SSL_SESSION **a, const uint8_t **pp, - long length); +OPENSSL_EXPORT SSL_SESSION *d2i_SSL_SESSION(SSL_SESSION **out, + const uint8_t **inp, long len); // i2d_SSL_SESSION_bio serializes |session| and writes the result to |bio|. It // returns the number of bytes written on success and <= 0 on error.
diff --git a/ssl/ssl_x509.cc b/ssl/ssl_x509.cc index 51e4ffa..4fe0da8 100644 --- a/ssl/ssl_x509.cc +++ b/ssl/ssl_x509.cc
@@ -26,6 +26,7 @@ #include <openssl/x509.h> #include "../crypto/internal.h" +#include "../crypto/bytestring/internal.h" #include "internal.h" @@ -820,27 +821,11 @@ IMPLEMENT_PEM_rw(SSL_SESSION, SSL_SESSION, PEM_STRING_SSL_SESSION, SSL_SESSION) -SSL_SESSION *d2i_SSL_SESSION(SSL_SESSION **a, const uint8_t **pp, long length) { - if (length < 0) { - OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); - return NULL; - } - - CBS cbs; - CBS_init(&cbs, *pp, length); - - UniquePtr<SSL_SESSION> ret = SSL_SESSION_parse(&cbs, &ssl_crypto_x509_method, - NULL /* no buffer pool */); - if (!ret) { - return NULL; - } - - if (a) { - SSL_SESSION_free(*a); - *a = ret.get(); - } - *pp = CBS_data(&cbs); - return ret.release(); +SSL_SESSION *d2i_SSL_SESSION(SSL_SESSION **out, const uint8_t **inp, long len) { + return bssl::D2IFromCBS(out, inp, len, [](CBS *cbs) { + return SSL_SESSION_parse(cbs, &ssl_crypto_x509_method, + nullptr /* no buffer pool */); + }); } STACK_OF(X509_NAME) *SSL_dup_CA_list(STACK_OF(X509_NAME) *list) {