Align with OpenSSL on constness of static ASN1_OBJECTs.
ASN1_OBJECTs are awkward. Sometimes they are static, when returned from
OBJ_nid2obj, and sometimes they are dynamic, when parsed from
crypto/asn1.
Most structures in crypto/asn1 need to support unknown OIDs and thus
must own their ASN1_OBJECTs. But they also may be initialized with
static ones in various APIs, such as X509_ALGOR_set0. To make that work,
ASN1_OBJECT_free detects static ASN1_OBJECTs and is a no-op.
Functions like X509_ALGOR_set0 take ownership, so OpenSSL has them take
a non-const ASN1_OBJECT*. To match, OBJ_nid2obj then returns a non-const
ASN1_OBJECT*, to signal that it is freeable.
However, this means OBJ_nid2obj's mutability doesn't match its return
type. In the fork, we switched OBJ_nid2obj to return const. But, in
doing so, we had to make X509_ALGOR_set0 and X509_PUBKEY_set0_param take
const ASN1_OBJECT, even though they would actually take ownership of
dynamic ASN1_OBJECTs. There are also a few internal casts with a TODO to
be const-correct.
Neither situation is ideal. (Perhaps a more sound model would be to copy
static ASN1_OBJECTs before putting them in most structs. But that would
not match current usage.) But I think aligning with OpenSSL is the
lesser evil here, since it avoids misleading set0 functions. Managing
ownership of ASN1_OBJECTs is much more common than mutating them. To
that end, I've added a note that ASN1_OBJECTs you didn't create must be
assumed immutable[*].
Update-Note: The change to OBJ_nid2obj should be compatible. The changes
to X509_PUBKEY_set0_param and X509_ALGOR_set0 may require fixing some
pointer types.
[*] This is *almost* honored by all of our functions. The exception is
c2i_ASN1_OBJECT, which instead checks the DYNAMIC flag as part of the
object reuse business. This would come up if we ever embedded
ASN1_OBJECTs directly in structs.
Change-Id: I1e6c700645c12b43323dd3887adb74e795c285b9
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46164
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/include/openssl/obj.h b/include/openssl/obj.h
index 764188f..49b7adc 100644
--- a/include/openssl/obj.h
+++ b/include/openssl/obj.h
@@ -124,9 +124,22 @@
// Getting information about nids.
-// OBJ_nid2obj returns the ASN1_OBJECT corresponding to |nid|, or NULL if |nid|
-// is unknown.
-OPENSSL_EXPORT const ASN1_OBJECT *OBJ_nid2obj(int nid);
+// OBJ_nid2obj returns the |ASN1_OBJECT| corresponding to |nid|, or NULL if
+// |nid| is unknown.
+//
+// This function returns a static, immutable |ASN1_OBJECT|. Although the output
+// is not const, callers may not mutate it. It is also not necessary to release
+// the object with |ASN1_OBJECT_free|.
+//
+// However, functions like |X509_ALGOR_set0| expect to take ownership of a
+// possibly dynamically-allocated |ASN1_OBJECT|. |ASN1_OBJECT_free| is a no-op
+// for static |ASN1_OBJECT|s, so |OBJ_nid2obj| is compatible with such
+// functions.
+//
+// Callers are encouraged to store the result of this function in a const
+// pointer. However, if using functions like |X509_ALGOR_set0|, callers may use
+// a non-const pointer and manage ownership.
+OPENSSL_EXPORT ASN1_OBJECT *OBJ_nid2obj(int nid);
// OBJ_nid2sn returns the short name for |nid|, or NULL if |nid| is unknown.
OPENSSL_EXPORT const char *OBJ_nid2sn(int nid);
