X509{,_CRL,_REVOKED}_{set,sign}*(): fix 'modified' field and return values

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com>
(Merged from https://github.com/openssl/openssl/pull/19271)

crypto/x509/x509_set.c

@@ -23,16 +23,22 @@ int X509_set_version(X509 *x, long version)
 {
     if (x == NULL)
         return 0;
-    if (version == 0) {
+    if (version == X509_get_version(x))
+        return 1; /* avoid needless modification even re-allocation */
+    if (version == X509_VERSION_1) {
         ASN1_INTEGER_free(x->cert_info.version);
         x->cert_info.version = NULL;
+        x->cert_info.enc.modified = 1;
         return 1;
     }
     if (x->cert_info.version == NULL) {
         if ((x->cert_info.version = ASN1_INTEGER_new()) == NULL)
             return 0;
     }
-    return ASN1_INTEGER_set(x->cert_info.version, version);
+    if (!ASN1_INTEGER_set(x->cert_info.version, version))
+        return 0;
+    x->cert_info.enc.modified = 1;
+    return 1;
 }
 
 int X509_set_serialNumber(X509 *x, ASN1_INTEGER *serial)
@@ -44,56 +50,66 @@ int X509_set_serialNumber(X509 *x, ASN1_INTEGER *serial)
     in = &x->cert_info.serialNumber;
     if (in != serial)
         return ASN1_STRING_copy(in, serial);
+    x->cert_info.enc.modified = 1;
     return 1;
 }
 
 int X509_set_issuer_name(X509 *x, const X509_NAME *name)
 {
-    if (x == NULL)
+    if (x == NULL || !X509_NAME_set(&x->cert_info.issuer, name))
         return 0;
-    return X509_NAME_set(&x->cert_info.issuer, name);
+    x->cert_info.enc.modified = 1;
+    return 1;
 }
 
 int X509_set_subject_name(X509 *x, const X509_NAME *name)
 {
-    if (x == NULL)
+    if (x == NULL || !X509_NAME_set(&x->cert_info.subject, name))
         return 0;
-    return X509_NAME_set(&x->cert_info.subject, name);
+    x->cert_info.enc.modified = 1;
+    return 1;
 }
 
-int ossl_x509_set1_time(ASN1_TIME **ptm, const ASN1_TIME *tm)
+int ossl_x509_set1_time(int *modified, ASN1_TIME **ptm, const ASN1_TIME *tm)
 {
-    ASN1_TIME *in;
-    in = *ptm;
-    if (in != tm) {
-        in = ASN1_STRING_dup(tm);
-        if (in != NULL) {
-            ASN1_TIME_free(*ptm);
-            *ptm = in;
-        }
-    }
-    return (in != NULL);
+    ASN1_TIME *new;
+
+    if (*ptm == tm)
+        return 1;
+    new = ASN1_STRING_dup(tm);
+    if (tm != NULL && new == NULL)
+        return 0;
+    ASN1_TIME_free(*ptm);
+    *ptm = new;
+    if (modified != NULL)
+        *modified = 1;
+    return 1;
 }
 
 int X509_set1_notBefore(X509 *x, const ASN1_TIME *tm)
 {
-    if (x == NULL)
+    if (x == NULL || tm == NULL)
         return 0;
-    return ossl_x509_set1_time(&x->cert_info.validity.notBefore, tm);
+    return ossl_x509_set1_time(&x->cert_info.enc.modified,
+                               &x->cert_info.validity.notBefore, tm);
 }
 
 int X509_set1_notAfter(X509 *x, const ASN1_TIME *tm)
 {
-    if (x == NULL)
+    if (x == NULL || tm == NULL)
         return 0;
-    return ossl_x509_set1_time(&x->cert_info.validity.notAfter, tm);
+    return ossl_x509_set1_time(&x->cert_info.enc.modified,
+                               &x->cert_info.validity.notAfter, tm);
 }
 
 int X509_set_pubkey(X509 *x, EVP_PKEY *pkey)
 {
     if (x == NULL)
         return 0;
-    return X509_PUBKEY_set(&(x->cert_info.key), pkey);
+    if (!X509_PUBKEY_set(&(x->cert_info.key), pkey))
+        return 0;
+    x->cert_info.enc.modified = 1;
+    return 1;
 }
 
 int X509_up_ref(X509 *x)
@@ -105,7 +121,7 @@ int X509_up_ref(X509 *x)
 
     REF_PRINT_COUNT("X509", x);
     REF_ASSERT_ISNT(i < 2);
-    return ((i > 1) ? 1 : 0);
+    return i > 1;
 }
 
 long X509_get_version(const X509 *x)

crypto/x509/x509cset.c

@@ -24,34 +24,41 @@ int X509_CRL_set_version(X509_CRL *x, long version)
         if ((x->crl.version = ASN1_INTEGER_new()) == NULL)
             return 0;
     }
-    return ASN1_INTEGER_set(x->crl.version, version);
+    if (!ASN1_INTEGER_set(x->crl.version, version))
+        return 0;
+    x->crl.enc.modified = 1;
+    return 1;
 }
 
 int X509_CRL_set_issuer_name(X509_CRL *x, const X509_NAME *name)
 {
     if (x == NULL)
         return 0;
-    return X509_NAME_set(&x->crl.issuer, name);
+    if (!X509_NAME_set(&x->crl.issuer, name))
+        return 0;
+    x->crl.enc.modified = 1;
+    return 1;
 }
 
 int X509_CRL_set1_lastUpdate(X509_CRL *x, const ASN1_TIME *tm)
 {
-    if (x == NULL)
+    if (x == NULL || tm == NULL)
         return 0;
-    return ossl_x509_set1_time(&x->crl.lastUpdate, tm);
+    return ossl_x509_set1_time(&x->crl.enc.modified, &x->crl.lastUpdate, tm);
 }
 
 int X509_CRL_set1_nextUpdate(X509_CRL *x, const ASN1_TIME *tm)
 {
     if (x == NULL)
         return 0;
-    return ossl_x509_set1_time(&x->crl.nextUpdate, tm);
+    return ossl_x509_set1_time(&x->crl.enc.modified, &x->crl.nextUpdate, tm);
 }
 
 int X509_CRL_sort(X509_CRL *c)
 {
     int i;
     X509_REVOKED *r;
+
     /*
      * sort the data so it will be written in serial number order
      */
@@ -73,7 +80,7 @@ int X509_CRL_up_ref(X509_CRL *crl)
 
     REF_PRINT_COUNT("X509_CRL", crl);
     REF_ASSERT_ISNT(i < 2);
-    return ((i > 1) ? 1 : 0);
+    return i > 1;
 }
 
 long X509_CRL_get_version(const X509_CRL *crl)
@@ -139,19 +146,9 @@ const ASN1_TIME *X509_REVOKED_get0_revocationDate(const X509_REVOKED *x)
 
 int X509_REVOKED_set_revocationDate(X509_REVOKED *x, ASN1_TIME *tm)
 {
-    ASN1_TIME *in;
-
-    if (x == NULL)
+    if (x == NULL || tm == NULL)
         return 0;
-    in = x->revocationDate;
-    if (in != tm) {
-        in = ASN1_STRING_dup(tm);
-        if (in != NULL) {
-            ASN1_TIME_free(x->revocationDate);
-            x->revocationDate = in;
-        }
-    }
-    return (in != NULL);
+    return ossl_x509_set1_time(NULL, &x->revocationDate, tm);
 }
 
 const ASN1_INTEGER *X509_REVOKED_get0_serialNumber(const X509_REVOKED *x)
@@ -171,7 +168,8 @@ int X509_REVOKED_set_serialNumber(X509_REVOKED *x, ASN1_INTEGER *serial)
     return 1;
 }
 
-const STACK_OF(X509_EXTENSION) *X509_REVOKED_get0_extensions(const X509_REVOKED *r)
+const STACK_OF(X509_EXTENSION) *X509_REVOKED_get0_extensions(const
+                                                             X509_REVOKED *r)
 {
     return r->extensions;
 }

crypto/x509/x_all.c

@@ -282,7 +282,8 @@ X509_REQ *d2i_X509_REQ_bio(BIO *bp, X509_REQ **req)
         propq = (*req)->propq;
     }
 
-    return ASN1_item_d2i_bio_ex(ASN1_ITEM_rptr(X509_REQ), bp, req, libctx, propq);
+    return
+        ASN1_item_d2i_bio_ex(ASN1_ITEM_rptr(X509_REQ), bp, req, libctx, propq);
 }
 
 int i2d_X509_REQ_bio(BIO *bp, const X509_REQ *req)
@@ -575,15 +576,17 @@ int X509_CRL_digest(const X509_CRL *data, const EVP_MD *type,
         memcpy(md, data->sha1_hash, sizeof(data->sha1_hash));
         return 1;
     }
-    return ossl_asn1_item_digest_ex(ASN1_ITEM_rptr(X509_CRL), type, (char *)data,
-                                    md, len, data->libctx, data->propq);
+    return
+        ossl_asn1_item_digest_ex(ASN1_ITEM_rptr(X509_CRL), type, (char *)data,
+                                 md, len, data->libctx, data->propq);
 }
 
 int X509_REQ_digest(const X509_REQ *data, const EVP_MD *type,
                     unsigned char *md, unsigned int *len)
 {
-    return ossl_asn1_item_digest_ex(ASN1_ITEM_rptr(X509_REQ), type, (char *)data,
-                                    md, len, data->libctx, data->propq);
+    return
+        ossl_asn1_item_digest_ex(ASN1_ITEM_rptr(X509_REQ), type, (char *)data,
+                                 md, len, data->libctx, data->propq);
 }
 
 int X509_NAME_digest(const X509_NAME *data, const EVP_MD *type,

doc/man3/X509_get0_notBefore.pod

@@ -29,7 +29,7 @@ X509_CRL_set1_nextUpdate - get or set certificate or CRL dates
 =head1 DESCRIPTION
 
 X509_get0_notBefore() and X509_get0_notAfter() return the B<notBefore>
-and B<notAfter> fields of certificate B<x> respectively. The value
+and B<notAfter> fields of certificate I<x> respectively. The value
 returned is an internal pointer which must not be freed up after
 the call.
 
@@ -39,20 +39,22 @@ non-constant mutable references to the associated date field of
 the certificate.
 
 X509_set1_notBefore() and X509_set1_notAfter() set the B<notBefore>
-and B<notAfter> fields of B<x> to B<tm>. Ownership of the passed
-parameter B<tm> is not transferred by these functions so it must
+and B<notAfter> fields of I<x> to I<tm>. Ownership of the passed
+parameter I<tm> is not transferred by these functions so it must
 be freed up after the call.
 
 X509_CRL_get0_lastUpdate() and X509_CRL_get0_nextUpdate() return the
-B<lastUpdate> and B<nextUpdate> fields of B<crl>. The value
+B<lastUpdate> and B<nextUpdate> fields of I<crl>. The value
 returned is an internal pointer which must not be freed up after
-the call. If the B<nextUpdate> field is absent from B<crl> then
-B<NULL> is returned.
+the call. If the B<nextUpdate> field is absent from I<crl> then
+NULL is returned.
 
 X509_CRL_set1_lastUpdate() and X509_CRL_set1_nextUpdate() set the B<lastUpdate>
-and B<nextUpdate> fields of B<crl> to B<tm>. Ownership of the passed parameter
-B<tm> is not transferred by these functions so it must be freed up after the
+and B<nextUpdate> fields of I<crl> to I<tm>. Ownership of the passed parameter
+I<tm> is not transferred by these functions so it must be freed up after the
 call.
+For X509_CRL_set1_nextUpdate() the I<tm> argument may be NULL,
+which implies removal of the optional B<nextUpdate> field.
 
 =head1 RETURN VALUES
 

include/crypto/x509.h

@@ -309,7 +309,7 @@ struct x509_object_st {
 };
 
 int ossl_a2i_ipadd(unsigned char *ipout, const char *ipasc);
-int ossl_x509_set1_time(ASN1_TIME **ptm, const ASN1_TIME *tm);
+int ossl_x509_set1_time(int *modified, ASN1_TIME **ptm, const ASN1_TIME *tm);
 int ossl_x509_print_ex_brief(BIO *bio, X509 *cert, unsigned long neg_cflags);
 int ossl_x509v3_cache_extensions(X509 *x);
 int ossl_x509_init_sig_info(X509 *x);