Introduce constants for ASN1_BOOLEAN
Between the type being sometimes a tri-state and capturing the
underlying DER/BER representation, this type is a bit confusing. Add
constants for these.
I've left a case in ASN1_TBOOLEAN unconverted because it's actually
wrong. It will be fixed in a subsequent CL.
Change-Id: I75d846af14f6b4cd5278c3833b979e89c92c4203
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56487
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/crypto/asn1/a_bool.c b/crypto/asn1/a_bool.c
index 2a4448c..8dc84d4 100644
--- a/crypto/asn1/a_bool.c
+++ b/crypto/asn1/a_bool.c
@@ -78,7 +78,7 @@
}
ASN1_put_object(&p, 0, 1, V_ASN1_BOOLEAN, V_ASN1_UNIVERSAL);
- *p = a ? 0xff : 0x00;
+ *p = a ? ASN1_BOOLEAN_TRUE : ASN1_BOOLEAN_FALSE;
// If a new buffer was allocated, just return it back.
// If not, return the incremented buffer pointer.
@@ -94,22 +94,22 @@
inf = ASN1_get_object(&p, &len, &tag, &xclass, length);
if (inf & 0x80) {
OPENSSL_PUT_ERROR(ASN1, ASN1_R_BAD_OBJECT_HEADER);
- return -1;
+ return ASN1_BOOLEAN_NONE;
}
if (inf & V_ASN1_CONSTRUCTED) {
OPENSSL_PUT_ERROR(ASN1, ASN1_R_TYPE_NOT_PRIMITIVE);
- return -1;
+ return ASN1_BOOLEAN_NONE;
}
if (tag != V_ASN1_BOOLEAN || xclass != V_ASN1_UNIVERSAL) {
OPENSSL_PUT_ERROR(ASN1, ASN1_R_EXPECTING_A_BOOLEAN);
- return -1;
+ return ASN1_BOOLEAN_NONE;
}
if (len != 1) {
OPENSSL_PUT_ERROR(ASN1, ASN1_R_BOOLEAN_IS_WRONG_LENGTH);
- return -1;
+ return ASN1_BOOLEAN_NONE;
}
ASN1_BOOLEAN ret = (ASN1_BOOLEAN) * (p++);
if (a != NULL) {
diff --git a/crypto/asn1/tasn_enc.c b/crypto/asn1/tasn_enc.c
index 5b9a8d4..afac17d 100644
--- a/crypto/asn1/tasn_enc.c
+++ b/crypto/asn1/tasn_enc.c
@@ -631,7 +631,7 @@
case V_ASN1_BOOLEAN:
tbool = (ASN1_BOOLEAN *)pval;
- if (*tbool == -1) {
+ if (*tbool == ASN1_BOOLEAN_NONE) {
*out_omit = 1;
return 0;
}
diff --git a/crypto/asn1/tasn_typ.c b/crypto/asn1/tasn_typ.c
index abfac93..a9409e2 100644
--- a/crypto/asn1/tasn_typ.c
+++ b/crypto/asn1/tasn_typ.c
@@ -107,9 +107,10 @@
DIRECTORYSTRING)
// Three separate BOOLEAN type: normal, DEFAULT TRUE and DEFAULT FALSE
-IMPLEMENT_ASN1_TYPE_ex(ASN1_BOOLEAN, ASN1_BOOLEAN, -1)
+// TODO(davidben): ASN1_TBOOLEAN should be ASN1_BOOLEAN_TRUE, or 0xff.
+IMPLEMENT_ASN1_TYPE_ex(ASN1_BOOLEAN, ASN1_BOOLEAN, ASN1_BOOLEAN_NONE)
IMPLEMENT_ASN1_TYPE_ex(ASN1_TBOOLEAN, ASN1_BOOLEAN, 1)
-IMPLEMENT_ASN1_TYPE_ex(ASN1_FBOOLEAN, ASN1_BOOLEAN, 0)
+IMPLEMENT_ASN1_TYPE_ex(ASN1_FBOOLEAN, ASN1_BOOLEAN, ASN1_BOOLEAN_FALSE)
ASN1_ITEM_TEMPLATE(ASN1_SEQUENCE_ANY) =
ASN1_EX_TEMPLATE_TYPE(ASN1_TFLG_SEQUENCE_OF, 0, ASN1_SEQUENCE_ANY, ASN1_ANY)
diff --git a/crypto/x509/x509_v3.c b/crypto/x509/x509_v3.c
index 4b88ea7..9153dce 100644
--- a/crypto/x509/x509_v3.c
+++ b/crypto/x509/x509_v3.c
@@ -250,7 +250,9 @@
if (ex == NULL) {
return 0;
}
- ex->critical = (crit) ? 0xFF : -1;
+ // The critical field is DEFAULT FALSE, so non-critical extensions should omit
+ // the value.
+ ex->critical = crit ? ASN1_BOOLEAN_TRUE : ASN1_BOOLEAN_NONE;
return 1;
}
diff --git a/crypto/x509v3/internal.h b/crypto/x509v3/internal.h
index 0632f23..51e15e4 100644
--- a/crypto/x509v3/internal.h
+++ b/crypto/x509v3/internal.h
@@ -134,7 +134,7 @@
int X509V3_NAME_from_section(X509_NAME *nm, const STACK_OF(CONF_VALUE) *dn_sk,
int chtype);
-int X509V3_get_value_bool(const CONF_VALUE *value, int *asn1_bool);
+int X509V3_get_value_bool(const CONF_VALUE *value, ASN1_BOOLEAN *asn1_bool);
int X509V3_get_value_int(const CONF_VALUE *value, ASN1_INTEGER **aint);
const STACK_OF(CONF_VALUE) *X509V3_get_section(const X509V3_CTX *ctx,
const char *section);
diff --git a/crypto/x509v3/v3_utl.c b/crypto/x509v3/v3_utl.c
index bb5db40..eec7d08 100644
--- a/crypto/x509v3/v3_utl.c
+++ b/crypto/x509v3/v3_utl.c
@@ -300,19 +300,19 @@
return ret;
}
-int X509V3_get_value_bool(const CONF_VALUE *value, int *asn1_bool) {
+int X509V3_get_value_bool(const CONF_VALUE *value, ASN1_BOOLEAN *asn1_bool) {
char *btmp;
if (!(btmp = value->value)) {
goto err;
}
if (!strcmp(btmp, "TRUE") || !strcmp(btmp, "true") || !strcmp(btmp, "Y") ||
!strcmp(btmp, "y") || !strcmp(btmp, "YES") || !strcmp(btmp, "yes")) {
- *asn1_bool = 0xff;
+ *asn1_bool = ASN1_BOOLEAN_TRUE;
return 1;
} else if (!strcmp(btmp, "FALSE") || !strcmp(btmp, "false") ||
!strcmp(btmp, "N") || !strcmp(btmp, "n") || !strcmp(btmp, "NO") ||
!strcmp(btmp, "no")) {
- *asn1_bool = 0;
+ *asn1_bool = ASN1_BOOLEAN_FALSE;
return 1;
}
err:
diff --git a/include/openssl/asn1.h b/include/openssl/asn1.h
index b402c1d..7866deb 100644
--- a/include/openssl/asn1.h
+++ b/include/openssl/asn1.h
@@ -447,10 +447,22 @@
// integer type. FALSE is zero, TRUE is 0xff, and an omitted OPTIONAL BOOLEAN is
// -1.
+// ASN1_BOOLEAN_FALSE is FALSE as an |ASN1_BOOLEAN|.
+#define ASN1_BOOLEAN_FALSE 0
+
+// ASN1_BOOLEAN_TRUE is TRUE as an |ASN1_BOOLEAN|. Some code incorrectly uses
+// 1, so prefer |b != ASN1_BOOLEAN_FALSE| over |b == ASN1_BOOLEAN_TRUE|.
+#define ASN1_BOOLEAN_TRUE 0xff
+
+// ASN1_BOOLEAN_NONE, in contexts where the |ASN1_BOOLEAN| represents an
+// OPTIONAL BOOLEAN, is an omitted value. Using this value in other contexts is
+// undefined and may be misinterpreted as TRUE.
+#define ASN1_BOOLEAN_NONE (-1)
+
// d2i_ASN1_BOOLEAN parses a DER-encoded ASN.1 BOOLEAN from up to |len| bytes at
// |*inp|. On success, it advances |*inp| by the number of bytes read and
// returns the result. If |out| is non-NULL, it additionally writes the result
-// to |*out|. On error, it returns -1.
+// to |*out|. On error, it returns |ASN1_BOOLEAN_NONE|.
//
// This function does not reject trailing data in the input. This allows the
// caller to parse a sequence of concatenated structures. Callers parsing only
@@ -472,7 +484,8 @@
// The following |ASN1_ITEM|s have ASN.1 type BOOLEAN and C type |ASN1_BOOLEAN|.
// |ASN1_TBOOLEAN| and |ASN1_FBOOLEAN| must be marked OPTIONAL. When omitted,
-// they are parsed as TRUE and FALSE, respectively, rather than -1.
+// they are parsed as TRUE and FALSE, respectively, rather than
+// |ASN1_BOOLEAN_NONE|.
DECLARE_ASN1_ITEM(ASN1_BOOLEAN)
DECLARE_ASN1_ITEM(ASN1_TBOOLEAN)
DECLARE_ASN1_ITEM(ASN1_FBOOLEAN)
