X509v3_get_ext_by_NID.pod: Add warning on counter-intuitive behavior of X509v3_delete...

Also simplify two uses of these functions.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/13711)

diff --git a/crypto/ct/ct_sct_ctx.c b/crypto/ct/ct_sct_ctx.c
index a84c476caf84582dba6d8e23010e72ae80e9f8ee..353b5a7f0edc0ba904b9914d9cc82fb02c1d357f 100644 (file)
--- a/crypto/ct/ct_sct_ctx.c
+++ b/crypto/ct/ct_sct_ctx.c
@@ -168,15 +168,12 @@ int SCT_CTX_set1_cert(SCT_CTX *sctx, X509 *cert, X509 *presigner)
      * SCT.
      */
     if (idx >= 0) {
-        X509_EXTENSION *ext;
-
         /* Take a copy of certificate so we don't modify passed version */
         pretmp = X509_dup(cert);
         if (pretmp == NULL)
             goto err;
 
-        ext = X509_delete_ext(pretmp, idx);
-        X509_EXTENSION_free(ext);
+        X509_EXTENSION_free(X509_delete_ext(pretmp, idx));
 
         if (!ct_x509_cert_fixup(pretmp, presigner))
             goto err;

diff --git a/crypto/x509/v3_conf.c b/crypto/x509/v3_conf.c
index 740108fefd94e04f2a2791e53c1449191672d712..9eda71348c501074fed1eba59420bce36a08bf64 100644 (file)
--- a/crypto/x509/v3_conf.c
+++ b/crypto/x509/v3_conf.c
@@ -295,12 +295,8 @@ static void delete_ext(STACK_OF(X509_EXTENSION) *sk, X509_EXTENSION *dext)
     ASN1_OBJECT *obj;
 
     obj = X509_EXTENSION_get_object(dext);
-    while ((idx = X509v3_get_ext_by_OBJ(sk, obj, -1)) >= 0) {
-        X509_EXTENSION *tmpext = X509v3_get_ext(sk, idx);
-
-        X509v3_delete_ext(sk, idx);
-        X509_EXTENSION_free(tmpext);
-    }
+    while ((idx = X509v3_get_ext_by_OBJ(sk, obj, -1)) >= 0)
+        X509_EXTENSION_free(X509v3_delete_ext(sk, idx));
 }
 
 /*

diff --git a/doc/man3/X509v3_get_ext_by_NID.pod b/doc/man3/X509v3_get_ext_by_NID.pod
index f77474ca80434f5de57938fd2590a22d90ada315..79c68e1478b0ad34136d93ee5c8f45ee815265d6 100644 (file)
--- a/doc/man3/X509v3_get_ext_by_NID.pod
+++ b/doc/man3/X509v3_get_ext_by_NID.pod
@@ -74,9 +74,9 @@ looks for an extension of criticality B<crit>. A zero value for B<crit>
 looks for a non-critical extension a nonzero value looks for a critical
 extension.
 
-X509v3_delete_ext() deletes the extension with index B<loc> from B<x>. The
-deleted extension is returned and must be freed by the caller. If B<loc>
-is in invalid index value B<NULL> is returned.
+X509v3_delete_ext() deletes the extension with index B<loc> from B<x>.
+The deleted extension is returned and must be freed by the caller.
+If B<loc> is in invalid index value B<NULL> is returned.
 
 X509v3_add_ext() adds extension B<ex> to stack B<*x> at position B<loc>. If
 B<loc> is B<-1> the new extension is added to the end. If B<*x> is B<NULL>
@@ -111,6 +111,11 @@ error. These search functions start from the extension B<after> the B<lastpos>
 parameter so it should initially be set to B<-1>, if it is set to zero the
 initial extension will not be checked.
 
+=head1 BUGS
+
+X509v3_delete_ext() and its variants are a bit counter-intuitive
+because these functions do not free the extension they delete.
+
 =head1 RETURN VALUES
 
 X509v3_get_ext_count() returns the extension count.