Document and tidy up X509_alias_get0, etc.
The getters would leave the length uninitialized when empty, which is
dangerous if the caller does not check. Instead, always fill it in.
This opens a can of worms around whether empty alias and missing alias
are meaningfully different. Treating {NULL, 0} differently from
{non-NULL, 0} has typically caused problems. At the PKCS#12 level,
neither friendlyName, nor localKeyId are allowed to be empty, which
suggests we should not distinguish. However, X509_CERT_AUX, which is
serialized in i2d_X509_AUX, does distinguish the two states. The getters
try to, but an empty ASN1_STRING can have NULL data pointer. (Although,
when parsed, they usually do not because OpenSSL helpfully
NUL-terminates it for you.)
For now, I've just written the documentation to suggest the empty string
is the same as the missing state. I'm thinking we can make the PKCS#12
functions not bother distinguishing the two and see how it goes. I've
also gone ahead and grouped them with d2i_X509_AUX, although the rest of
the headers has not yet been grouped into sections.
Bug: 426, 481
Change-Id: Ic9c21bc2b5ef3b012c2f812b0474f04d5232db06
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/51745
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/crypto/x509/x_x509a.c b/crypto/x509/x_x509a.c
index fca02a6..447a891 100644
--- a/crypto/x509/x_x509a.c
+++ b/crypto/x509/x_x509a.c
@@ -95,6 +95,9 @@
int X509_alias_set1(X509 *x, const unsigned char *name, int len)
{
X509_CERT_AUX *aux;
+ /* TODO(davidben): Empty aliases are not meaningful in PKCS#12, and the
+ * getters cannot quite represent them. Also erase the object if |len| is
+ * zero. */
if (!name) {
if (!x || !x->aux || !x->aux->alias)
return 1;
@@ -112,6 +115,9 @@
int X509_keyid_set1(X509 *x, const unsigned char *id, int len)
{
X509_CERT_AUX *aux;
+ /* TODO(davidben): Empty key IDs are not meaningful in PKCS#12, and the
+ * getters cannot quite represent them. Also erase the object if |len| is
+ * zero. */
if (!id) {
if (!x || !x->aux || !x->aux->keyid)
return 1;
@@ -126,22 +132,22 @@
return ASN1_STRING_set(aux->keyid, id, len);
}
-unsigned char *X509_alias_get0(X509 *x, int *len)
+unsigned char *X509_alias_get0(X509 *x, int *out_len)
{
- if (!x->aux || !x->aux->alias)
- return NULL;
- if (len)
- *len = x->aux->alias->length;
- return x->aux->alias->data;
+ const ASN1_UTF8STRING *alias = x->aux != NULL ? x->aux->alias : NULL;
+ if (out_len != NULL) {
+ *out_len = alias != NULL ? alias->length : 0;
+ }
+ return alias != NULL ? alias->data : NULL;
}
-unsigned char *X509_keyid_get0(X509 *x, int *len)
+unsigned char *X509_keyid_get0(X509 *x, int *out_len)
{
- if (!x->aux || !x->aux->keyid)
- return NULL;
- if (len)
- *len = x->aux->keyid->length;
- return x->aux->keyid->data;
+ const ASN1_OCTET_STRING *keyid = x->aux != NULL ? x->aux->keyid : NULL;
+ if (out_len != NULL) {
+ *out_len = keyid != NULL ? keyid->length : 0;
+ }
+ return keyid != NULL ? keyid->data : NULL;
}
int X509_add1_trust_object(X509 *x, ASN1_OBJECT *obj)
diff --git a/include/openssl/x509.h b/include/openssl/x509.h
index 6696988..1a45d75 100644
--- a/include/openssl/x509.h
+++ b/include/openssl/x509.h
@@ -869,9 +869,6 @@
CRYPTO_EX_free *free_func);
OPENSSL_EXPORT int X509_set_ex_data(X509 *r, int idx, void *arg);
OPENSSL_EXPORT void *X509_get_ex_data(X509 *r, int idx);
-OPENSSL_EXPORT int i2d_X509_AUX(X509 *a, unsigned char **pp);
-OPENSSL_EXPORT X509 *d2i_X509_AUX(X509 **a, const unsigned char **pp,
- long length);
// i2d_re_X509_tbs serializes the TBSCertificate portion of |x509|, as described
// in |i2d_SAMPLE|.
@@ -924,19 +921,84 @@
// a known NID.
OPENSSL_EXPORT int X509_get_signature_nid(const X509 *x509);
-OPENSSL_EXPORT int X509_alias_set1(X509 *x, const unsigned char *name, int len);
-OPENSSL_EXPORT int X509_keyid_set1(X509 *x, const unsigned char *id, int len);
-OPENSSL_EXPORT unsigned char *X509_alias_get0(X509 *x, int *len);
-OPENSSL_EXPORT unsigned char *X509_keyid_get0(X509 *x, int *len);
-OPENSSL_EXPORT int (*X509_TRUST_set_default(int (*trust)(int, X509 *,
- int)))(int, X509 *,
- int);
-OPENSSL_EXPORT int X509_TRUST_set(int *t, int trust);
+
+// Auxiliary properties.
+//
+// |X509| objects optionally maintain auxiliary properties. These are not part
+// of the certificates themselves, and thus are not covered by signatures or
+// preserved by the standard serialization. They are used as inputs or outputs
+// to other functions in this library.
+
+// i2d_X509_AUX marshals |x509| as a DER-encoded X.509 Certificate (RFC 5280),
+// followed optionally by a separate, OpenSSL-specific structure with auxiliary
+// properties. It behaves as described in |i2d_SAMPLE|.
+//
+// Unlike similarly-named functions, this function does not output a single
+// ASN.1 element. Directly embedding the output in a larger ASN.1 structure will
+// not behave correctly.
+OPENSSL_EXPORT int i2d_X509_AUX(X509 *x509, unsigned char **outp);
+
+// d2i_X509_AUX parses up to |length| bytes from |*inp| as a DER-encoded X.509
+// Certificate (RFC 5280), followed optionally by a separate, OpenSSL-specific
+// structure with auxiliary properties. It behaves as described in
+// |d2i_SAMPLE_with_reuse|.
+//
+// Some auxiliary properties affect trust decisions, so this function should not
+// be used with untrusted input.
+//
+// Unlike similarly-named functions, this function does not parse a single
+// ASN.1 element. Trying to parse data directly embedded in a larger ASN.1
+// structure will not behave correctly.
+OPENSSL_EXPORT X509 *d2i_X509_AUX(X509 **x509, const unsigned char **inp,
+ long length);
+
+// X509_alias_set1 sets |x509|'s alias to |len| bytes from |name|. If |name| is
+// NULL, the alias is cleared instead. Aliases are not part of the certificate
+// itself and will not be serialized by |i2d_X509|.
+OPENSSL_EXPORT int X509_alias_set1(X509 *x509, const unsigned char *name,
+ int len);
+
+// X509_keyid_set1 sets |x509|'s key ID to |len| bytes from |id|. If |id| is
+// NULL, the key ID is cleared instead. Key IDs are not part of the certificate
+// itself and will not be serialized by |i2d_X509|.
+OPENSSL_EXPORT int X509_keyid_set1(X509 *x509, const unsigned char *id,
+ int len);
+
+// X509_alias_get0 looks up |x509|'s alias. If found, it sets |*out_len| to the
+// alias's length and returns a pointer to a buffer containing the contents. If
+// not found, it outputs the empty string by returning NULL and setting
+// |*out_len| to zero.
+//
+// If |x509| was parsed from a PKCS#12 structure (see
+// |PKCS12_get_key_and_certs|), the alias will reflect the friendlyName
+// attribute (RFC 2985).
+//
+// WARNING: In OpenSSL, this function did not set |*out_len| when the alias was
+// missing. Callers that target both OpenSSL and BoringSSL should set the value
+// to zero before calling this function.
+OPENSSL_EXPORT unsigned char *X509_alias_get0(X509 *x509, int *out_len);
+
+// X509_keyid_get0 looks up |x509|'s key ID. If found, it sets |*out_len| to the
+// key ID's length and returns a pointer to a buffer containing the contents. If
+// not found, it outputs the empty string by returning NULL and setting
+// |*out_len| to zero.
+//
+// WARNING: In OpenSSL, this function did not set |*out_len| when the alias was
+// missing. Callers that target both OpenSSL and BoringSSL should set the value
+// to zero before calling this function.
+OPENSSL_EXPORT unsigned char *X509_keyid_get0(X509 *x509, int *out_len);
+
OPENSSL_EXPORT int X509_add1_trust_object(X509 *x, ASN1_OBJECT *obj);
OPENSSL_EXPORT int X509_add1_reject_object(X509 *x, ASN1_OBJECT *obj);
OPENSSL_EXPORT void X509_trust_clear(X509 *x);
OPENSSL_EXPORT void X509_reject_clear(X509 *x);
+
+OPENSSL_EXPORT int (*X509_TRUST_set_default(int (*trust)(int, X509 *,
+ int)))(int, X509 *,
+ int);
+OPENSSL_EXPORT int X509_TRUST_set(int *t, int trust);
+
DECLARE_ASN1_FUNCTIONS(X509_REVOKED)
DECLARE_ASN1_FUNCTIONS(X509_CRL)
