Fix CMP -days option range checking and test failing with enable-ubsan

Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com>
(Merged from https://github.com/openssl/openssl/pull/12175)

12 files changed, 66 insertions, 56 deletions

diff --git a/apps/cmp.c b/apps/cmp.c
index f80410fe9c..05fae77d38 100644
--- a/apps/cmp.c
+++ b/apps/cmp.c
@@ -1880,9 +1880,12 @@ static int setup_request_ctx(OSSL_CMP_CTX *ctx, ENGINE *e)
         }
     }
 
-    if (opt_days > 0)
-        (void)OSSL_CMP_CTX_set_option(ctx, OSSL_CMP_OPT_VALIDITY_DAYS,
-                                      opt_days);
+    if (opt_days > 0
+            && !OSSL_CMP_CTX_set_option(ctx, OSSL_CMP_OPT_VALIDITY_DAYS,
+                                        opt_days)) {
+        CMP_err("could to set requested cert validity period");
+        goto err;
+    }
 
     if (opt_policies != NULL && opt_policy_oids != NULL) {
         CMP_err("cannot have policies both via -policies and via -policy_oids");
diff --git a/crypto/cmp/cmp_ctx.c b/crypto/cmp/cmp_ctx.c
index 9f70de5038..558414bb5c 100644
--- a/crypto/cmp/cmp_ctx.c
+++ b/crypto/cmp/cmp_ctx.c
@@ -916,14 +916,14 @@ int OSSL_CMP_CTX_set_option(OSSL_CMP_CTX *ctx, int opt, int val)
         break;
     }
     if (val < min_val) {
-        CMPerr(0, CMP_R_INVALID_ARGS);
+        CMPerr(0, CMP_R_VALUE_TOO_SMALL);
         return 0;
     }
 
     switch (opt) {
     case OSSL_CMP_OPT_LOG_VERBOSITY:
         if (val > OSSL_CMP_LOG_DEBUG) {
-            CMPerr(0, CMP_R_INVALID_ARGS);
+            CMPerr(0, CMP_R_VALUE_TOO_LARGE);
             return 0;
         }
         ctx->log_verbosity = val;
@@ -957,7 +957,7 @@ int OSSL_CMP_CTX_set_option(OSSL_CMP_CTX *ctx, int opt, int val)
         break;
     case OSSL_CMP_OPT_POPO_METHOD:
         if (val > OSSL_CRMF_POPO_KEYAGREE) {
-            CMPerr(0, CMP_R_INVALID_ARGS);
+            CMPerr(0, CMP_R_VALUE_TOO_LARGE);
             return 0;
         }
         ctx->popoMethod = val;
@@ -982,13 +982,13 @@ int OSSL_CMP_CTX_set_option(OSSL_CMP_CTX *ctx, int opt, int val)
         break;
     case OSSL_CMP_OPT_REVOCATION_REASON:
         if (val > OCSP_REVOKED_STATUS_AACOMPROMISE) {
-            CMPerr(0, CMP_R_INVALID_ARGS);
+            CMPerr(0, CMP_R_VALUE_TOO_LARGE);
             return 0;
         }
         ctx->revocationReason = val;
         break;
     default:
-        CMPerr(0, CMP_R_INVALID_ARGS);
+        CMPerr(0, CMP_R_INVALID_OPTION);
         return 0;
     }
 
@@ -1044,7 +1044,7 @@ int OSSL_CMP_CTX_get_option(const OSSL_CMP_CTX *ctx, int opt)
     case OSSL_CMP_OPT_REVOCATION_REASON:
         return ctx->revocationReason;
     default:
-        CMPerr(0, CMP_R_INVALID_ARGS);
+        CMPerr(0, CMP_R_INVALID_OPTION);
         return -1;
     }
 }
diff --git a/crypto/cmp/cmp_err.c b/crypto/cmp/cmp_err.c
index 5f2f713b08..1ee1002233 100644
--- a/crypto/cmp/cmp_err.c
+++ b/crypto/cmp/cmp_err.c
@@ -85,6 +85,7 @@ static const ERR_STRING_DATA CMP_str_reasons[] = {
     {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_FAIL_INFO_OUT_OF_RANGE),
     "fail info out of range"},
     {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_INVALID_ARGS), "invalid args"},
+    {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_INVALID_OPTION), "invalid option"},
     {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_MISSING_KEY_INPUT_FOR_CREATING_PROTECTION),
     "missing key input for creating protection"},
     {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_MISSING_KEY_USAGE_DIGITALSIGNATURE),
@@ -143,6 +144,8 @@ static const ERR_STRING_DATA CMP_str_reasons[] = {
     "unsupported key type"},
     {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_UNSUPPORTED_PROTECTION_ALG_DHBASEDMAC),
     "unsupported protection alg dhbasedmac"},
+    {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_VALUE_TOO_LARGE), "value too large"},
+    {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_VALUE_TOO_SMALL), "value too small"},
     {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_WRONG_ALGORITHM_OID),
     "wrong algorithm oid"},
     {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_WRONG_CERTID_IN_RP), "wrong certid in rp"},
diff --git a/crypto/cmp/cmp_msg.c b/crypto/cmp/cmp_msg.c
index 9735a1c0b7..bbc3e9157e 100644
--- a/crypto/cmp/cmp_msg.c
+++ b/crypto/cmp/cmp_msg.c
@@ -253,12 +253,17 @@ static OSSL_CRMF_MSG *crm_new(OSSL_CMP_CTX *ctx, int bodytype, int rid)
                                             NULL /* serial */))
         goto err;
     if (ctx->days != 0) {
-        time_t notBefore, notAfter;
-
-        notBefore = time(NULL);
-        notAfter = notBefore + 60 * 60 * 24 * ctx->days;
-        if (!OSSL_CRMF_MSG_set_validity(crm, notBefore, notAfter))
+        time_t now = time(NULL);
+        ASN1_TIME *notBefore = ASN1_TIME_adj(NULL, now, 0, 0);
+        ASN1_TIME *notAfter = ASN1_TIME_adj(NULL, now, ctx->days, 0);
+
+        if (notBefore == NULL
+                || notAfter == NULL
+                || !OSSL_CRMF_MSG_set0_validity(crm, notBefore, notAfter)) {
+            ASN1_TIME_free(notBefore);
+            ASN1_TIME_free(notAfter);
             goto err;
+        }
     }
 
     /* extensions */
diff --git a/crypto/crmf/crmf_lib.c b/crypto/crmf/crmf_lib.c
index c20a6da0f2..7530120ff3 100644
--- a/crypto/crmf/crmf_lib.c
+++ b/crypto/crmf/crmf_lib.c
@@ -244,35 +244,23 @@ OSSL_CRMF_CERTTEMPLATE *OSSL_CRMF_MSG_get0_tmpl(const OSSL_CRMF_MSG *crm)
 }
 
 
-int OSSL_CRMF_MSG_set_validity(OSSL_CRMF_MSG *crm, time_t from, time_t to)
+int OSSL_CRMF_MSG_set0_validity(OSSL_CRMF_MSG *crm,
+                                ASN1_TIME *notBefore, ASN1_TIME *notAfter)
 {
-    OSSL_CRMF_OPTIONALVALIDITY *vld = NULL;
-    ASN1_TIME *from_asn = NULL;
-    ASN1_TIME *to_asn = NULL;
+    OSSL_CRMF_OPTIONALVALIDITY *vld;
     OSSL_CRMF_CERTTEMPLATE *tmpl = OSSL_CRMF_MSG_get0_tmpl(crm);
 
     if (tmpl == NULL) { /* also crm == NULL implies this */
-        CRMFerr(CRMF_F_OSSL_CRMF_MSG_SET_VALIDITY, CRMF_R_NULL_ARGUMENT);
+        CRMFerr(CRMF_F_OSSL_CRMF_MSG_SET0_VALIDITY, CRMF_R_NULL_ARGUMENT);
         return 0;
     }
 
-    if (from != 0 && ((from_asn = ASN1_TIME_set(NULL, from)) == NULL))
-        goto err;
-    if (to != 0 && ((to_asn = ASN1_TIME_set(NULL, to)) == NULL))
-        goto err;
     if ((vld = OSSL_CRMF_OPTIONALVALIDITY_new()) == NULL)
-        goto err;
-
-    vld->notBefore = from_asn;
-    vld->notAfter = to_asn;
-
+        return 0;
+    vld->notBefore = notBefore;
+    vld->notAfter = notAfter;
     tmpl->validity = vld;
-
     return 1;
- err:
-    ASN1_TIME_free(from_asn);
-    ASN1_TIME_free(to_asn);
-    return 0;
 }
 
 
diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt
index a30b808a25..1585688c83 100644
--- a/crypto/err/openssl.txt
+++ b/crypto/err/openssl.txt
@@ -378,7 +378,7 @@ CRMF_F_OSSL_CRMF_MSG_SET0_SINGLEPUBINFO:113:OSSL_CRMF_MSG_set0_SinglePubInfo
 CRMF_F_OSSL_CRMF_MSG_SET_CERTREQID:114:OSSL_CRMF_MSG_set_certReqId
 CRMF_F_OSSL_CRMF_MSG_SET_PKIPUBLICATIONINFO_ACTION:115:\
 	OSSL_CRMF_MSG_set_PKIPublicationInfo_action
-CRMF_F_OSSL_CRMF_MSG_SET_VALIDITY:116:OSSL_CRMF_MSG_set_validity
+CRMF_F_OSSL_CRMF_MSG_SET0_VALIDITY:116:OSSL_CRMF_MSG_set0_validity
 CRMF_F_OSSL_CRMF_PBMP_NEW:117:OSSL_CRMF_pbmp_new
 CRMF_F_OSSL_CRMF_PBM_NEW:118:OSSL_CRMF_pbm_new
 CRYPTO_F_CMAC_CTX_NEW:120:CMAC_CTX_new
@@ -2119,6 +2119,7 @@ CMP_R_FAILED_EXTRACTING_PUBKEY:141:failed extracting pubkey
 CMP_R_FAILURE_OBTAINING_RANDOM:110:failure obtaining random
 CMP_R_FAIL_INFO_OUT_OF_RANGE:129:fail info out of range
 CMP_R_INVALID_ARGS:100:invalid args
+CMP_R_INVALID_OPTION:174:invalid option
 CMP_R_MISSING_KEY_INPUT_FOR_CREATING_PROTECTION:130:\
 	missing key input for creating protection
 CMP_R_MISSING_KEY_USAGE_DIGITALSIGNATURE:142:missing key usage digitalsignature
@@ -2157,6 +2158,8 @@ CMP_R_UNSUPPORTED_ALGORITHM:136:unsupported algorithm
 CMP_R_UNSUPPORTED_KEY_TYPE:137:unsupported key type
 CMP_R_UNSUPPORTED_PROTECTION_ALG_DHBASEDMAC:154:\
 	unsupported protection alg dhbasedmac
+CMP_R_VALUE_TOO_LARGE:175:value too large
+CMP_R_VALUE_TOO_SMALL:177:value too small
 CMP_R_WRONG_ALGORITHM_OID:138:wrong algorithm oid
 CMP_R_WRONG_CERTID_IN_RP:187:wrong certid in rp
 CMP_R_WRONG_PBM_VALUE:155:wrong pbm value
diff --git a/doc/man3/OSSL_CRMF_MSG_set_validity.pod b/doc/man3/OSSL_CRMF_MSG_set0_validity.pod
index 2544c65775..3f86bfaf07 100644
--- a/doc/man3/OSSL_CRMF_MSG_set_validity.pod
+++ b/doc/man3/OSSL_CRMF_MSG_set0_validity.pod
@@ -2,7 +2,7 @@
 
 =head1 NAME
 
-OSSL_CRMF_MSG_set_validity,
+OSSL_CRMF_MSG_set0_validity,
 OSSL_CRMF_MSG_set_certReqId,
 OSSL_CRMF_CERTTEMPLATE_fill,
 OSSL_CRMF_MSG_set0_extensions,
@@ -15,7 +15,8 @@ OSSL_CRMF_MSGS_verify_popo
 
  #include <openssl/crmf.h>
 
- int OSSL_CRMF_MSG_set_validity(OSSL_CRMF_MSG *crm, time_t from, time_t to);
+ int OSSL_CRMF_MSG_set0_validity(OSSL_CRMF_MSG *crm,
+                                 ASN1_TIME *notBefore, ASN1_TIME *notAfter);
 
  int OSSL_CRMF_MSG_set_certReqId(OSSL_CRMF_MSG *crm, int rid);
 
@@ -37,29 +38,32 @@ OSSL_CRMF_MSGS_verify_popo
 
 =head1 DESCRIPTION
 
-OSSL_CRMF_MSG_set_validity() sets B<from> as notBefore and B<to> as notAfter
-as the validity in the certTemplate of B<crm>.
+OSSL_CRMF_MSG_set0_validity() sets the I<notBefore> and I<notAfter> fields
+as validity constraints in the certTemplate of I<crm>.
+Any of the I<notBefore> and I<notAfter> parameters may be NULL,
+which means no constraint for the respective field.
+On success ownership of I<notBefore> and I<notAfter> is transferred to I<crm>.
 
-OSSL_CRMF_MSG_set_certReqId() sets B<rid> as the certReqId of B<crm>.
+OSSL_CRMF_MSG_set_certReqId() sets I<rid> as the certReqId of I<crm>.
 
-OSSL_CRMF_CERTTEMPLATE_fill() sets those fields of the certTemplate B<tmpl>
-for which non-NULL values are provided: B<pubkey>, B<subject>, B<issuer>,
-and/or B<serial>.
-On success the reference counter of the B<pubkey> (if given) is incremented,
-while the B<subject>, B<issuer>, and B<serial> structures (if given) are copied.
+OSSL_CRMF_CERTTEMPLATE_fill() sets those fields of the certTemplate I<tmpl>
+for which non-NULL values are provided: I<pubkey>, I<subject>, I<issuer>,
+and/or I<serial>.
+On success the reference counter of the I<pubkey> (if given) is incremented,
+while the I<subject>, I<issuer>, and I<serial> structures (if given) are copied.
 
-OSSL_CRMF_MSG_set0_extensions() sets B<exts> as the extensions in the
-certTemplate of B<crm>. Frees any pre-existing ones and consumes B<exts>.
+OSSL_CRMF_MSG_set0_extensions() sets I<exts> as the extensions in the
+certTemplate of I<crm>. Frees any pre-existing ones and consumes I<exts>.
 
-OSSL_CRMF_MSG_push0_extension() pushes the X509 extension B<ext> to the
-extensions in the certTemplate of B<crm>.  Consumes B<ext>.
+OSSL_CRMF_MSG_push0_extension() pushes the X509 extension I<ext> to the
+extensions in the certTemplate of I<crm>.  Consumes I<ext>.
 
 OSSL_CRMF_MSG_create_popo() creates and sets the Proof-of-Possession (POPO)
-according to the method B<ppmtd> in B<crm>.
+according to the method I<ppmtd> in I<crm>.
 In case the method is OSSL_CRMF_POPO_SIGNATURE the POPO is calculated
-using the private B<pkey> and the digest algorithm NID B<dgst>.
+using the private I<pkey> and the digest algorithm NID I<dgst>.
 
-B<ppmtd> can be one of the following:
+I<ppmtd> can be one of the following:
 
 =over 8
 
@@ -82,7 +86,7 @@ challenge-response exchange (challengeResp) not yet supported.
 =back
 
 OSSL_CRMF_MSGS_verify_popo verifies the Proof-of-Possession of the request with
-the given B<rid> in the list of B<reqs>. Optionally accepts RAVerified.
+the given I<rid> in the list of I<reqs>. Optionally accepts RAVerified.
 
 =head1 RETURN VALUES
 
diff --git a/include/openssl/cmperr.h b/include/openssl/cmperr.h
index d1ce2256fa..d220e55c5e 100644
--- a/include/openssl/cmperr.h
+++ b/include/openssl/cmperr.h
@@ -74,6 +74,7 @@ int ERR_load_CMP_strings(void);
 #  define CMP_R_FAILURE_OBTAINING_RANDOM                   110
 #  define CMP_R_FAIL_INFO_OUT_OF_RANGE                     129
 #  define CMP_R_INVALID_ARGS                               100
+#  define CMP_R_INVALID_OPTION                             174
 #  define CMP_R_MISSING_KEY_INPUT_FOR_CREATING_PROTECTION  130
 #  define CMP_R_MISSING_KEY_USAGE_DIGITALSIGNATURE         142
 #  define CMP_R_MISSING_PRIVATE_KEY                        131
@@ -109,6 +110,8 @@ int ERR_load_CMP_strings(void);
 #  define CMP_R_UNSUPPORTED_ALGORITHM                      136
 #  define CMP_R_UNSUPPORTED_KEY_TYPE                       137
 #  define CMP_R_UNSUPPORTED_PROTECTION_ALG_DHBASEDMAC      154
+#  define CMP_R_VALUE_TOO_LARGE                            175
+#  define CMP_R_VALUE_TOO_SMALL                            177
 #  define CMP_R_WRONG_ALGORITHM_OID                        138
 #  define CMP_R_WRONG_CERTID_IN_RP                         187
 #  define CMP_R_WRONG_PBM_VALUE                            155
diff --git a/include/openssl/crmf.h b/include/openssl/crmf.h
index d262a9b759..bf9c1c6159 100644
--- a/include/openssl/crmf.h
+++ b/include/openssl/crmf.h
@@ -105,7 +105,8 @@ int OSSL_CRMF_MSG_set1_regInfo_utf8Pairs(OSSL_CRMF_MSG *msg,
 int OSSL_CRMF_MSG_set1_regInfo_certReq(OSSL_CRMF_MSG *msg,
                                        const OSSL_CRMF_CERTREQUEST *cr);
 
-int OSSL_CRMF_MSG_set_validity(OSSL_CRMF_MSG *crm, time_t from, time_t to);
+int OSSL_CRMF_MSG_set0_validity(OSSL_CRMF_MSG *crm,
+                                ASN1_TIME *notBefore, ASN1_TIME *notAfter);
 int OSSL_CRMF_MSG_set_certReqId(OSSL_CRMF_MSG *crm, int rid);
 int OSSL_CRMF_MSG_get_certReqId(const OSSL_CRMF_MSG *crm);
 int OSSL_CRMF_MSG_set0_extensions(OSSL_CRMF_MSG *crm, X509_EXTENSIONS *exts);
diff --git a/include/openssl/crmferr.h b/include/openssl/crmferr.h
index 22936c620e..17e5c85cc2 100644
--- a/include/openssl/crmferr.h
+++ b/include/openssl/crmferr.h
@@ -44,7 +44,7 @@ int ERR_load_CRMF_strings(void);
 #   define CRMF_F_OSSL_CRMF_MSG_SET0_SINGLEPUBINFO          0
 #   define CRMF_F_OSSL_CRMF_MSG_SET_CERTREQID               0
 #   define CRMF_F_OSSL_CRMF_MSG_SET_PKIPUBLICATIONINFO_ACTION 0
-#   define CRMF_F_OSSL_CRMF_MSG_SET_VALIDITY                0
+#   define CRMF_F_OSSL_CRMF_MSG_SET0_VALIDITY               0
 #   define CRMF_F_OSSL_CRMF_PBMP_NEW                        0
 #   define CRMF_F_OSSL_CRMF_PBM_NEW                         0
 # endif
diff --git a/test/recipes/81-test_cmp_cli_data/test_enrollment.csv b/test/recipes/81-test_cmp_cli_data/test_enrollment.csv
index 64fb3fdcd6..305302e180 100644
--- a/test/recipes/81-test_cmp_cli_data/test_enrollment.csv
+++ b/test/recipes/81-test_cmp_cli_data/test_enrollment.csv
@@ -27,7 +27,7 @@ expected,description, -section,val, -cmd,val, -newkey,val,val, -newkeypass,val,
 ,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,
 0,days 1, -section,, -cmd,ir, -newkey,new.key,, -newkeypass,pass:,,,BLANK,, -days,1,BLANK,,BLANK,,BLANK,,BLANK,,BLANK,,BLANK,, -certout,test.cert.pem,, -out_trusted,root.crt,,BLANK,,BLANK,,,
 0,days 0, -section,, -cmd,ir, -newkey,new.key,, -newkeypass,pass:,,,BLANK,, -days,0,BLANK,,BLANK,,BLANK,,BLANK,,BLANK,,BLANK,, -certout,test.cert.pem,, -out_trusted,root.crt,,BLANK,,BLANK,,,
-0,days 36500, -section,, -cmd,ir, -newkey,new.key,, -newkeypass,pass:,,,BLANK,, -days,36500,BLANK,,BLANK,,BLANK,,BLANK,,BLANK,,BLANK,, -certout,test.cert.pem,, -out_trusted,root.crt,,BLANK,,BLANK,,,
+0,days 365*100 beyond 2038, -section,, -cmd,ir, -newkey,new.key,, -newkeypass,pass:,,,BLANK,, -days,36500,BLANK,,BLANK,,BLANK,,BLANK,,BLANK,,BLANK,, -certout,test.cert.pem,, -out_trusted,root.crt,,BLANK,,BLANK,,,
 1,days missing arg, -section,, -cmd,ir, -newkey,new.key,, -newkeypass,pass:,,,BLANK,, -days,,BLANK,,BLANK,,BLANK,,BLANK,,BLANK,,BLANK,, -certout,test.cert.pem,, -out_trusted,root.crt,,BLANK,,BLANK,,,
 1,days negative, -section,, -cmd,ir, -newkey,new.key,, -newkeypass,pass:,,,BLANK,, -days,-10,BLANK,,BLANK,,BLANK,,BLANK,,BLANK,,BLANK,, -certout,test.cert.pem,, -out_trusted,root.crt,,BLANK,,BLANK,,,
 1,days no not integer, -section,, -cmd,ir, -newkey,new.key,, -newkeypass,pass:,,,BLANK,, -days,1.5,BLANK,,BLANK,,BLANK,,BLANK,,BLANK,,BLANK,, -certout,test.cert.pem,, -out_trusted,root.crt,,BLANK,,BLANK,,,
diff --git a/util/libcrypto.num b/util/libcrypto.num
index acaa17394e..683cddbc79 100644
--- a/util/libcrypto.num
+++ b/util/libcrypto.num
@@ -4546,7 +4546,7 @@ OSSL_CRMF_MSG_set1_regCtrl_oldCertID    ?	3_0_0	EXIST::FUNCTION:CRMF
 OSSL_CRMF_CERTID_gen                    ?	3_0_0	EXIST::FUNCTION:CRMF
 OSSL_CRMF_MSG_set1_regInfo_utf8Pairs    ?	3_0_0	EXIST::FUNCTION:CRMF
 OSSL_CRMF_MSG_set1_regInfo_certReq      ?	3_0_0	EXIST::FUNCTION:CRMF
-OSSL_CRMF_MSG_set_validity              ?	3_0_0	EXIST::FUNCTION:CRMF
+OSSL_CRMF_MSG_set0_validity             ?	3_0_0	EXIST::FUNCTION:CRMF
 OSSL_CRMF_MSG_set_certReqId             ?	3_0_0	EXIST::FUNCTION:CRMF
 OSSL_CRMF_MSG_get_certReqId             ?	3_0_0	EXIST::FUNCTION:CRMF
 OSSL_CRMF_MSG_set0_extensions           ?	3_0_0	EXIST::FUNCTION:CRMF