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>

5 files changed, 95 insertions, 27 deletions

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 8f484c068e..71ddd0bda3 100644
--- 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 91c44faf5f..b3208dc875 100644
--- 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 1b954cfb96..0a6b41ca50 100644
--- 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 850a6d9bdf..f777e2f9a0 100644
--- 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 48e8bf19ae..cf4f00b764 100644
--- 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,