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)

3 files changed, 64 insertions, 47 deletions

diff --git a/crypto/x509/x509_set.c b/crypto/x509/x509_set.c
index d8ddde8aaa..e804380a64 100644
--- a/crypto/x509/x509_set.c
+++ b/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)
diff --git a/crypto/x509/x509cset.c b/crypto/x509/x509cset.c
index 2746b98925..840b15c61c 100644
--- a/crypto/x509/x509cset.c
+++ b/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;
 }
diff --git a/crypto/x509/x_all.c b/crypto/x509/x_all.c
index 075ea1ac03..c3f5ee11df 100644
--- a/crypto/x509/x_all.c
+++ b/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,