Fix X509_ATTRIBUTE_set1_data with negative attributes
One of the WARNINGs in this function is unambiguously a bug. Just fix
it. In doing so, add a helper for the ASN1_STRING -> ASN1_TYPE
conversion and make this function easier to follow.
Also document the attrtype == 0 case. I missed that one when enumerating
this overcomplicated calling convention. Move that check earlier so we
don't try to process the input. It's ignored anyway.
Change-Id: Ia6e2dcb7c69488b6e2e58a68c3b701040ed3d4ef
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64827
Reviewed-by: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/include/openssl/x509.h b/include/openssl/x509.h
index 66668a6..852a96f 100644
--- a/include/openssl/x509.h
+++ b/include/openssl/x509.h
@@ -2101,21 +2101,21 @@
// X509_ATTRIBUTE_set1_data appends a value to |attr|'s value set and returns
// one on success or zero on error. The value is determined as follows:
//
-// If |attrtype| is a |MBSTRING_*| constant, the value is an ASN.1 string. The
-// string is determined by decoding |len| bytes from |data| in the encoding
-// specified by |attrtype|, and then re-encoding it in a form appropriate for
-// |attr|'s type. If |len| is -1, |strlen(data)| is used instead. See
-// |ASN1_STRING_set_by_NID| for details.
+// If |attrtype| is zero, this function returns one and does nothing. This form
+// may be used when calling |X509_ATTRIBUTE_create_by_*| to create an attribute
+// with an empty value set. Such attributes are invalid, but OpenSSL supports
+// creating them.
+//
+// Otherwise, if |attrtype| is a |MBSTRING_*| constant, the value is an ASN.1
+// string. The string is determined by decoding |len| bytes from |data| in the
+// encoding specified by |attrtype|, and then re-encoding it in a form
+// appropriate for |attr|'s type. If |len| is -1, |strlen(data)| is used
+// instead. See |ASN1_STRING_set_by_NID| for details.
//
// Otherwise, if |len| is not -1, the value is an ASN.1 string. |attrtype| is an
// |ASN1_STRING| type value and the |len| bytes from |data| are copied as the
// type-specific representation of |ASN1_STRING|. See |ASN1_STRING| for details.
//
-// WARNING: If this form is used to construct a negative INTEGER or ENUMERATED,
-// |attrtype| includes the |V_ASN1_NEG| flag for |ASN1_STRING|, but the function
-// forgets to clear the flag for |ASN1_TYPE|. This matches OpenSSL but is
-// probably a bug. For now, do not use this form with negative values.
-//
// Otherwise, if |len| is -1, the value is constructed by passing |attrtype| and
// |data| to |ASN1_TYPE_set1|. That is, |attrtype| is an |ASN1_TYPE| type value,
// and |data| is cast to the corresponding pointer type.
