Skip to contentSkip to conversation

pem: remove endp increment past eol

PATCH In review James Prestwood 2 participants 5 messages Oct 4, 2019

lore
Share & tools
lore
#0 lore raw

PATCH v8 1/4 pem: remove endp increment past eol

Doing this could result in endp pointing past the bounds of the PEM if the PEM does not have a final newline. In terms of calculating the PEM length based off endp, we do want to take the byte into account, but potentially returning an invalid pointer on 'success' is not a great way to do this. It is now left up to the caller to bounds check and increment endp themselves for calculating the length or finding the next PEM.

The existing PEM parsers will also handle this newline if endp is used directly (without incrementing) when parsing the next PEM in a list, so removing the + 1 has no effect on other code in pem.c.

---
 ell/pem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
ell/pem.c +1 -1
index d8f57ed..3fa91cd 100644
@@ -165,7 +165,7 @@ const char *pem_next(const void *buf, size_t buf_len, char **type_label,
 				*base64_len = buf_ptr - base64_data;
 
 			if (endp)
-				*endp = eol + 1;
+				*endp = eol;
 
 			return base64_data;
 		}
Signature · 1 line
2.17.1
#1 lore raw

PATCH v8 2/4 settings: introduce extended groups concept

This patch introduces a new extended format for settings files. This new format allows raw data to be provided under a given group rather than key/value pairs. A group using an extended format will contain both the extended type and group name in the format, for example:

[@pem(a)certificate]

Where 'pem' is the type and 'certificate' is the name. The type of format requires an extension specific parser implementation in settings as the parser must know when to start/stop parsing data. This allows for any textual characters to be present after the group is declared, including special characters like [] (its completely up to the parser to decide whats valid and what is not).

Three new l_settings APIs were added:

l_settings_has_embedded_group
l_settings_get_embedded_groups
l_settings_get_embedded_value

In this patch, the 'pem' extension type is also introduced, and for now is the only extended type. The pem extended type expects a base64 encoded PEM (or list of PEMs) after the group. The pem parser will stop after the last PEM is found. Examples can be found in unit/test-settings.c

---
 ell/ell.sym    |   3 +
 ell/settings.c | 269 ++++++++++++++++++++++++++++++++++++++++++++++++-
 ell/settings.h |   7 ++
 3 files changed, 276 insertions(+), 3 deletions(-)

-v8:
* Updated to work with the pem_next fixup. The endp pointer will now point to the last byte of the PEM, not the byte after.

ell/ell.sym +3 -0
index e02f0fe..32d94f5 100644
@@ -425,6 +425,9 @@ global:
 	l_settings_set_float;
 	l_settings_remove_group;
 	l_settings_remove_key;
+	l_settings_has_embedded_group;
+	l_settings_get_embedded_groups;
+	l_settings_get_embedded_value;
 	/* signal */
 	l_signal_create;
 	l_signal_remove;
ell/settings.c +266 -3

335 lines not on this page · raw message

ell/settings.h +7 -0
index 0da9f55..e4203af 100644
@@ -123,6 +123,13 @@ bool l_settings_remove_key(struct l_settings *settings, const char *group_name,
 				const char *key);
 bool l_settings_remove_group(struct l_settings *settings,
 				const char *group_name);
+
+char **l_settings_get_embedded_groups(struct l_settings *settings);
+bool l_settings_has_embedded_group(struct l_settings *settings,
+					const char *group);
+const char *l_settings_get_embedded_value(struct l_settings *settings,
+						const char *group_name,
+						const char **out_type);
 #ifdef __cplusplus
 }
 #endif
Signature · 1 line
2.17.1
#2 lore raw

PATCH v8 3/4 unit/test-settings: add pem extended type tests

In reply to James Prestwood
---
 unit/settings.test   |  28 +++++
 unit/test-settings.c | 243 ++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 269 insertions(+), 2 deletions(-)
unit/settings.test +28 -0
index 3884f7d..5f2d4f8 100644
@@ -20,3 +20,31 @@ StringList=Foo,Bar,Baz
 StringListEmpty=
 StringListOne=FooBarBaz
 StringWithSpaces=Bar B Q
+
+[@pem(a)certificate]
+-----BEGIN CERTIFICATE-----
+MIIEajCCA1KgAwIBAgIUKIOXQXEyHZsOFw/+ihDBNGTQnWUwDQYJKoZIhvcNAQEL
+BQAweDE1MDMGA1UECgwsSW50ZXJuYXRpb25hbCBVbmlvbiBvZiBFeGFtcGxlIE9y
+Z2FuaXphdGlvbnMxHzAdBgNVBAMMFkNlcnRpZmljYXRlIGlzc3VlciBndXkxHjAc
+BgkqhkiG9w0BCQEWD2NhQG1haWwuZXhhbXBsZTAeFw0xOTA5MTYxNzEyNThaFw00
+NzAyMDExNzEyNThaMHgxNTAzBgNVBAoMLEludGVybmF0aW9uYWwgVW5pb24gb2Yg
+RXhhbXBsZSBPcmdhbml6YXRpb25zMR8wHQYDVQQDDBZDZXJ0aWZpY2F0ZSBpc3N1
+ZXIgZ3V5MR4wHAYJKoZIhvcNAQkBFg9jYUBtYWlsLmV4YW1wbGUwggEiMA0GCSqG
+SIb3DQEBAQUAA4IBDwAwggEKAoIBAQCjovj3aq26sAQ0k5vD/BVp40p0lhz1+Wet
+1EcdQa1arVIca9nhfvoHfJAmK+zzqQLbvI0/e2if4X6OKf41g7w7VaYS9qv5jZZ0
+v/7aL6PUa2F7C9HG/vuIII/dRvP2uQ43PLxeTeZyj7bBUB9xCFCpzB+7AZuUuH0H
+ABaC9CAGZWImBY5NUXST7E/BsvqU80KJglDovcabthvwoekji9DC/wwISLE1e9cO
+A9IB0Co0mA1ME6wzrawmuTzxUw9BsmEhbKhFGBRwIrrq0r4GvDmeMFiZjXv+I0vq
+wSCyRtgoeBmyemqIEgiN4Z23V7ps3dbYF/tw96Zj7rd5gtjY9VSdAgMBAAGjgesw
+gegwDwYDVR0TBAgwBgEB/wIBADAdBgNVHQ4EFgQUnvllvvxy17VqqR0FfgS4NwzX
+jScwgbUGA1UdIwSBrTCBqoAUnvllvvxy17VqqR0FfgS4NwzXjSehfKR6MHgxNTAz
+BgNVBAoMLEludGVybmF0aW9uYWwgVW5pb24gb2YgRXhhbXBsZSBPcmdhbml6YXRp
+b25zMR8wHQYDVQQDDBZDZXJ0aWZpY2F0ZSBpc3N1ZXIgZ3V5MR4wHAYJKoZIhvcN
+AQkBFg9jYUBtYWlsLmV4YW1wbGWCFCiDl0FxMh2bDhcP/ooQwTRk0J1lMA0GCSqG
+SIb3DQEBCwUAA4IBAQBROAyWfQyKXQ007U6ctgihHbg/lsfEEfeNPG+QRVt8/e53
+4fH6scuY9bW7CZQSdiBo178ITHrIOo2CuFMa0ysnW3V1M9/s0dUYjBHYdpTEEQ+d
+tgm1uRLiTsYeBtueRItEmZU6JjgmvAH8i1UqI0e5iYlfnovPmftpqIwRH7k7A9kS
+SehC9QkkrnIttDEoeYTGhLOJu1Fx2cwAodce6VNgz/k1zIXY5Tprg440zrCwc+th
+MpX48F31ggg8Wd5N6Xg1nricGwL8K90ts6xvwF1WwKsg6BeYdyC0eYBqQ41MA/7P
+DK3OGM6cC5tbQGWaIT0Q407GJBGpaijDicA2YqlK
+-----END CERTIFICATE-----
unit/test-settings.c +241 -2

279 lines not on this page · raw message

Signature · 1 line
2.17.1
#3 lore raw

PATCH v8 4/4 unit: fix memory leak in test-pem

In reply to James Prestwood
---
 unit/test-pem.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
unit/test-pem.c +3 -1
index 6a927fc..f1c202f 100644
@@ -226,7 +226,7 @@ static void test_pem(const void *data)
 {
 	const struct pem_test *test = data;
 	uint8_t *decoded;
-	char *label;
+	char *label = NULL;
 	size_t decoded_size;
 
 	decoded = l_pem_load_buffer((const uint8_t *) test->input,
@@ -234,6 +234,8 @@ static void test_pem(const void *data)
 					&label, &decoded_size);
 
 	if (!test->valid) {
+		if (label)
+			l_free(label);
 		assert(!decoded);
 		return;
 	}
Signature · 1 line
2.17.1
#4 lore raw

Re: [PATCH v8 4/4] unit: fix memory leak in test-pem

Hi James,

unit/test-pem.c static void test_pem(const void *data) · line 238

Quoted 3 lines · James Prestwood
        if (!test->valid) {
unit/test-pem.c +2 -0
+               if (label)
+                       l_free(label);

Checking for NULL before calling l_free() is unnecessary and redundant. l_free() and the underlying free() are no-ops when the pointer argument is NULL.

-Ossama