Download raw body.
isakmpd: don't reach into ASN1_STRING [step 2/2]
On Tue, Nov 25, 2025 at 07:07:31AM +0100, Theo Buehler wrote:
> 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.
Anyone? It's a very simple diff... It's not my fault that it is so big :)
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]