From 699a3ea2ebc86f9e9dc9d59e1d9db488ac4ff352 Mon Sep 17 00:00:00 2001
From: Valentin Bartenev <vbart@nginx.com>
Date: Wed, 24 Mar 2021 16:55:47 +0300
Subject: Certificates: fixed in name attributes processing.

The idea is to put SAN after CN, but the previous version of the code
incorrectly assumed that CN was always present, which caused writes
outside the allocated object if there were no standard name attributes.
---
 docs/changes.xml |  7 +++++++
 src/nxt_cert.c   | 41 ++++++++++++++++-------------------------
 2 files changed, 23 insertions(+), 25 deletions(-)

diff --git a/docs/changes.xml b/docs/changes.xml
index ad963c40..3c25481a 100644
--- a/docs/changes.xml
+++ b/docs/changes.xml
@@ -44,6 +44,13 @@ certificate with a non-DNS SAN entry.
 </para>
 </change>
 
+<change type="bugfix">
+<para>
+the controller process could crash on manipulations with a certificate
+containing a SAN and no standart name attributes in subject or issuer.
+</para>
+</change>
+
 <change type="bugfix">
 <para>
 the Ruby module didn't respect user locale for defaults in the Encoding class.
diff --git a/src/nxt_cert.c b/src/nxt_cert.c
index f3f4bace..3cdb69c1 100644
--- a/src/nxt_cert.c
+++ b/src/nxt_cert.c
@@ -690,12 +690,23 @@ nxt_cert_name_details(nxt_mp_t *mp, X509 *x509, nxt_bool_t issuer)
                                  NULL, NULL);
 
     if (alt_names != NULL) {
+        names = nxt_cert_alt_names_details(mp, alt_names);
+
+        sk_GENERAL_NAME_pop_free(alt_names, GENERAL_NAME_free);
+
+        if (nxt_slow_path(names == NULL)) {
+            return NULL;
+        }
+
         count++;
+
+    } else {
+        names = NULL;
     }
 
     object = nxt_conf_create_object(mp, count);
     if (nxt_slow_path(object == NULL)) {
-        goto fail;
+        return NULL;
     }
 
     for (n = 0, i = 0; n != nxt_nitems(nids) && i != count; n++) {
@@ -703,12 +714,12 @@ nxt_cert_name_details(nxt_mp_t *mp, X509 *x509, nxt_bool_t issuer)
         len = X509_NAME_get_text_by_NID(x509_name, nids[n].nid,
                                         (char *) buf, sizeof(buf));
 
-        if (len < 0) {
-            continue;
+        if (n == 1 && names != NULL) {
+            nxt_conf_set_member(object, &alt_names_str, names, i++);
         }
 
-        if (i == 1 && alt_names != NULL) {
-            i++;
+        if (len < 0) {
+            continue;
         }
 
         str.length = len;
@@ -717,31 +728,11 @@ nxt_cert_name_details(nxt_mp_t *mp, X509 *x509, nxt_bool_t issuer)
         ret = nxt_conf_set_member_string_dup(object, mp, &nids[n].name,
                                              &str, i++);
         if (nxt_slow_path(ret != NXT_OK)) {
-            goto fail;
-        }
-    }
-
-    if (alt_names != NULL) {
-        names = nxt_cert_alt_names_details(mp, alt_names);
-
-        sk_GENERAL_NAME_pop_free(alt_names, GENERAL_NAME_free);
-
-        if (nxt_slow_path(names == NULL)) {
             return NULL;
         }
-
-        nxt_conf_set_member(object, &alt_names_str, names, 1);
     }
 
     return object;
-
-fail:
-
-    if (alt_names != NULL) {
-        sk_GENERAL_NAME_pop_free(alt_names, GENERAL_NAME_free);
-    }
-
-    return NULL;
 }
 
 
-- 
cgit