Const-correct sk_FOO_deep_copy's copy callback.
This aligns with upstream OpenSSL, so it's hopefully more compatible.
Code search says no one outside of the project uses this function, so
it's unlikely to break anyone.
Whether it makes things better is a bit of a wash: OBJ_dup and
OPENSSL_strdup loose a pointless wrapper. X509_NAME_dup gains one, but
hopefully that can be resolved once we solve the X509_NAME
const-correctness problem. CRYPTO_BUFFER_up_ref gains one... really
FOO_up_ref should have type const T * -> T *, but OpenSSL decided it
returns int, so we've got to cast.
Change-Id: Ifa6eaf26777ac7239db6021fc1eafcaed98e42c4
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56032
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/crypto/stack/stack_test.cc b/crypto/stack/stack_test.cc
index f96b942..98e5448 100644
--- a/crypto/stack/stack_test.cc
+++ b/crypto/stack/stack_test.cc
@@ -163,7 +163,7 @@
// Test both deep and shallow copies.
bssl::UniquePtr<STACK_OF(TEST_INT)> copy(sk_TEST_INT_deep_copy(
sk.get(),
- [](TEST_INT *x) -> TEST_INT * {
+ [](const TEST_INT *x) -> TEST_INT * {
return x == nullptr ? nullptr : TEST_INT_new(*x).release();
},
TEST_INT_free));
@@ -179,13 +179,12 @@
}
// Deep copies may fail. This should clean up temporaries.
- EXPECT_FALSE(sk_TEST_INT_deep_copy(sk.get(),
- [](TEST_INT *x) -> TEST_INT * {
- return x == nullptr || *x == 4
- ? nullptr
- : TEST_INT_new(*x).release();
- },
- TEST_INT_free));
+ EXPECT_FALSE(sk_TEST_INT_deep_copy(
+ sk.get(),
+ [](const TEST_INT *x) -> TEST_INT * {
+ return x == nullptr || *x == 4 ? nullptr : TEST_INT_new(*x).release();
+ },
+ TEST_INT_free));
// sk_TEST_INT_zero clears a stack, but does not free the elements.
ShallowStack shallow2(sk_TEST_INT_dup(sk.get()));
@@ -274,7 +273,9 @@
// Copies preserve comparison and sorted information.
bssl::UniquePtr<STACK_OF(TEST_INT)> copy(sk_TEST_INT_deep_copy(
sk.get(),
- [](TEST_INT *x) -> TEST_INT * { return TEST_INT_new(*x).release(); },
+ [](const TEST_INT *x) -> TEST_INT * {
+ return TEST_INT_new(*x).release();
+ },
TEST_INT_free));
ASSERT_TRUE(copy);
EXPECT_TRUE(sk_TEST_INT_is_sorted(copy.get()));
diff --git a/crypto/x509/x509_vpm.c b/crypto/x509/x509_vpm.c
index 21ad5e0..8ea2c6a 100644
--- a/crypto/x509/x509_vpm.c
+++ b/crypto/x509/x509_vpm.c
@@ -72,8 +72,6 @@
#define SET_HOST 0
#define ADD_HOST 1
-static char *str_copy(char *s) { return OPENSSL_strdup(s); }
-
static void str_free(char *s) { OPENSSL_free(s); }
#define string_stack_free(sk) sk_OPENSSL_STRING_pop_free(sk, str_free)
@@ -279,7 +277,8 @@
dest->hosts = NULL;
}
if (src->hosts) {
- dest->hosts = sk_OPENSSL_STRING_deep_copy(src->hosts, str_copy, str_free);
+ dest->hosts =
+ sk_OPENSSL_STRING_deep_copy(src->hosts, OPENSSL_strdup, str_free);
if (dest->hosts == NULL) {
return 0;
}
@@ -400,8 +399,6 @@
return 1;
}
-static ASN1_OBJECT *dup_object(ASN1_OBJECT *obj) { return OBJ_dup(obj); }
-
int X509_VERIFY_PARAM_set1_policies(X509_VERIFY_PARAM *param,
const STACK_OF(ASN1_OBJECT) *policies) {
if (!param) {
@@ -415,7 +412,7 @@
}
param->policies =
- sk_ASN1_OBJECT_deep_copy(policies, dup_object, ASN1_OBJECT_free);
+ sk_ASN1_OBJECT_deep_copy(policies, OBJ_dup, ASN1_OBJECT_free);
if (!param->policies) {
return 0;
}
diff --git a/include/openssl/stack.h b/include/openssl/stack.h
index f667f63..72da30d 100644
--- a/include/openssl/stack.h
+++ b/include/openssl/stack.h
@@ -120,7 +120,7 @@
// sk_SAMPLE_copy_func is a callback to copy an element in a stack. It should
// return the copy or NULL on error.
-typedef SAMPLE *(*sk_SAMPLE_copy_func)(SAMPLE *);
+typedef SAMPLE *(*sk_SAMPLE_copy_func)(const SAMPLE *);
// sk_SAMPLE_cmp_func is a callback to compare |*a| to |*b|. It should return a
// value < 0, 0, or > 0 if |*a| is less than, equal to, or greater than |*b|,
@@ -255,9 +255,9 @@
typedef void (*OPENSSL_sk_free_func)(void *ptr);
// OPENSSL_sk_copy_func is a function that copies an element in a stack. Note
-// its actual type is T *(*)(T *) for some T. Low-level |sk_*| functions will be
-// passed a type-specific wrapper to call it correctly.
-typedef void *(*OPENSSL_sk_copy_func)(void *ptr);
+// its actual type is T *(*)(const T *) for some T. Low-level |sk_*| functions
+// will be passed a type-specific wrapper to call it correctly.
+typedef void *(*OPENSSL_sk_copy_func)(const void *ptr);
// OPENSSL_sk_cmp_func is a comparison function that returns a value < 0, 0 or >
// 0 if |*a| is less than, equal to or greater than |*b|, respectively. Note
@@ -279,7 +279,7 @@
// The following function types call the above type-erased signatures with the
// true types.
typedef void (*OPENSSL_sk_call_free_func)(OPENSSL_sk_free_func, void *);
-typedef void *(*OPENSSL_sk_call_copy_func)(OPENSSL_sk_copy_func, void *);
+typedef void *(*OPENSSL_sk_call_copy_func)(OPENSSL_sk_copy_func, const void *);
typedef int (*OPENSSL_sk_call_cmp_func)(OPENSSL_sk_cmp_func,
const void *const *,
const void *const *);
@@ -386,7 +386,7 @@
DECLARE_STACK_OF(name) \
\
typedef void (*sk_##name##_free_func)(ptrtype); \
- typedef ptrtype (*sk_##name##_copy_func)(ptrtype); \
+ typedef ptrtype (*sk_##name##_copy_func)(constptrtype); \
typedef int (*sk_##name##_cmp_func)(constptrtype *, constptrtype *); \
typedef int (*sk_##name##_delete_if_func)(ptrtype, void *); \
\
@@ -396,8 +396,8 @@
} \
\
OPENSSL_INLINE void *sk_##name##_call_copy_func( \
- OPENSSL_sk_copy_func copy_func, void *ptr) { \
- return (void *)((sk_##name##_copy_func)copy_func)((ptrtype)ptr); \
+ OPENSSL_sk_copy_func copy_func, const void *ptr) { \
+ return (void *)((sk_##name##_copy_func)copy_func)((constptrtype)ptr); \
} \
\
OPENSSL_INLINE int sk_##name##_call_cmp_func(OPENSSL_sk_cmp_func cmp_func, \
diff --git a/ssl/ssl_cert.cc b/ssl/ssl_cert.cc
index 68e010a..b6f1e61 100644
--- a/ssl/ssl_cert.cc
+++ b/ssl/ssl_cert.cc
@@ -142,9 +142,9 @@
x509_method->cert_free(this);
}
-static CRYPTO_BUFFER *buffer_up_ref(CRYPTO_BUFFER *buffer) {
- CRYPTO_BUFFER_up_ref(buffer);
- return buffer;
+static CRYPTO_BUFFER *buffer_up_ref(const CRYPTO_BUFFER *buffer) {
+ CRYPTO_BUFFER_up_ref(const_cast<CRYPTO_BUFFER *>(buffer));
+ return const_cast<CRYPTO_BUFFER *>(buffer);
}
UniquePtr<CERT> ssl_cert_dup(CERT *cert) {
diff --git a/ssl/ssl_session.cc b/ssl/ssl_session.cc
index 05bcf6f..5b61eba 100644
--- a/ssl/ssl_session.cc
+++ b/ssl/ssl_session.cc
@@ -214,9 +214,9 @@
}
}
if (session->certs != nullptr) {
- auto buf_up_ref = [](CRYPTO_BUFFER *buf) {
- CRYPTO_BUFFER_up_ref(buf);
- return buf;
+ auto buf_up_ref = [](const CRYPTO_BUFFER *buf) {
+ CRYPTO_BUFFER_up_ref(const_cast<CRYPTO_BUFFER *>(buf));
+ return const_cast<CRYPTO_BUFFER*>(buf);
};
new_session->certs.reset(sk_CRYPTO_BUFFER_deep_copy(
session->certs.get(), buf_up_ref, CRYPTO_BUFFER_free));
diff --git a/ssl/ssl_x509.cc b/ssl/ssl_x509.cc
index 5533c7f..c89d4ed 100644
--- a/ssl/ssl_x509.cc
+++ b/ssl/ssl_x509.cc
@@ -1041,7 +1041,11 @@
}
STACK_OF(X509_NAME) *SSL_dup_CA_list(STACK_OF(X509_NAME) *list) {
- return sk_X509_NAME_deep_copy(list, X509_NAME_dup, X509_NAME_free);
+ // TODO(https://crbug.com/boringssl/407): |X509_NAME_dup| should be const.
+ auto name_dup = [](const X509_NAME *name) {
+ return X509_NAME_dup(const_cast<X509_NAME *>(name));
+ };
+ return sk_X509_NAME_deep_copy(list, name_dup, X509_NAME_free);
}
static void set_client_CA_list(UniquePtr<STACK_OF(CRYPTO_BUFFER)> *ca_list,
diff --git a/tool/speed.cc b/tool/speed.cc
index ee4b830..d600c28 100644
--- a/tool/speed.cc
+++ b/tool/speed.cc
@@ -1062,9 +1062,9 @@
}
static TRUST_TOKEN_PRETOKEN *trust_token_pretoken_dup(
- TRUST_TOKEN_PRETOKEN *in) {
- return (TRUST_TOKEN_PRETOKEN *)OPENSSL_memdup(in,
- sizeof(TRUST_TOKEN_PRETOKEN));
+ const TRUST_TOKEN_PRETOKEN *in) {
+ return static_cast<TRUST_TOKEN_PRETOKEN *>(
+ OPENSSL_memdup(in, sizeof(TRUST_TOKEN_PRETOKEN)));
}
static bool SpeedTrustToken(std::string name, const TRUST_TOKEN_METHOD *method,
