Rewrite X509_ALGOR_set0()
authortb <tb@openbsd.org>
Wed, 11 Oct 2023 13:20:18 +0000 (13:20 +0000)
committertb <tb@openbsd.org>
Wed, 11 Oct 2023 13:20:18 +0000 (13:20 +0000)
The current implementation is a complete mess. There are three cases:
1) ptype == V_ASN1_UNDEF: parameter must be freed and set to NULL.
2) ptype == 0: existing non-NULL parameters are left untouched, NULL
   parameters are replaced with ASN1_TYPE_new()'s wacky defaults.
3) otherwise allocate new parameters if needed and set them to ptype/pval.
In all three cases free the algorithm and set it to aobj.

The challenge now is to implement this using nine if statements and one
else clause... We can do better. This preserves existing behavior. There
would be cleaner implementations possible, but they would change behavior.

There are many callers in the ecosystem that do not error check
X509_ALGOR_set0() since OpenSSL failed to do so. So this was carefully
rewritten to leave alg in a consisten state so that unchecking callers
don't encounter corrupted algs.

ok jsing

lib/libcrypto/asn1/x_algor.c

index 08742c5..74d1235 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: x_algor.c,v 1.29 2023/10/11 13:12:46 tb Exp $ */
+/* $OpenBSD: x_algor.c,v 1.30 2023/10/11 13:20:18 tb Exp $ */
 /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL
  * project 2000.
  */
@@ -150,28 +150,24 @@ X509_ALGOR_dup(X509_ALGOR *x)
 int
 X509_ALGOR_set0(X509_ALGOR *alg, ASN1_OBJECT *aobj, int ptype, void *pval)
 {
-       if (!alg)
+       if (alg == NULL)
                return 0;
-       if (ptype != V_ASN1_UNDEF) {
+
+       if (ptype == V_ASN1_UNDEF) {
+               ASN1_TYPE_free(alg->parameter);
+               alg->parameter = NULL;
+       } else {
                if (alg->parameter == NULL)
                        alg->parameter = ASN1_TYPE_new();
                if (alg->parameter == NULL)
                        return 0;
+               if (ptype != 0)
+                       ASN1_TYPE_set(alg->parameter, ptype, pval);
        }
-       if (alg) {
-               if (alg->algorithm)
-                       ASN1_OBJECT_free(alg->algorithm);
-               alg->algorithm = aobj;
-       }
-       if (ptype == 0)
-               return 1;
-       if (ptype == V_ASN1_UNDEF) {
-               if (alg->parameter) {
-                       ASN1_TYPE_free(alg->parameter);
-                       alg->parameter = NULL;
-               }
-       } else
-               ASN1_TYPE_set(alg->parameter, ptype, pval);
+
+       ASN1_OBJECT_free(alg->algorithm);
+       alg->algorithm = aobj;
+
        return 1;
 }