gnutls_priority_init: fix err_pos on invalid strings

When the provided string would be resolved (e.g., due to a @ priority
being used), to a different string, then do not attempt to
detect the right location of the error. It will not be useful to the caller.

This addresses the issue of test suite failure when --with-system-priority-file
and --with-default-priority-string are provided. It also enhances the test suite
with these options being active.

Resolves #517

Signed-off-by: Nikos Mavrogiannopoulos <nmav@redhat.com>

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 8f484c068e2dc84089004d4290f9b8f2468c60e1..71ddd0bda33a9e2aa2f973dde027362a96ba8940 100644 (file)
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -212,9 +212,9 @@ asan.Fedora.x86_64:
   - LSAN_OPTIONS="suppressions=$(pwd)/fuzz/lsan.supp" make -C fuzz check -j$(nproc) GNUTLS_CPUID_OVERRIDE=0x4
   - LSAN_OPTIONS="suppressions=$(pwd)/fuzz/lsan.supp" make -C fuzz check -j$(nproc) GNUTLS_CPUID_OVERRIDE=0x8
   - CFLAGS="-fsanitize=address -g -O2" CXXFLAGS=$CFLAGS LDFLAGS="-static-libasan"
-    dash ./configure --cache-file cache/config.cache --disable-doc --with-default-trust-store-pkcs11="pkcs11:" --disable-guile
+    dash ./configure --cache-file cache/config.cache --disable-doc --with-system-priority-file=/etc/crypto-policies/back-ends/gnutls.config --with-default-priority-string=@SYSTEM --with-default-trust-store-pkcs11="pkcs11:" --disable-guile
   - make -j$(nproc)
-  - make -C tests check -j$(nproc) TESTS="trust-store p11-kit-load.sh" SUBDIRS=.
+  - make -C tests check -j$(nproc) TESTS="trust-store p11-kit-load.sh priority-init2 set-default-prio" SUBDIRS=.
   tags:
   - shared
   except:

diff --git a/lib/libgnutls.map b/lib/libgnutls.map
index 91c44faf5fe4468a8d2141704ab43661900976c4..b3208dc875f84bc941f933f45e5ab2f0119dd54b 100644 (file)
--- a/lib/libgnutls.map
+++ b/lib/libgnutls.map
@@ -1294,6 +1294,7 @@ GNUTLS_PRIVATE_3_4 {
        _gnutls_mpi_release;
 
        # Internal symbols needed by tests/:
+       _gnutls_default_priority_string;
        _gnutls_supplemental_deinit;
        _gnutls_record_overhead;
        _gnutls_cipher_to_entry;

diff --git a/lib/priority.c b/lib/priority.c
index 1b954cfb9648aaf6204da03a14af778d8ba49287..0a6b41ca50d41c2e69e138312ff7ca98d24b1546 100644 (file)
--- a/lib/priority.c
+++ b/lib/priority.c
@@ -40,6 +40,7 @@
 
 /* This function is used by the test suite */
 char *_gnutls_resolve_priorities(const char* priorities);
+const char *_gnutls_default_priority_string = DEFAULT_PRIORITY_STRING;
 
 static void prio_remove(priority_st * priority_list, unsigned int algo);
 static void prio_add(priority_st * priority_list, unsigned int algo);
@@ -1510,7 +1511,7 @@ gnutls_priority_init2(gnutls_priority_t * priority_cache,
 
                _gnutls_buffer_init(&buf);
 
-               ret = _gnutls_buffer_append_str(&buf, DEFAULT_PRIORITY_STRING);
+               ret = _gnutls_buffer_append_str(&buf, _gnutls_default_priority_string);
                if (ret < 0) {
                        _gnutls_buffer_clear(&buf);
                        return gnutls_assert_val(ret);
@@ -1531,7 +1532,7 @@ gnutls_priority_init2(gnutls_priority_t * priority_cache,
                ret = gnutls_priority_init(priority_cache, (const char*)buf.data, &ep);
                if (ret < 0 && ep != (const char*)buf.data && ep != NULL) {
                        ptrdiff_t diff = (ptrdiff_t)ep-(ptrdiff_t)buf.data;
-                       unsigned hlen = strlen(DEFAULT_PRIORITY_STRING)+1;
+                       unsigned hlen = strlen(_gnutls_default_priority_string)+1;
 
                        if (err_pos && diff > hlen) {
                                *err_pos = priorities + diff - hlen;
@@ -1578,6 +1579,7 @@ gnutls_priority_init(gnutls_priority_t * priority_cache,
        bulk_rmadd_func *bulk_fn;
        bulk_rmadd_func *bulk_given_fn;
        const cipher_entry_st *centry;
+       unsigned resolved_match = 1;
 
        if (err_pos)
                *err_pos = priorities;
@@ -1596,8 +1598,10 @@ gnutls_priority_init(gnutls_priority_t * priority_cache,
        (*priority_cache)->min_record_version = 1;
        gnutls_atomic_init(&(*priority_cache)->usage_cnt);
 
-       if (priorities == NULL)
-               priorities = DEFAULT_PRIORITY_STRING;
+       if (priorities == NULL) {
+               priorities = _gnutls_default_priority_string;
+               resolved_match = 0;
+       }
 
        darg = _gnutls_resolve_priorities(priorities);
        if (darg == NULL) {
@@ -1605,6 +1609,9 @@ gnutls_priority_init(gnutls_priority_t * priority_cache,
                goto error;
        }
 
+       if (strcmp(darg, priorities) != 0)
+               resolved_match = 0;
+
        break_list(darg, broken_list, &broken_list_size);
        /* This is our default set of protocol version, certificate types.
         */
@@ -1805,7 +1812,7 @@ gnutls_priority_init(gnutls_priority_t * priority_cache,
        return 0;
 
  error:
-       if (err_pos != NULL && i < broken_list_size) {
+       if (err_pos != NULL && i < broken_list_size && resolved_match) {
                *err_pos = priorities;
                for (j = 0; j < i; j++) {
                        (*err_pos) += strlen(broken_list[j]) + 1;

diff --git a/tests/priority-init2.c b/tests/priority-init2.c
index 850a6d9bdf6874e729ce55cfa4f3ff9194e1b2c4..f777e2f9a091a5a56919017b39488117c6ac3563 100644 (file)
--- a/tests/priority-init2.c
+++ b/tests/priority-init2.c
@@ -35,6 +35,7 @@
 #include "cert-common.h"
 
 const char *side;
+extern const char *_gnutls_default_priority_string;
 
 static void tls_log_func(int level, const char *str)
 {
@@ -44,6 +45,7 @@ static void tls_log_func(int level, const char *str)
 struct test_st {
        const char *name;
        const char *add_prio;
+       const char *def_prio;
        int exp_err;
        int exp_etm;
        unsigned err_pos;
@@ -69,6 +71,11 @@ static void start(struct test_st *test)
        else
                success("running %s\n", test->name);
 
+       if (test && test->def_prio)
+               _gnutls_default_priority_string = test->def_prio;
+       else
+               _gnutls_default_priority_string = "NORMAL";
+
        /* General init. */
        global_init();
        gnutls_global_set_log_function(tls_log_func);
@@ -83,20 +90,30 @@ static void start(struct test_st *test)
        assert(gnutls_init(&server, GNUTLS_SERVER) >= 0);
        gnutls_credentials_set(server, GNUTLS_CRD_CERTIFICATE,
                                serverx509cred);
-       if (test == NULL)
+       if (test == NULL) {
                ret = gnutls_priority_init(&cache, NULL, NULL);
-       else
+               if (ret < 0)
+                       fail("error: %s\n", gnutls_strerror(ret));
+       } else {
                ret = gnutls_priority_init2(&cache, test->add_prio, &ep, GNUTLS_PRIORITY_INIT_DEF_APPEND);
-       if (ret < 0) {
-               if (test->exp_err == ret) {
-                       if (ep-test->add_prio != test->err_pos) {
-                               fprintf(stderr, "diff: %d\n", (int)(ep-test->add_prio));
-                               fail("error expected error on different position[%d]: %s\n",
-                                       test->err_pos, test->add_prio);
+               if (ret < 0) {
+                       if (test->exp_err == ret) {
+                               if (strchr(_gnutls_default_priority_string, '@') != 0) {
+                                       if (ep != test->add_prio) {
+                                               fail("error expected error on start of string[%d]: %s\n",
+                                                       test->err_pos, test->add_prio);
+                                       }
+                               } else {
+                                       if (ep-test->add_prio != test->err_pos) {
+                                               fprintf(stderr, "diff: %d\n", (int)(ep-test->add_prio));
+                                               fail("error expected error on different position[%d]: %s\n",
+                                                       test->err_pos, test->add_prio);
+                                       }
+                               }
+                               goto cleanup;
                        }
-                       goto cleanup;
+                       fail("error: %s\n", gnutls_strerror(ret));
                }
-               fail("error: %s\n", gnutls_strerror(ret));
        }
        gnutls_priority_set(server, cache);
 
@@ -229,29 +246,41 @@ static void start(struct test_st *test)
 struct test_st tests[] = {
        {
                .name = "additional flag",
+               .def_prio = "NORMAL",
                .add_prio = "%FORCE_ETM",
                .exp_err = 0
        },
        {
                .name = "additional flag typo1",
+               .def_prio = "NORMAL",
                .add_prio = ":%FORCE_ETM",
                .exp_err = GNUTLS_E_INVALID_REQUEST,
                .err_pos = 0
        },
        {
                .name = "additional flag typo2",
+               .def_prio = "NORMAL",
                .add_prio = "%FORCE_ETM::%NO_TICKETS",
                .exp_err = GNUTLS_E_INVALID_REQUEST,
                .err_pos = 11
        },
        {
                .name = "additional flag typo3",
+               .def_prio = "NORMAL",
                .add_prio = "%FORCE_ETM:%%NO_TICKETS",
                .exp_err = GNUTLS_E_INVALID_REQUEST,
                .err_pos = 11
        },
+       {
+               .name = "additional flag typo3 (with resolved def prio)",
+               .def_prio = "@HELLO",
+               .add_prio = "%FORCE_ETM:%%NO_TICKETS",
+               .exp_err = GNUTLS_E_INVALID_REQUEST,
+               .err_pos = 0
+       },
        {
                .name = "additional flag for version (functional)",
+               .def_prio = "NORMAL",
                .add_prio = "-VERS-ALL:+VERS-TLS1.1",
                .exp_etm = 1,
                .exp_err = 0,

diff --git a/tests/set-default-prio.c b/tests/set-default-prio.c
index 48e8bf19aec890355dc4762b8184c51931fae6f9..cf4f00b764341a8f0feb62f9abfbdcd40d132dcd 100644 (file)
--- a/tests/set-default-prio.c
+++ b/tests/set-default-prio.c
@@ -35,6 +35,7 @@
 #include "cert-common.h"
 
 const char *side;
+extern const char *_gnutls_default_priority_string;
 
 static void tls_log_func(int level, const char *str)
 {
@@ -44,6 +45,7 @@ static void tls_log_func(int level, const char *str)
 struct test_st {
        const char *name;
        const char *add_prio;
+       const char *def_prio;
        int exp_err;
        unsigned err_pos;
        unsigned exp_vers;
@@ -68,6 +70,11 @@ static void start(struct test_st *test)
        else
                success("running %s\n", test->name);
 
+       if (test && test->def_prio)
+               _gnutls_default_priority_string = test->def_prio;
+       else
+               _gnutls_default_priority_string = "NORMAL";
+
        /* General init. */
        global_init();
        gnutls_global_set_log_function(tls_log_func);
@@ -82,20 +89,32 @@ static void start(struct test_st *test)
        assert(gnutls_init(&server, GNUTLS_SERVER) >= 0);
        gnutls_credentials_set(server, GNUTLS_CRD_CERTIFICATE,
                                serverx509cred);
-       if (test == NULL)
+       if (test == NULL) {
                ret = gnutls_set_default_priority(server);
-       else
+               if (ret < 0)
+               fail("error: %s\n", gnutls_strerror(ret));
+       } else {
                ret = gnutls_set_default_priority_append(server, test->add_prio, &ep, 0);
-       if (ret < 0) {
-               if (test->exp_err == ret) {
-                       if (ep-test->add_prio != test->err_pos) {
-                               fprintf(stderr, "diff: %d\n", (int)(ep-test->add_prio));
-                               fail("error expected error on different position[%d]: %s\n",
-                                       test->err_pos, test->add_prio);
+               if (ret < 0) {
+                       if (test->exp_err == ret) {
+                       /* the &ep value is only accurate when the default priorities are not overriden;
+                        * otherwise it should be a pointer to the start of the string */
+                               if (strchr(_gnutls_default_priority_string, '@') != 0) {
+                                       if (ep != test->add_prio) {
+                                               fail("error expected error on start of string[%d]: %s\n",
+                                                       test->err_pos, test->add_prio);
+                                       }
+                               } else {
+                                       if (ep-test->add_prio != test->err_pos) {
+                                               fprintf(stderr, "diff: %d\n", (int)(ep-test->add_prio));
+                                               fail("error expected error on different position[%d]: %s\n",
+                                                    test->err_pos, test->add_prio);
+                                       }
+                               }
+                               goto cleanup;
                        }
-                       goto cleanup;
+                       fail("error: %s\n", gnutls_strerror(ret));
                }
-               fail("error: %s\n", gnutls_strerror(ret));
        }
 
        gnutls_transport_set_push_function(server, server_push);
@@ -226,29 +245,41 @@ static void start(struct test_st *test)
 struct test_st tests[] = {
        {
                .name = "additional flag",
+               .def_prio = "NORMAL",
                .add_prio = "%FORCE_ETM",
                .exp_err = 0
        },
        {
                .name = "additional flag typo1",
+               .def_prio = "NORMAL",
                .add_prio = ":%FORCE_ETM",
                .exp_err = GNUTLS_E_INVALID_REQUEST,
                .err_pos = 0
        },
        {
                .name = "additional flag typo2",
+               .def_prio = "NORMAL",
                .add_prio = "%FORCE_ETM::%NO_TICKETS",
                .exp_err = GNUTLS_E_INVALID_REQUEST,
                .err_pos = 11
        },
        {
                .name = "additional flag typo3",
+               .def_prio = "NORMAL",
                .add_prio = "%FORCE_ETM:%%NO_TICKETS",
                .exp_err = GNUTLS_E_INVALID_REQUEST,
                .err_pos = 11
        },
+       {
+               .name = "additional flag typo3 (with resolved def prio)",
+               .def_prio = "@HELLO",
+               .add_prio = "%FORCE_ETM:%%NO_TICKETS",
+               .exp_err = GNUTLS_E_INVALID_REQUEST,
+               .err_pos = 0
+       },
        {
                .name = "additional flag for version (functional)",
+               .def_prio = "NORMAL",
                .add_prio = "-VERS-ALL:+VERS-TLS1.1",
                .exp_err = 0,
                .exp_etm = 1,