Download raw body.
isakmpd: don't reach into ASN1_STRING [step 2/2]
On Mon, Nov 24, 2025 at 03:31:58PM +0100, Theo Buehler wrote:
> beck and davidben have plans to make ASN1_STRING opaque in OpenSSL 4 [1].
> This breaks somewhere in the vicinity of 200 ports, so I don't think we'll
> flip the switch anytime soon in libressl, but I want base to be ready for
> that since that makes my life easier.
Here's the second step. This allows things that aren't really kosher in
modern X.509 like omitting seconds or allowing local times.
Since this transforms notBefore and notAfter into a string to be used in
a keynote assertion, existing libcrypto interfaces aren't suitable, so I
kept this mechanical by assigning the data and length fields to local
variables accessors and replacing tm->data by data and tm->length by
len, only occasionally fixing up whitespace and omitting parentheses.
The whole dance should be deduplicated, but I'll leave that for someone
else.
Index: x509.c
===================================================================
RCS file: /cvs/src/sbin/isakmpd/x509.c,v
diff -u -p -r1.127 x509.c
--- x509.c 25 Nov 2025 05:12:44 -0000 1.127
+++ x509.c 25 Nov 2025 05:43:37 -0000
@@ -116,7 +116,8 @@ x509_generate_kn(int id, X509 *cert)
time_t tt;
char before[15], after[15], *timecomp, *timecomp2;
ASN1_TIME *tm;
- int i;
+ const unsigned char *data;
+ int i, len;
LOG_DBG((LOG_POLICY, 90,
"x509_generate_kn: generating KeyNote policy for certificate %p",
@@ -220,8 +221,8 @@ x509_generate_kn(int id, X509 *cert)
key = NULL;
if (((tm = X509_get_notBefore(cert)) == NULL) ||
- (tm->type != V_ASN1_UTCTIME &&
- tm->type != V_ASN1_GENERALIZEDTIME)) {
+ (ASN1_STRING_type(tm) != V_ASN1_UTCTIME &&
+ ASN1_STRING_type(tm) != V_ASN1_GENERALIZEDTIME)) {
struct tm *ltm;
tt = time(NULL);
@@ -233,16 +234,18 @@ x509_generate_kn(int id, X509 *cert)
strftime(before, 14, "%Y%m%d%H%M%S", ltm);
timecomp = "LocalTimeOfDay";
} else {
- if (tm->data[tm->length - 1] == 'Z') {
+ data = ASN1_STRING_get0_data(tm);
+ len = ASN1_STRING_length(tm);
+ if (data[len - 1] == 'Z') {
timecomp = "GMTTimeOfDay";
- i = tm->length - 2;
+ i = len - 2;
} else {
timecomp = "LocalTimeOfDay";
- i = tm->length - 1;
+ i = len - 1;
}
for (; i >= 0; i--) {
- if (tm->data[i] < '0' || tm->data[i] > '9') {
+ if (data[i] < '0' || data[i] > '9') {
LOG_DBG((LOG_POLICY, 30,
"x509_generate_kn: invalid data in "
"NotValidBefore time field"));
@@ -250,64 +253,64 @@ x509_generate_kn(int id, X509 *cert)
}
}
- if (tm->type == V_ASN1_UTCTIME) {
- if ((tm->length < 10) || (tm->length > 13)) {
+ if (ASN1_STRING_type(tm) == V_ASN1_UTCTIME) {
+ if (len < 10 || len > 13) {
LOG_DBG((LOG_POLICY, 30,
"x509_generate_kn: invalid length "
"of NotValidBefore time field (%d)",
- tm->length));
+ len));
goto fail;
}
/* Validity checks. */
- if ((tm->data[2] != '0' && tm->data[2] != '1') ||
- (tm->data[2] == '0' && tm->data[3] == '0') ||
- (tm->data[2] == '1' && tm->data[3] > '2') ||
- (tm->data[4] > '3') ||
- (tm->data[4] == '0' && tm->data[5] == '0') ||
- (tm->data[4] == '3' && tm->data[5] > '1') ||
- (tm->data[6] > '2') ||
- (tm->data[6] == '2' && tm->data[7] > '3') ||
- (tm->data[8] > '5')) {
+ if ((data[2] != '0' && data[2] != '1') ||
+ (data[2] == '0' && data[3] == '0') ||
+ (data[2] == '1' && data[3] > '2') ||
+ (data[4] > '3') ||
+ (data[4] == '0' && data[5] == '0') ||
+ (data[4] == '3' && data[5] > '1') ||
+ (data[6] > '2') ||
+ (data[6] == '2' && data[7] > '3') ||
+ (data[8] > '5')) {
LOG_DBG((LOG_POLICY, 30,
"x509_generate_kn: invalid value in "
"NotValidBefore time field"));
goto fail;
}
/* Stupid UTC tricks. */
- if (tm->data[0] < '5')
+ if (data[0] < '5')
snprintf(before, sizeof before, "20%s",
- tm->data);
+ data);
else
snprintf(before, sizeof before, "19%s",
- tm->data);
- } else { /* V_ASN1_GENERICTIME */
- if ((tm->length < 12) || (tm->length > 15)) {
+ data);
+ } else { /* V_ASN1_GENERALIZEDTIME */
+ if (len < 12 || len > 15) {
LOG_DBG((LOG_POLICY, 30,
"x509_generate_kn: invalid length of "
"NotValidBefore time field (%d)",
- tm->length));
+ len));
goto fail;
}
/* Validity checks. */
- if ((tm->data[4] != '0' && tm->data[4] != '1') ||
- (tm->data[4] == '0' && tm->data[5] == '0') ||
- (tm->data[4] == '1' && tm->data[5] > '2') ||
- (tm->data[6] > '3') ||
- (tm->data[6] == '0' && tm->data[7] == '0') ||
- (tm->data[6] == '3' && tm->data[7] > '1') ||
- (tm->data[8] > '2') ||
- (tm->data[8] == '2' && tm->data[9] > '3') ||
- (tm->data[10] > '5')) {
+ if ((data[4] != '0' && data[4] != '1') ||
+ (data[4] == '0' && data[5] == '0') ||
+ (data[4] == '1' && data[5] > '2') ||
+ (data[6] > '3') ||
+ (data[6] == '0' && data[7] == '0') ||
+ (data[6] == '3' && data[7] > '1') ||
+ (data[8] > '2') ||
+ (data[8] == '2' && data[9] > '3') ||
+ (data[10] > '5')) {
LOG_DBG((LOG_POLICY, 30,
"x509_generate_kn: invalid value in "
"NotValidBefore time field"));
goto fail;
}
- snprintf(before, sizeof before, "%s", tm->data);
+ snprintf(before, sizeof before, "%s", data);
}
/* Fix missing seconds. */
- if (tm->length < 12) {
+ if (len < 12) {
before[12] = '0';
before[13] = '0';
}
@@ -317,8 +320,8 @@ x509_generate_kn(int id, X509 *cert)
tm = X509_get_notAfter(cert);
if (tm == NULL ||
- (tm->type != V_ASN1_UTCTIME &&
- tm->type != V_ASN1_GENERALIZEDTIME)) {
+ (ASN1_STRING_type(tm) != V_ASN1_UTCTIME &&
+ ASN1_STRING_type(tm) != V_ASN1_GENERALIZEDTIME)) {
struct tm *ltm;
tt = time(0);
@@ -330,16 +333,18 @@ x509_generate_kn(int id, X509 *cert)
strftime(after, 14, "%Y%m%d%H%M%S", ltm);
timecomp2 = "LocalTimeOfDay";
} else {
- if (tm->data[tm->length - 1] == 'Z') {
+ data = ASN1_STRING_get0_data(tm);
+ len = ASN1_STRING_length(tm);
+ if (data[len - 1] == 'Z') {
timecomp2 = "GMTTimeOfDay";
- i = tm->length - 2;
+ i = len - 2;
} else {
timecomp2 = "LocalTimeOfDay";
- i = tm->length - 1;
+ i = len - 1;
}
for (; i >= 0; i--) {
- if (tm->data[i] < '0' || tm->data[i] > '9') {
+ if (data[i] < '0' || data[i] > '9') {
LOG_DBG((LOG_POLICY, 30,
"x509_generate_kn: invalid data in "
"NotValidAfter time field"));
@@ -347,64 +352,64 @@ x509_generate_kn(int id, X509 *cert)
}
}
- if (tm->type == V_ASN1_UTCTIME) {
- if ((tm->length < 10) || (tm->length > 13)) {
+ if (ASN1_STRING_type(tm) == V_ASN1_UTCTIME) {
+ if (len < 10 || len > 13) {
LOG_DBG((LOG_POLICY, 30,
"x509_generate_kn: invalid length of "
"NotValidAfter time field (%d)",
- tm->length));
+ len));
goto fail;
}
/* Validity checks. */
- if ((tm->data[2] != '0' && tm->data[2] != '1') ||
- (tm->data[2] == '0' && tm->data[3] == '0') ||
- (tm->data[2] == '1' && tm->data[3] > '2') ||
- (tm->data[4] > '3') ||
- (tm->data[4] == '0' && tm->data[5] == '0') ||
- (tm->data[4] == '3' && tm->data[5] > '1') ||
- (tm->data[6] > '2') ||
- (tm->data[6] == '2' && tm->data[7] > '3') ||
- (tm->data[8] > '5')) {
+ if ((data[2] != '0' && data[2] != '1') ||
+ (data[2] == '0' && data[3] == '0') ||
+ (data[2] == '1' && data[3] > '2') ||
+ (data[4] > '3') ||
+ (data[4] == '0' && data[5] == '0') ||
+ (data[4] == '3' && data[5] > '1') ||
+ (data[6] > '2') ||
+ (data[6] == '2' && data[7] > '3') ||
+ (data[8] > '5')) {
LOG_DBG((LOG_POLICY, 30,
"x509_generate_kn: invalid value in "
"NotValidAfter time field"));
goto fail;
}
/* Stupid UTC tricks. */
- if (tm->data[0] < '5')
+ if (data[0] < '5')
snprintf(after, sizeof after, "20%s",
- tm->data);
+ data);
else
snprintf(after, sizeof after, "19%s",
- tm->data);
- } else { /* V_ASN1_GENERICTIME */
- if ((tm->length < 12) || (tm->length > 15)) {
+ data);
+ } else { /* V_ASN1_GENERALIZEDTIME */
+ if (len < 12 || len > 15) {
LOG_DBG((LOG_POLICY, 30,
"x509_generate_kn: invalid length of "
"NotValidAfter time field (%d)",
- tm->length));
+ len));
goto fail;
}
/* Validity checks. */
- if ((tm->data[4] != '0' && tm->data[4] != '1') ||
- (tm->data[4] == '0' && tm->data[5] == '0') ||
- (tm->data[4] == '1' && tm->data[5] > '2') ||
- (tm->data[6] > '3') ||
- (tm->data[6] == '0' && tm->data[7] == '0') ||
- (tm->data[6] == '3' && tm->data[7] > '1') ||
- (tm->data[8] > '2') ||
- (tm->data[8] == '2' && tm->data[9] > '3') ||
- (tm->data[10] > '5')) {
+ if ((data[4] != '0' && data[4] != '1') ||
+ (data[4] == '0' && data[5] == '0') ||
+ (data[4] == '1' && data[5] > '2') ||
+ (data[6] > '3') ||
+ (data[6] == '0' && data[7] == '0') ||
+ (data[6] == '3' && data[7] > '1') ||
+ (data[8] > '2') ||
+ (data[8] == '2' && data[9] > '3') ||
+ (data[10] > '5')) {
LOG_DBG((LOG_POLICY, 30,
"x509_generate_kn: invalid value in "
"NotValidAfter time field"));
goto fail;
}
- snprintf(after, sizeof after, "%s", tm->data);
+ snprintf(after, sizeof after, "%s", data);
}
/* Fix missing seconds. */
- if (tm->length < 12) {
+ if (len < 12) {
after[12] = '0';
after[13] = '0';
}
isakmpd: don't reach into ASN1_STRING [step 2/2]