12:57 p.m.
Give the user time to set up the tls instance's optional parameters --
those that can not be passed as parameters of l_tls_new, before sending
the Client Hello message in client mode. We now send the message from
l_tls_start which has to be called after the optional setup is done,
e.g. set_cacert, set_auth_data, set_debug are called. This way Client
Hello can avoid proposing cipher suites that would not have worked if
they were negotiated, due to bad certificate type. It also allows us to
output debug messages in the Client Hello sending code, and will allow
to add methods to define security profiles.
---
ell/ell.sym | 1 +
ell/tls-private.h | 1 +
ell/tls.c | 48 ++++++++++++++++++++++++++++-------------------
ell/tls.h | 3 +++
4 files changed, 34 insertions(+), 19 deletions(-)
diff --git a/ell/ell.sym b/ell/ell.sym
index 841bc49..7d7a5e4 100644
--- a/ell/ell.sym
+++ b/ell/ell.sym
@@ -413,6 +413,7 @@ global:
l_tls_new;
l_tls_free;
l_tls_write;
+ l_tls_start;
l_tls_close;
l_tls_set_cacert;
l_tls_set_auth_data;
diff --git a/ell/tls-private.h b/ell/tls-private.h
index f2b6b14..b6d1461 100644
--- a/ell/tls-private.h
+++ b/ell/tls-private.h
@@ -98,6 +98,7 @@ struct tls_compression_method {
};
enum tls_handshake_state {
+ TLS_HANDSHAKE_WAIT_START,
TLS_HANDSHAKE_WAIT_HELLO,
TLS_HANDSHAKE_WAIT_CERTIFICATE,
TLS_HANDSHAKE_WAIT_KEY_EXCHANGE,
diff --git a/ell/tls.c b/ell/tls.c
index f27f35c..d05ae8d 100644
--- a/ell/tls.c
+++ b/ell/tls.c
@@ -195,7 +195,7 @@ static void tls_reset_handshake(struct l_tls *tls)
for (hash = 0; hash < __HANDSHAKE_HASH_COUNT; hash++)
tls_drop_handshake_hash(tls, hash);
- TLS_SET_STATE(TLS_HANDSHAKE_WAIT_HELLO);
+ TLS_SET_STATE(TLS_HANDSHAKE_WAIT_START);
tls->cert_requested = 0;
tls->cert_sent = 0;
}
@@ -826,13 +826,6 @@ static bool tls_send_client_hello(struct l_tls *tls)
*ptr++ = 0; /* No SessionID */
- /*
- * FIXME: We do need to filter the cipher suites by key exchange
- * mechanism compatibility with the certificate but we don't normally
- * have the certificate at this point because we're called from
- * l_tls_new. We also don't know the TLS version that's going to
- * be negotiated yet.
- */
len_ptr = ptr;
ptr += 2;
@@ -2463,17 +2456,11 @@ LIB_EXPORT struct l_tls *l_tls_new(bool server,
tls->signature_hash = HANDSHAKE_HASH_SHA256;
- /* If we're the client, start the handshake right away */
- if (!tls->server) {
- if (!tls_init_handshake_hash(tls) ||
- !tls_send_client_hello(tls)) {
- l_free(tls);
-
- return NULL;
- }
- }
-
- TLS_SET_STATE(TLS_HANDSHAKE_WAIT_HELLO);
+ /* If we're the server wait for the Client Hello already */
+ if (tls->server)
+ TLS_SET_STATE(TLS_HANDSHAKE_WAIT_HELLO);
+ else
+ TLS_SET_STATE(TLS_HANDSHAKE_WAIT_START);
return tls;
}
@@ -2652,6 +2639,28 @@ bool tls_handle_message(struct l_tls *tls, const uint8_t *message,
return false;
}
+LIB_EXPORT bool l_tls_start(struct l_tls *tls)
+{
+ /* This is a nop in server mode */
+ if (tls->server)
+ return true;
+
+ if (tls->state != TLS_HANDSHAKE_WAIT_START) {
+ TLS_DEBUG("Call invalid in state %s",
+ tls_handshake_state_to_str(tls->state));
+ return false;
+ }
+
+ if (!tls_init_handshake_hash(tls))
+ return false;
+
+ if (!tls_send_client_hello(tls))
+ return false;
+
+ TLS_SET_STATE(TLS_HANDSHAKE_WAIT_HELLO);
+ return true;
+}
+
LIB_EXPORT void l_tls_close(struct l_tls *tls)
{
TLS_DISCONNECT(TLS_ALERT_CLOSE_NOTIFY, 0, "Closing session");
@@ -2800,6 +2809,7 @@ const char *tls_handshake_state_to_str(enum tls_handshake_state
state)
static char buf[100];
switch (state) {
+ SWITCH_ENUM_TO_STR(TLS_HANDSHAKE_WAIT_START)
SWITCH_ENUM_TO_STR(TLS_HANDSHAKE_WAIT_HELLO)
SWITCH_ENUM_TO_STR(TLS_HANDSHAKE_WAIT_CERTIFICATE)
SWITCH_ENUM_TO_STR(TLS_HANDSHAKE_WAIT_KEY_EXCHANGE)
diff --git a/ell/tls.h b/ell/tls.h
index 505f0e0..fb33404 100644
--- a/ell/tls.h
+++ b/ell/tls.h
@@ -77,6 +77,9 @@ struct l_tls *l_tls_new(bool server, l_tls_write_cb_t app_data_handler,
void l_tls_free(struct l_tls *tls);
+/* Begin sending connection setup messages to the server */
+bool l_tls_start(struct l_tls *tls);
+
/* Properly disconnect a connected session */
void l_tls_close(struct l_tls *tls);
--
2.19.1
12:57 p.m.
New subject: [PATCH 2/9] unit: Call l_tls_start in tls tests
---
unit/test-tls.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/unit/test-tls.c b/unit/test-tls.c
index 84dd950..04fd3ea 100644
--- a/unit/test-tls.c
+++ b/unit/test-tls.c
@@ -469,6 +469,9 @@ static void test_tls_test(const void *data)
assert(l_tls_set_cacert(s[0].tls, test->server_ca_cert_path));
assert(l_tls_set_cacert(s[1].tls, test->client_ca_cert_path));
+ assert(l_tls_start(s[0].tls));
+ assert(l_tls_start(s[1].tls));
+
while (1) {
if (s[0].raw_buf_len) {
l_tls_handle_rx(s[1].tls, s[0].raw_buf,
--
2.19.1
12:57 p.m.
New subject: [PATCH 3/9] tls: Add TLS version number printf macros
---
ell/tls-private.h | 3 +++
ell/tls.c | 17 +++++++++++------
2 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/ell/tls-private.h b/ell/tls-private.h
index b6d1461..8e6c277 100644
--- a/ell/tls-private.h
+++ b/ell/tls-private.h
@@ -258,4 +258,7 @@ int tls_parse_certificate_list(const void *data, size_t len,
tls_disconnect(tls, desc, local_desc); \
} while (0)
+#define TLS_VER_FMT "1.%i"
+#define TLS_VER_ARGS(version) ((version & 0xff) - 1)
+
const char *tls_handshake_state_to_str(enum tls_handshake_state state);
diff --git a/ell/tls.c b/ell/tls.c
index d05ae8d..8099e76 100644
--- a/ell/tls.c
+++ b/ell/tls.c
@@ -535,14 +535,17 @@ static bool tls_cipher_suite_is_compatible(struct l_tls *tls,
if (suite->encryption &&
suite->encryption->cipher_type == TLS_CIPHER_AEAD) {
- if (tls->negotiated_version &&
- tls->negotiated_version < TLS_V12) {
+ uint16_t negotiated = tls->negotiated_version;
+
+ if (negotiated && negotiated < L_TLS_V12) {
if (error) {
*error = error_buf;
snprintf(error_buf, sizeof(error_buf),
"Cipher suite %s uses an AEAD "
- "cipher but TLS < 1.2 was "
- "negotiated", suite->name);
+ "cipher (TLS 1.2+) but "
+ TLS_VER_FMT " was negotiated",
+ suite->name,
+ TLS_VER_ARGS(negotiated));
}
return false;
@@ -1587,7 +1590,8 @@ static void tls_handle_client_hello(struct l_tls *tls,
if (i != HANDSHAKE_HASH_SHA1 && i != HANDSHAKE_HASH_MD5)
tls_drop_handshake_hash(tls, i);
- TLS_DEBUG("Negotiated TLS 1.%i", (tls->negotiated_version & 0xff) -
1);
+ TLS_DEBUG("Negotiated TLS " TLS_VER_FMT,
+ TLS_VER_ARGS(tls->negotiated_version));
/* Select a cipher suite according to client's preference list */
while (cipher_suites_size) {
@@ -1715,7 +1719,8 @@ static void tls_handle_server_hello(struct l_tls *tls,
if (i != HANDSHAKE_HASH_SHA1 && i != HANDSHAKE_HASH_MD5)
tls_drop_handshake_hash(tls, i);
- TLS_DEBUG("Negotiated TLS 1.%i", (tls->negotiated_version & 0xff) -
1);
+ TLS_DEBUG("Negotiated TLS " TLS_VER_FMT,
+ TLS_VER_ARGS(tls->negotiated_version));
/* Set the new cipher suite and compression method structs */
tls->pending.cipher_suite = tls_find_cipher_suite(cipher_suite_id);
--
2.19.1
8:53 a.m.
New subject: [PATCH 3/9] tls: Add TLS version number printf macros
Hi Andrew,
On 12/13/2018 01:57 PM, Andrew Zaborowski wrote:
---
ell/tls-private.h | 3 +++
ell/tls.c | 17 +++++++++++------
2 files changed, 14 insertions(+), 6 deletions(-)
ell/tls.c: In function ‘tls_cipher_suite_is_compatible’:
ell/tls.c:540:34: error: ‘L_TLS_V12’ undeclared (first use in this
function); did you mean ‘TLS_V12’?
if (negotiated && negotiated < L_TLS_V12) {
^~~~~~~~~
TLS_V12
ell/tls.c:540:34: note: each undeclared identifier is reported only once
for each function it appears in
I know we allow breaking compilation if unavoidable, but I think here
this can be easily avoided.
Can this patch be sent out of order prior to L_TLS_V refactoring and
then fixed up there?
Regards,
-Denis
11:48 a.m.
New subject: [PATCH 3/9] tls: Add TLS version number printf macros
Hi Denis,
On Fri, 14 Dec 2018 at 16:53, Denis Kenzior <denkenz(a)gmail.com> wrote:
On 12/13/2018 01:57 PM, Andrew Zaborowski wrote:
> ---
> ell/tls-private.h | 3 +++
> ell/tls.c | 17 +++++++++++------
> 2 files changed, 14 insertions(+), 6 deletions(-)
>
ell/tls.c: In function ‘tls_cipher_suite_is_compatible’:
ell/tls.c:540:34: error: ‘L_TLS_V12’ undeclared (first use in this
function); did you mean ‘TLS_V12’?
if (negotiated && negotiated < L_TLS_V12) {
^~~~~~~~~
TLS_V12
ell/tls.c:540:34: note: each undeclared identifier is reported only once
for each function it appears in
I know we allow breaking compilation if unavoidable, but I think here
this can be easily avoided.
Ah indeed, I try to not break compilation if possible, will fix this patch.
Best regards
12:57 p.m.
New subject: [PATCH 4/9] tls: Implement l_tls_set_version_range
Allow user to set custom min and max TLS version limits, this can be
used to ensure we comply with a specific security profile.
---
ell/ell.sym | 1 +
ell/tls-private.h | 10 ++---
ell/tls-record.c | 14 +++----
ell/tls.c | 99 +++++++++++++++++++++++++++++++++--------------
ell/tls.h | 8 +++-
5 files changed, 87 insertions(+), 45 deletions(-)
diff --git a/ell/ell.sym b/ell/ell.sym
index 7d7a5e4..2ff7d30 100644
--- a/ell/ell.sym
+++ b/ell/ell.sym
@@ -417,6 +417,7 @@ global:
l_tls_close;
l_tls_set_cacert;
l_tls_set_auth_data;
+ l_tls_set_version_range;
l_tls_alert_to_str;
l_tls_set_debug;
/* uintset */
diff --git a/ell/tls-private.h b/ell/tls-private.h
index 8e6c277..e2ec014 100644
--- a/ell/tls-private.h
+++ b/ell/tls-private.h
@@ -20,13 +20,8 @@
*
*/
-/* Only TLS 1.2 supported */
-#define TLS_V12 ((3 << 8) | 3)
-#define TLS_V11 ((3 << 8) | 2)
-#define TLS_V10 ((3 << 8) | 1)
-
-#define TLS_VERSION TLS_V12
-#define TLS_MIN_VERSION TLS_V10
+#define TLS_MAX_VERSION L_TLS_V12
+#define TLS_MIN_VERSION L_TLS_V10
enum tls_cipher_type {
TLS_CIPHER_STREAM,
@@ -145,6 +140,7 @@ struct l_tls {
l_tls_debug_cb_t debug_handler;
l_tls_destroy_cb_t debug_destroy;
void *debug_data;
+ uint16_t min_version, max_version;
struct l_queue *ca_certs;
struct l_certchain *cert;
diff --git a/ell/tls-record.c b/ell/tls-record.c
index bffa413..bdab0c4 100644
--- a/ell/tls-record.c
+++ b/ell/tls-record.c
@@ -135,14 +135,14 @@ static void tls_tx_record_plaintext(struct l_tls *tls,
offset = 0;
- if (tls->negotiated_version >= TLS_V12) {
+ if (tls->negotiated_version >= L_TLS_V12) {
l_getrandom(ciphertext, tls->record_iv_length[1]);
l_cipher_set_iv(tls->cipher[1], ciphertext,
tls->record_iv_length[1]);
offset = tls->record_iv_length[1];
- } else if (tls->negotiated_version >= TLS_V11) {
+ } else if (tls->negotiated_version >= L_TLS_V11) {
l_getrandom(iv, tls->record_iv_length[1]);
l_cipher_encrypt(tls->cipher[1], iv, ciphertext,
@@ -223,7 +223,7 @@ void tls_tx_record(struct l_tls *tls, enum tls_content_type type,
TX_RECORD_TAILROOM];
uint8_t *fragment, *plaintext;
uint16_t fragment_len;
- uint16_t version = tls->negotiated_version ?: TLS_MIN_VERSION;
+ uint16_t version = tls->negotiated_version ?: tls->min_version;
if (type == TLS_CT_ALERT)
tls->record_flush = true;
@@ -393,7 +393,7 @@ static bool tls_handle_ciphertext(struct l_tls *tls)
if ((tls->negotiated_version && tls->negotiated_version != version) ||
(!tls->negotiated_version &&
- tls->record_buf[1] != 0x03 /* Appending E.1 */)) {
+ tls->record_buf[1] != 0x03 /* Appendix E.1 */)) {
TLS_DISCONNECT(TLS_ALERT_PROTOCOL_VERSION, 0,
"Record version mismatch: %02x", version);
return false;
@@ -444,7 +444,7 @@ static bool tls_handle_ciphertext(struct l_tls *tls)
case TLS_CIPHER_BLOCK:
i = 0;
- if (tls->negotiated_version >= TLS_V11)
+ if (tls->negotiated_version >= L_TLS_V11)
i = tls->record_iv_length[0];
if (fragment_len <= tls->mac_length[0] + i) {
@@ -470,7 +470,7 @@ static bool tls_handle_ciphertext(struct l_tls *tls)
return false;
}
- if (tls->negotiated_version >= TLS_V12) {
+ if (tls->negotiated_version >= L_TLS_V12) {
if (!l_cipher_set_iv(tls->cipher[0],
tls->record_buf + 5,
tls->record_iv_length[0])) {
@@ -478,7 +478,7 @@ static bool tls_handle_ciphertext(struct l_tls *tls)
"Setting fragment IV failed");
return false;
}
- } else if (tls->negotiated_version >= TLS_V11)
+ } else if (tls->negotiated_version >= L_TLS_V11)
if (!l_cipher_decrypt(tls->cipher[0],
tls->record_buf + 5, iv,
tls->record_iv_length[0])) {
diff --git a/ell/tls.c b/ell/tls.c
index 8099e76..8abbabd 100644
--- a/ell/tls.c
+++ b/ell/tls.c
@@ -130,7 +130,7 @@ static bool tls_prf_get_bytes(struct l_tls *tls,
const void *seed, size_t seed_len,
uint8_t *buf, size_t len)
{
- if (tls->negotiated_version >= TLS_V12)
+ if (tls->negotiated_version >= L_TLS_V12)
return tls12_prf(tls->prf_hmac->l_id, tls->prf_hmac->length,
secret, secret_len, label,
seed, seed_len, buf, len);
@@ -332,7 +332,7 @@ static bool tls_change_cipher_spec(struct l_tls *tls, bool txrx,
key_offset += 2 * enc->key_length;
}
- if (tls->negotiated_version <= TLS_V10 &&
+ if (tls->negotiated_version <= L_TLS_V10 &&
tls->cipher_suite[txrx]->encryption &&
tls->cipher_suite[txrx]->encryption->cipher_type ==
TLS_CIPHER_BLOCK) {
@@ -532,10 +532,24 @@ static bool tls_cipher_suite_is_compatible(struct l_tls *tls,
{
static char error_buf[200];
struct l_cert *leaf;
+ uint16_t negotiated = tls->negotiated_version;
if (suite->encryption &&
suite->encryption->cipher_type == TLS_CIPHER_AEAD) {
- uint16_t negotiated = tls->negotiated_version;
+ if (tls->max_version < L_TLS_V12) {
+ if (error) {
+ *error = error_buf;
+ snprintf(error_buf, sizeof(error_buf),
+ "Cipher suite %s uses an AEAD "
+ "cipher (TLS 1.2+) but "
+ TLS_VER_FMT
+ " is the max version allowed",
+ suite->name,
+ TLS_VER_ARGS(tls->max_version));
+ }
+
+ return false;
+ }
if (negotiated && negotiated < L_TLS_V12) {
if (error) {
@@ -589,10 +603,13 @@ static bool tls_cipher_suite_is_compatible(struct l_tls *tls,
return false;
}
- if ((tls->negotiated_version && tls->negotiated_version < TLS_V12
&&
- (!l_checksum_is_supported(L_CHECKSUM_MD5, true) ||
- !l_checksum_is_supported(L_CHECKSUM_SHA1, true))) ||
- (tls->negotiated_version >= TLS_V12 &&
+ if (
+ ((tls->max_version < L_TLS_V12 ||
+ (negotiated && negotiated < L_TLS_V12)) &&
+ (!l_checksum_is_supported(L_CHECKSUM_MD5, true) ||
+ !l_checksum_is_supported(L_CHECKSUM_SHA1, true))) ||
+ ((tls->min_version >= L_TLS_V12 ||
+ tls->negotiated_version >= L_TLS_V12) &&
!l_checksum_is_supported(
suite->prf_hmac != L_CHECKSUM_NONE ?
suite->prf_hmac : L_CHECKSUM_SHA256,
@@ -664,8 +681,14 @@ static const struct tls_hash_algorithm tls_handshake_hash_data[] = {
static bool tls_init_handshake_hash(struct l_tls *tls)
{
enum handshake_hash_type hash;
+ bool tls10 = tls->max_version < L_TLS_V12;
for (hash = 0; hash < __HANDSHAKE_HASH_COUNT; hash++) {
+ /* Skip hash types we already know we won't need */
+ if (tls10 && hash != HANDSHAKE_HASH_SHA1 &&
+ hash != HANDSHAKE_HASH_MD5)
+ continue;
+
if (tls->handshake_hash[hash]) {
TLS_DEBUG("Handshake hash %s already exists",
tls_handshake_hash_data[hash].name);
@@ -820,8 +843,8 @@ static bool tls_send_client_hello(struct l_tls *tls)
/* Fill in the Client Hello body */
- *ptr++ = (uint8_t) (TLS_VERSION >> 8);
- *ptr++ = (uint8_t) (TLS_VERSION >> 0);
+ *ptr++ = (uint8_t) (tls->max_version >> 8);
+ *ptr++ = (uint8_t) (tls->max_version >> 0);
tls_write_random(tls->pending.client_random);
memcpy(ptr, tls->pending.client_random, 32);
@@ -843,7 +866,8 @@ static bool tls_send_client_hello(struct l_tls *tls)
}
if (ptr == len_ptr + 2) {
- TLS_DEBUG("No compatible cipher suites, check kernel config");
+ TLS_DEBUG("No compatible cipher suites, check kernel config, "
+ "certificate's key type and TLS version range");
return false;
}
@@ -1034,7 +1058,7 @@ static bool tls_send_certificate_request(struct l_tls *tls)
* affect both of these steps so revisit which set we're passing
* here.
*/
- if (tls->negotiated_version >= TLS_V12) {
+ if (tls->negotiated_version >= L_TLS_V12) {
signature_hash_ptr = ptr;
ptr += 2;
@@ -1112,7 +1136,7 @@ static void tls_generate_master_secret(struct l_tls *tls,
tls->pending.cipher_suite->mac->mac_length;
if (tls->pending.cipher_suite->encryption &&
- tls->negotiated_version <= TLS_V10 &&
+ tls->negotiated_version <= L_TLS_V10 &&
tls->pending.cipher_suite->encryption->cipher_type ==
TLS_CIPHER_BLOCK)
key_block_size += 2 *
@@ -1147,8 +1171,9 @@ static bool tls_send_rsa_client_key_xchg(struct l_tls *tls)
return false;
}
- pre_master_secret[0] = (uint8_t) (TLS_VERSION >> 8);
- pre_master_secret[1] = (uint8_t) (TLS_VERSION >> 0);
+ /* Must match the version in tls_send_client_hello */
+ pre_master_secret[0] = (uint8_t) (tls->max_version >> 8);
+ pre_master_secret[1] = (uint8_t) (tls->max_version >> 0);
l_getrandom(pre_master_secret + 2, 46);
if (tls->peer_pubkey_size + 32 > (int) sizeof(buf)) {
@@ -1198,7 +1223,7 @@ static ssize_t tls_rsa_sign(struct l_tls *tls, uint8_t *out, size_t
len,
return -ENOKEY;
}
- if (tls->negotiated_version >= TLS_V12) {
+ if (tls->negotiated_version >= L_TLS_V12) {
const struct tls_hash_algorithm *hash_type =
&tls_handshake_hash_data[tls->signature_hash];
@@ -1250,7 +1275,7 @@ static bool tls_rsa_verify(struct l_tls *tls, const uint8_t *in,
size_t len,
/* 2 bytes for SignatureAndHashAlgorithm if version >= 1.2 */
offset = 2;
- if (tls->negotiated_version < TLS_V12)
+ if (tls->negotiated_version < L_TLS_V12)
offset = 0;
if (len < offset + 2 ||
@@ -1271,7 +1296,7 @@ static bool tls_rsa_verify(struct l_tls *tls, const uint8_t *in,
size_t len,
return false;
}
- if (tls->negotiated_version >= TLS_V12) {
+ if (tls->negotiated_version >= L_TLS_V12) {
/* Only RSA supported */
if (in[1] != 1 /* RSA_sign */) {
TLS_DISCONNECT(TLS_ALERT_DECRYPT_ERROR, 0,
@@ -1392,7 +1417,7 @@ static bool tls_send_certificate_verify(struct l_tls *tls)
return false;
/* Stop maintaining handshake message hashes other than the PRF hash */
- if (tls->negotiated_version >= TLS_V12)
+ if (tls->negotiated_version >= L_TLS_V12)
for (i = 0; i < __HANDSHAKE_HASH_COUNT; i++)
if (&tls_handshake_hash_data[i] != tls->prf_hmac)
tls_drop_handshake_hash(tls, i);
@@ -1417,7 +1442,7 @@ static void tls_send_finished(struct l_tls *tls)
uint8_t seed[HANDSHAKE_HASH_MAX_SIZE * 2];
size_t seed_len;
- if (tls->negotiated_version >= TLS_V12) {
+ if (tls->negotiated_version >= L_TLS_V12) {
/* Same hash type as that used for the PRF (usually SHA256) */
enum handshake_hash_type hash;
@@ -1458,7 +1483,7 @@ static bool tls_verify_finished(struct l_tls *tls, const uint8_t
*received,
return false;
}
- if (tls->negotiated_version >= TLS_V12) {
+ if (tls->negotiated_version >= L_TLS_V12) {
enum handshake_hash_type hash;
for (hash = 0; hash < __HANDSHAKE_HASH_COUNT; hash++)
@@ -1574,18 +1599,18 @@ static void tls_handle_client_hello(struct l_tls *tls,
/* Save client_version for Premaster Secret verification */
tls->client_version = l_get_be16(buf);
- if (tls->client_version < TLS_MIN_VERSION) {
+ if (tls->client_version < tls->min_version) {
TLS_DISCONNECT(TLS_ALERT_PROTOCOL_VERSION, 0,
"Client version too low: %02x",
tls->client_version);
return;
}
- tls->negotiated_version = TLS_VERSION < tls->client_version ?
- TLS_VERSION : tls->client_version;
+ tls->negotiated_version = tls->client_version > tls->max_version ?
+ tls->max_version : tls->client_version;
/* Stop maintaining handshake message hashes other than MD1 and SHA. */
- if (tls->negotiated_version < TLS_V12)
+ if (tls->negotiated_version < L_TLS_V12)
for (i = 0; i < __HANDSHAKE_HASH_COUNT; i++)
if (i != HANDSHAKE_HASH_SHA1 && i != HANDSHAKE_HASH_MD5)
tls_drop_handshake_hash(tls, i);
@@ -1703,9 +1728,9 @@ static void tls_handle_server_hello(struct l_tls *tls,
tls->negotiated_version = l_get_be16(buf);
- if (tls->negotiated_version < TLS_MIN_VERSION ||
- tls->negotiated_version > TLS_VERSION) {
- TLS_DISCONNECT(tls->negotiated_version < TLS_MIN_VERSION ?
+ if (tls->negotiated_version < tls->min_version ||
+ tls->negotiated_version > tls->max_version) {
+ TLS_DISCONNECT(tls->negotiated_version < tls->min_version ?
TLS_ALERT_PROTOCOL_VERSION :
TLS_ALERT_ILLEGAL_PARAM, 0,
"Unsupported version %02x",
@@ -1714,7 +1739,7 @@ static void tls_handle_server_hello(struct l_tls *tls,
}
/* Stop maintaining handshake message hashes other than MD1 and SHA. */
- if (tls->negotiated_version < TLS_V12)
+ if (tls->negotiated_version < L_TLS_V12)
for (i = 0; i < __HANDSHAKE_HASH_COUNT; i++)
if (i != HANDSHAKE_HASH_SHA1 && i != HANDSHAKE_HASH_MD5)
tls_drop_handshake_hash(tls, i);
@@ -1911,7 +1936,7 @@ static void tls_handle_certificate_request(struct l_tls *tls,
* lists for use in tls_send_certificate.
*/
- if (tls->negotiated_version >= TLS_V12) {
+ if (tls->negotiated_version >= L_TLS_V12) {
/*
* This only makes sense as a variable-length field, assume
* there's a typo in RFC5246 7.4.4 here.
@@ -2125,7 +2150,7 @@ static void tls_handle_certificate_verify(struct l_tls *tls,
return;
/* Stop maintaining handshake message hashes other than the PRF hash */
- if (tls->negotiated_version >= TLS_V12)
+ if (tls->negotiated_version >= L_TLS_V12)
for (i = 0; i < __HANDSHAKE_HASH_COUNT; i++)
if (&tls_handshake_hash_data[i] != tls->prf_hmac)
tls_drop_handshake_hash(tls, i);
@@ -2458,6 +2483,8 @@ LIB_EXPORT struct l_tls *l_tls_new(bool server,
tls->ready_handle = ready_handler;
tls->disconnected = disconnect_handler;
tls->user_data = user_data;
+ tls->min_version = TLS_MIN_VERSION;
+ tls->max_version = TLS_MAX_VERSION;
tls->signature_hash = HANDSHAKE_HASH_SHA256;
@@ -2751,6 +2778,18 @@ LIB_EXPORT bool l_tls_set_auth_data(struct l_tls *tls, const char
*cert_path,
return true;
}
+LIB_EXPORT void l_tls_set_version_range(struct l_tls *tls,
+ uint16_t min_version,
+ uint16_t max_version)
+{
+ tls->min_version =
+ (min_version && min_version > TLS_MIN_VERSION) ?
+ min_version : TLS_MIN_VERSION;
+ tls->max_version =
+ (max_version && max_version < TLS_MAX_VERSION) ?
+ max_version : TLS_MAX_VERSION;
+}
+
LIB_EXPORT const char *l_tls_alert_to_str(enum l_tls_alert_desc desc)
{
switch (desc) {
diff --git a/ell/tls.h b/ell/tls.h
index fb33404..5b2f398 100644
--- a/ell/tls.h
+++ b/ell/tls.h
@@ -25,6 +25,10 @@
extern "C" {
#endif
+#define L_TLS_V12 ((3 << 8) | 3)
+#define L_TLS_V11 ((3 << 8) | 2)
+#define L_TLS_V10 ((3 << 8) | 1)
+
struct l_tls;
enum l_tls_alert_desc {
@@ -63,7 +67,6 @@ typedef void (*l_tls_disconnect_cb_t)(enum l_tls_alert_desc reason,
typedef void (*l_tls_debug_cb_t)(const char *str, void *user_data);
typedef void (*l_tls_destroy_cb_t)(void *user_data);
-
/*
* app_data_handler gets called with newly received decrypted data.
* tx_handler gets called to send TLS payloads off to remote end.
@@ -107,6 +110,9 @@ bool l_tls_set_auth_data(struct l_tls *tls, const char *cert_path,
const char *priv_key_path,
const char *priv_key_passphrase);
+void l_tls_set_version_range(struct l_tls *tls,
+ uint16_t min_version, uint16_t max_version);
+
const char *l_tls_alert_to_str(enum l_tls_alert_desc desc);
enum l_checksum_type;
--
2.19.1
8:55 a.m.
New subject: [PATCH 4/9] tls: Implement l_tls_set_version_range
Hi Andrew,
On 12/13/2018 01:57 PM, Andrew Zaborowski wrote:
Allow user to set custom min and max TLS version limits, this can be
used to ensure we comply with a specific security profile.
---
ell/ell.sym | 1 +
ell/tls-private.h | 10 ++---
ell/tls-record.c | 14 +++----
ell/tls.c | 99 +++++++++++++++++++++++++++++++++--------------
ell/tls.h | 8 +++-
5 files changed, 87 insertions(+), 45 deletions(-)
diff --git a/ell/ell.sym b/ell/ell.sym
index 7d7a5e4..2ff7d30 100644
--- a/ell/ell.sym
+++ b/ell/ell.sym
@@ -417,6 +417,7 @@ global:
l_tls_close;
l_tls_set_cacert;
l_tls_set_auth_data;
+ l_tls_set_version_range;
l_tls_alert_to_str;
l_tls_set_debug;
/* uintset */
diff --git a/ell/tls-private.h b/ell/tls-private.h
index 8e6c277..e2ec014 100644
--- a/ell/tls-private.h
+++ b/ell/tls-private.h
@@ -20,13 +20,8 @@
*
*/
-/* Only TLS 1.2 supported */
-#define TLS_V12 ((3 << 8) | 3)
-#define TLS_V11 ((3 << 8) | 2)
-#define TLS_V10 ((3 << 8) | 1)
-
-#define TLS_VERSION TLS_V12
-#define TLS_MIN_VERSION TLS_V10
+#define TLS_MAX_VERSION L_TLS_V12
+#define TLS_MIN_VERSION L_TLS_V10
enum tls_cipher_type {
TLS_CIPHER_STREAM,
@@ -145,6 +140,7 @@ struct l_tls {
l_tls_debug_cb_t debug_handler;
l_tls_destroy_cb_t debug_destroy;
void *debug_data;
+ uint16_t min_version, max_version;
So why isn't this an enum?
struct l_queue *ca_certs;
struct l_certchain *cert;
<snip>
diff --git a/ell/tls.h b/ell/tls.h
index fb33404..5b2f398 100644
--- a/ell/tls.h
+++ b/ell/tls.h
@@ -25,6 +25,10 @@
extern "C" {
#endif
+#define L_TLS_V12 ((3 << 8) | 3)
+#define L_TLS_V11 ((3 << 8) | 2)
+#define L_TLS_V10 ((3 << 8) | 1)
+
This can easily be an enum, no?
struct l_tls;
enum l_tls_alert_desc {
@@ -63,7 +67,6 @@ typedef void (*l_tls_disconnect_cb_t)(enum l_tls_alert_desc reason,
typedef void (*l_tls_debug_cb_t)(const char *str, void *user_data);
typedef void (*l_tls_destroy_cb_t)(void *user_data);
-
/*
* app_data_handler gets called with newly received decrypted data.
* tx_handler gets called to send TLS payloads off to remote end.
@@ -107,6 +110,9 @@ bool l_tls_set_auth_data(struct l_tls *tls, const char *cert_path,
const char *priv_key_path,
const char *priv_key_passphrase);
+void l_tls_set_version_range(struct l_tls *tls,
+ uint16_t min_version, uint16_t max_version);
+
Same here?
const char *l_tls_alert_to_str(enum l_tls_alert_desc desc);
enum l_checksum_type;
Regards,
-Denis
12:57 p.m.
New subject: [PATCH 5/9] unit: Test TLS 1.0, 1.1 and 1.2
---
unit/test-tls.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/unit/test-tls.c b/unit/test-tls.c
index 04fd3ea..f39d95a 100644
--- a/unit/test-tls.c
+++ b/unit/test-tls.c
@@ -417,7 +417,8 @@ static void tls_debug_cb(const char *str, void *user_data)
l_info("%s %s", (const char *) user_data, str);
}
-static void test_tls_test(const void *data)
+static void test_tls_with_ver(const void *data,
+ uint16_t server_ver, uint16_t client_ver)
{
bool auth_ok;
const struct tls_conn_test *test = data;
@@ -452,6 +453,12 @@ static void test_tls_test(const void *data)
assert(s[0].tls);
assert(s[1].tls);
+ if (server_ver)
+ l_tls_set_version_range(s[0].tls, server_ver, server_ver);
+
+ if (client_ver)
+ l_tls_set_version_range(s[1].tls, client_ver, client_ver);
+
if (getenv("TLS_SERVER_DEBUG"))
l_tls_set_debug(s[0].tls, tls_debug_cb, "server", NULL);
@@ -491,6 +498,18 @@ static void test_tls_test(const void *data)
l_tls_free(s[1].tls);
}
+static void test_tls_test(const void *data)
+{
+ /*
+ * 1.2 should get negotiated in the first case. If the three
+ * scenarios succeed that's already good but can be checked with:
+ * $ TLS_DEBUG=1 unit/test-tls 2>&1 | grep "Negotiated"
+ */
+ test_tls_with_ver(data, 0, 0);
+ test_tls_with_ver(data, 0, L_TLS_V11);
+ test_tls_with_ver(data, L_TLS_V10, 0);
+}
+
int main(int argc, char *argv[])
{
l_test_init(&argc, &argv);
--
2.19.1
12:57 p.m.
New subject: [PATCH 6/9] unit: Move tls_cert_load_file to relevant unit tests
tls_cert_load_file and l_pem_load_certificate have no users other than
the two unit tests and have little to do with TLS.
---
unit/test-key.c | 26 ++++++++++++++++++++------
unit/test-tls.c | 28 +++++++++++++++++++++-------
2 files changed, 41 insertions(+), 13 deletions(-)
diff --git a/unit/test-key.c b/unit/test-key.c
index 53fb394..6b519f5 100644
--- a/unit/test-key.c
+++ b/unit/test-key.c
@@ -392,6 +392,20 @@ static void test_simple_keyring(const void *data)
l_key_free(key2);
}
+static struct l_cert *load_cert_file(const char *filename)
+{
+ const uint8_t *der;
+ size_t len;
+ char *label;
+
+ der = l_pem_load_file(filename, 0, &label, &len);
+ if (!der)
+ return NULL;
+
+ l_free(label);
+ return l_cert_new_from_der(der, len);
+}
+
static void test_trusted_keyring(const void *data)
{
struct l_keyring *ring;
@@ -402,9 +416,9 @@ static void test_trusted_keyring(const void *data)
struct l_key *key;
bool success;
- cacert = tls_cert_load_file(CERTDIR "cert-ca.pem");
+ cacert = load_cert_file(CERTDIR "cert-ca.pem");
assert(cacert);
- cert = tls_cert_load_file(CERTDIR "cert-server.pem");
+ cert = load_cert_file(CERTDIR "cert-server.pem");
assert(cert);
cakey = l_cert_get_pubkey(cacert);
@@ -447,11 +461,11 @@ static void test_trust_chain(const void *data)
struct l_key *key;
bool success;
- cacert = tls_cert_load_file(CERTDIR "cert-ca.pem");
+ cacert = load_cert_file(CERTDIR "cert-ca.pem");
assert(cacert);
- intcert = tls_cert_load_file(CERTDIR "cert-intca.pem");
+ intcert = load_cert_file(CERTDIR "cert-intca.pem");
assert(intcert);
- cert = tls_cert_load_file(CERTDIR "cert-entity-int.pem");
+ cert = load_cert_file(CERTDIR "cert-entity-int.pem");
assert(cert);
cakey = l_cert_get_pubkey(cacert);
@@ -543,7 +557,7 @@ static void test_key_crypto(const void *data)
int hash = L_CHECKSUM_NONE;
int rsa = L_KEY_RSA_PKCS1_V1_5;
- cert = tls_cert_load_file(CERTDIR "cert-client.pem");
+ cert = load_cert_file(CERTDIR "cert-client.pem");
assert(cert);
pubkey = l_cert_get_pubkey(cert);
assert(pubkey);
diff --git a/unit/test-tls.c b/unit/test-tls.c
index f39d95a..2b5d16b 100644
--- a/unit/test-tls.c
+++ b/unit/test-tls.c
@@ -210,6 +210,20 @@ static void test_tls12_prf(const void *data)
assert(!memcmp(out_buf, test->expected, test->out_len));
}
+static struct l_cert *load_cert_file(const char *filename)
+{
+ const uint8_t *der;
+ size_t len;
+ char *label;
+
+ der = l_pem_load_file(filename, 0, &label, &len);
+ if (!der)
+ return NULL;
+
+ l_free(label);
+ return l_cert_new_from_der(der, len);
+}
+
static void test_certificates(const void *data)
{
struct l_queue *cacert;
@@ -246,13 +260,13 @@ static void test_certificates(const void *data)
assert(l_certchain_verify(chain2, twocas, NULL));
chain3 = certchain_new_from_leaf(
- tls_cert_load_file(CERTDIR "cert-server.pem"));
+ load_cert_file(CERTDIR "cert-server.pem"));
certchain_link_issuer(chain3,
- tls_cert_load_file(CERTDIR "cert-entity-int.pem"));
+ load_cert_file(CERTDIR "cert-entity-int.pem"));
certchain_link_issuer(chain3,
- tls_cert_load_file(CERTDIR "cert-intca.pem"));
+ load_cert_file(CERTDIR "cert-intca.pem"));
certchain_link_issuer(chain3,
- tls_cert_load_file(CERTDIR "cert-ca.pem"));
+ load_cert_file(CERTDIR "cert-ca.pem"));
assert(chain3);
assert(!l_certchain_verify(chain3, wrongca, NULL));
@@ -261,11 +275,11 @@ static void test_certificates(const void *data)
assert(!l_certchain_verify(chain3, twocas, NULL));
chain4 = certchain_new_from_leaf(
- tls_cert_load_file(CERTDIR "cert-entity-int.pem"));
+ load_cert_file(CERTDIR "cert-entity-int.pem"));
certchain_link_issuer(chain4,
- tls_cert_load_file(CERTDIR "cert-intca.pem"));
+ load_cert_file(CERTDIR "cert-intca.pem"));
certchain_link_issuer(chain4,
- tls_cert_load_file(CERTDIR "cert-ca.pem"));
+ load_cert_file(CERTDIR "cert-ca.pem"));
assert(chain4);
assert(!l_certchain_verify(chain4, wrongca, NULL));
--
2.19.1
9:01 a.m.
New subject: [PATCH 6/9] unit: Move tls_cert_load_file to relevant unit tests
Hi Andrew,
On 12/13/2018 01:57 PM, Andrew Zaborowski wrote:
tls_cert_load_file and l_pem_load_certificate have no users other
than
the two unit tests and have little to do with TLS.
---
unit/test-key.c | 26 ++++++++++++++++++++------
unit/test-tls.c | 28 +++++++++++++++++++++-------
2 files changed, 41 insertions(+), 13 deletions(-)
Patch 6 & 7 applied, thanks.
Regards,
-Denis
12:57 p.m.
New subject: [PATCH 7/9] tls, pem: Drop tls_cert_load_file, l_pem_load_certificate
These functions have no users left.
---
ell/ell.sym | 1 -
ell/pem.c | 20 --------------------
ell/pem.h | 1 -
ell/tls-private.h | 1 -
ell/tls.c | 15 ---------------
5 files changed, 38 deletions(-)
diff --git a/ell/ell.sym b/ell/ell.sym
index 2ff7d30..23d2349 100644
--- a/ell/ell.sym
+++ b/ell/ell.sym
@@ -337,7 +337,6 @@ global:
l_netlink_set_debug;
/* pem */
l_pem_load_buffer;
- l_pem_load_certificate;
l_pem_load_certificate_chain;
l_pem_load_certificate_list;
l_pem_load_file;
diff --git a/ell/pem.c b/ell/pem.c
index dffd476..fa08962 100644
--- a/ell/pem.c
+++ b/ell/pem.c
@@ -245,26 +245,6 @@ LIB_EXPORT uint8_t *l_pem_load_file(const char *filename, int index,
return result;
}
-LIB_EXPORT uint8_t *l_pem_load_certificate(const char *filename, size_t *len)
-{
- uint8_t *content;
- char *label;
-
- content = l_pem_load_file(filename, 0, &label, len);
-
- if (!content)
- return NULL;
-
- if (strcmp(label, "CERTIFICATE")) {
- l_free(content);
- content = NULL;
- }
-
- l_free(label);
-
- return content;
-}
-
LIB_EXPORT struct l_certchain *l_pem_load_certificate_chain(
const char *filename)
{
diff --git a/ell/pem.h b/ell/pem.h
index bb9e53a..fc5b019 100644
--- a/ell/pem.h
+++ b/ell/pem.h
@@ -37,7 +37,6 @@ uint8_t *l_pem_load_buffer(const uint8_t *buf, size_t buf_len,
uint8_t *l_pem_load_file(const char *filename, int index,
char **type_label, size_t *len);
-uint8_t *l_pem_load_certificate(const char *filename, size_t *len);
struct l_certchain *l_pem_load_certificate_chain(const char *filename);
struct l_queue *l_pem_load_certificate_list(const char *filename);
diff --git a/ell/tls-private.h b/ell/tls-private.h
index e2ec014..3fda4b5 100644
--- a/ell/tls-private.h
+++ b/ell/tls-private.h
@@ -233,7 +233,6 @@ void tls_tx_record(struct l_tls *tls, enum tls_content_type type,
bool tls_handle_message(struct l_tls *tls, const uint8_t *message,
int len, enum tls_content_type type, uint16_t version);
-struct l_cert *tls_cert_load_file(const char *filename);
int tls_parse_certificate_list(const void *data, size_t len,
struct l_certchain **out_certchain);
diff --git a/ell/tls.c b/ell/tls.c
index 8abbabd..59d7f5a 100644
--- a/ell/tls.c
+++ b/ell/tls.c
@@ -2868,21 +2868,6 @@ const char *tls_handshake_state_to_str(enum tls_handshake_state
state)
return buf;
}
-struct l_cert *tls_cert_load_file(const char *filename)
-{
- uint8_t *der;
- size_t len;
- struct l_cert *cert;
-
- der = l_pem_load_certificate(filename, &len);
- if (!der)
- return NULL;
-
- cert = l_cert_new_from_der(der, len);
- l_free(der);
- return cert;
-}
-
int tls_parse_certificate_list(const void *data, size_t len,
struct l_certchain **out_certchain)
{
--
2.19.1
12:57 p.m.
New subject: [PATCH 8/9] tls: Allow user to set custom list of cipher suites
TLS "security profiles" define which minimum cipher suites and other
parameter values are allowed in a specific usage scenario for TLS, for
example WPA3 in theory requires a specific security profile for its
TLS-based EAP method. This will also allow the unit tests to test
individual cipher suites.
---
ell/ell.sym | 1 +
ell/tls-private.h | 2 +
ell/tls.c | 101 ++++++++++++++++++++++++++++++++++++++++++----
ell/tls.h | 3 ++
4 files changed, 99 insertions(+), 8 deletions(-)
diff --git a/ell/ell.sym b/ell/ell.sym
index 23d2349..e4f157e 100644
--- a/ell/ell.sym
+++ b/ell/ell.sym
@@ -417,6 +417,7 @@ global:
l_tls_set_cacert;
l_tls_set_auth_data;
l_tls_set_version_range;
+ l_tls_set_cipher_suites;
l_tls_alert_to_str;
l_tls_set_debug;
/* uintset */
diff --git a/ell/tls-private.h b/ell/tls-private.h
index 3fda4b5..29ce8f9 100644
--- a/ell/tls-private.h
+++ b/ell/tls-private.h
@@ -147,6 +147,8 @@ struct l_tls {
struct l_key *priv_key;
size_t priv_key_size;
+ struct tls_cipher_suite **cipher_suite_pref_list;
+
/* Record layer */
uint8_t *record_buf;
diff --git a/ell/tls.c b/ell/tls.c
index 59d7f5a..61bcc19 100644
--- a/ell/tls.c
+++ b/ell/tls.c
@@ -41,6 +41,7 @@
#include "tls-private.h"
#include "key.h"
#include "asn1-private.h"
+#include "strv.h"
bool tls10_prf(const void *secret, size_t secret_len,
const char *label,
@@ -840,6 +841,7 @@ static bool tls_send_client_hello(struct l_tls *tls)
uint8_t *ptr = buf + TLS_HANDSHAKE_HEADER_SIZE;
uint8_t *len_ptr;
int i;
+ struct tls_cipher_suite **suite;
/* Fill in the Client Hello body */
@@ -855,14 +857,16 @@ static bool tls_send_client_hello(struct l_tls *tls)
len_ptr = ptr;
ptr += 2;
- for (i = 0; i < (int) L_ARRAY_SIZE(tls_cipher_suite_pref); i++) {
- if (!tls_cipher_suite_is_compatible(tls,
- &tls_cipher_suite_pref[i],
- NULL))
+ for (suite = tls->cipher_suite_pref_list; *suite; suite++) {
+ const char *error;
+
+ if (!tls_cipher_suite_is_compatible(tls, *suite, &error)) {
+ TLS_DEBUG("non-fatal: %s", error);
continue;
+ }
- *ptr++ = tls_cipher_suite_pref[i].id[0];
- *ptr++ = tls_cipher_suite_pref[i].id[1];
+ *ptr++ = (*suite)->id[0];
+ *ptr++ = (*suite)->id[1];
}
if (ptr == len_ptr + 2) {
@@ -1524,6 +1528,7 @@ static void tls_handle_client_hello(struct l_tls *tls,
const uint8_t *cipher_suites;
const uint8_t *compression_methods;
int i;
+ enum l_tls_alert_desc alert_desc = TLS_ALERT_HANDSHAKE_FAIL;
/* Do we have enough for ProtocolVersion + Random + SessionID size? */
if (len < 2 + 32 + 1)
@@ -1618,12 +1623,39 @@ static void tls_handle_client_hello(struct l_tls *tls,
TLS_DEBUG("Negotiated TLS " TLS_VER_FMT,
TLS_VER_ARGS(tls->negotiated_version));
+ if (!tls->cipher_suite_pref_list[0]) {
+ TLS_DISCONNECT(TLS_ALERT_INTERNAL_ERROR, 0,
+ "No usable cipher suites");
+ return;
+ }
+
/* Select a cipher suite according to client's preference list */
while (cipher_suites_size) {
struct tls_cipher_suite *suite =
tls_find_cipher_suite(cipher_suites);
+ struct tls_cipher_suite **iter;
+ const char *error;
+
+ for (iter = tls->cipher_suite_pref_list; *iter; iter++)
+ if (*iter == suite)
+ break;
- if (suite && tls_cipher_suite_is_compatible(tls, suite, NULL)) {
+ if (!suite)
+ TLS_DEBUG("non-fatal: Cipher suite %04x unknown",
+ l_get_be16(cipher_suites));
+ else if (!tls_cipher_suite_is_compatible(tls, suite, &error))
+ TLS_DEBUG("non-fatal: %s", error);
+ else if (!*iter) {
+ /*
+ * We have at least one matching compatible suite but
+ * it is not allowed in this security profile. If the
+ * handshake ends up failing then we blame the security
+ * profile.
+ */
+ alert_desc = TLS_ALERT_INSUFFICIENT_SECURITY;
+ TLS_DEBUG("non-fatal: Cipher suite %s disallowed "
+ "by config", suite->name);
+ } else {
tls->pending.cipher_suite = suite;
break;
}
@@ -1633,7 +1665,7 @@ static void tls_handle_client_hello(struct l_tls *tls,
}
if (!cipher_suites_size) {
- TLS_DISCONNECT(TLS_ALERT_HANDSHAKE_FAIL, 0,
+ TLS_DISCONNECT(alert_desc, 0,
"No common cipher suites matching negotiated "
"TLS version and our certificate's key type");
return;
@@ -1701,6 +1733,7 @@ static void tls_handle_server_hello(struct l_tls *tls,
{
uint8_t session_id_size, cipher_suite_id[2], compression_method_id;
const char *error;
+ struct tls_cipher_suite **iter;
int i;
/* Do we have enough for ProtocolVersion + Random + SessionID len ? */
@@ -1756,6 +1789,16 @@ static void tls_handle_server_hello(struct l_tls *tls,
return;
}
+ for (iter = tls->cipher_suite_pref_list; *iter; iter++)
+ if (*iter == tls->pending.cipher_suite)
+ break;
+ if (!*iter) {
+ TLS_DISCONNECT(TLS_ALERT_INSUFFICIENT_SECURITY, 0,
+ "Selected cipher suite %s disallowed by config",
+ tls->pending.cipher_suite->name);
+ return;
+ }
+
if (!tls_cipher_suite_is_compatible(tls, tls->pending.cipher_suite,
&error)) {
TLS_DISCONNECT(TLS_ALERT_HANDSHAKE_FAIL, 0,
@@ -2485,6 +2528,7 @@ LIB_EXPORT struct l_tls *l_tls_new(bool server,
tls->user_data = user_data;
tls->min_version = TLS_MIN_VERSION;
tls->max_version = TLS_MAX_VERSION;
+ l_tls_set_cipher_suites(tls, NULL);
tls->signature_hash = HANDSHAKE_HASH_SHA256;
@@ -2525,6 +2569,8 @@ LIB_EXPORT void l_tls_free(struct l_tls *tls)
if (tls->debug_destroy)
tls->debug_destroy(tls->debug_data);
+ l_free(tls->cipher_suite_pref_list);
+
l_free(tls);
}
@@ -2790,6 +2836,45 @@ LIB_EXPORT void l_tls_set_version_range(struct l_tls *tls,
max_version : TLS_MAX_VERSION;
}
+LIB_EXPORT void l_tls_set_cipher_suites(struct l_tls *tls,
+ const char **suite_list)
+{
+ unsigned int i, len;
+ struct tls_cipher_suite **suite;
+
+ l_free(tls->cipher_suite_pref_list);
+
+ if (suite_list)
+ len = l_strv_length((char **) suite_list);
+ else
+ len = L_ARRAY_SIZE(tls_cipher_suite_pref);
+
+ tls->cipher_suite_pref_list = l_new(struct tls_cipher_suite *, len + 1);
+
+ if (!suite_list) {
+ /* Use our default cipher suite preference list */
+ for (i = 0; i < L_ARRAY_SIZE(tls_cipher_suite_pref); i++)
+ tls->cipher_suite_pref_list[i] =
+ &tls_cipher_suite_pref[i];
+
+ return;
+ }
+
+ suite = tls->cipher_suite_pref_list;
+
+ for (; *suite_list; suite_list++) {
+ for (i = 0; i < L_ARRAY_SIZE(tls_cipher_suite_pref); i++)
+ if (!strcmp(tls_cipher_suite_pref[i].name, *suite_list))
+ break;
+
+ if (i < L_ARRAY_SIZE(tls_cipher_suite_pref))
+ *suite++ = &tls_cipher_suite_pref[i];
+ else
+ TLS_DEBUG("Cipher suite %s is not supported",
+ *suite_list);
+ }
+}
+
LIB_EXPORT const char *l_tls_alert_to_str(enum l_tls_alert_desc desc)
{
switch (desc) {
diff --git a/ell/tls.h b/ell/tls.h
index 5b2f398..c684dc2 100644
--- a/ell/tls.h
+++ b/ell/tls.h
@@ -113,6 +113,9 @@ bool l_tls_set_auth_data(struct l_tls *tls, const char *cert_path,
void l_tls_set_version_range(struct l_tls *tls,
uint16_t min_version, uint16_t max_version);
+/* Optionally limit allowed cipher suites to a custom set */
+void l_tls_set_cipher_suites(struct l_tls *tls, const char **suite_list);
+
const char *l_tls_alert_to_str(enum l_tls_alert_desc desc);
enum l_checksum_type;
--
2.19.1
9:33 a.m.
New subject: [PATCH 8/9] tls: Allow user to set custom list of cipher suites
Hi Andrew,
On 12/13/2018 01:57 PM, Andrew Zaborowski wrote:
TLS "security profiles" define which minimum cipher suites
and other
parameter values are allowed in a specific usage scenario for TLS, for
example WPA3 in theory requires a specific security profile for its
TLS-based EAP method. This will also allow the unit tests to test
individual cipher suites.
Do we want to expose this fine level of granularity in a public API
though? It would seem at least at WPA2 vs WPA3 level that a single
SuiteB selector is all that is needed?
Unless you have a usecase in mind where one would really want to twiddle
the preference order?
---
ell/ell.sym | 1 +
ell/tls-private.h | 2 +
ell/tls.c | 101 ++++++++++++++++++++++++++++++++++++++++++----
ell/tls.h | 3 ++
4 files changed, 99 insertions(+), 8 deletions(-)
<snip>
<snip>
@@ -2790,6 +2836,45 @@ LIB_EXPORT void l_tls_set_version_range(struct
l_tls *tls,
max_version : TLS_MAX_VERSION;
}
+LIB_EXPORT void l_tls_set_cipher_suites(struct l_tls *tls,
+ const char **suite_list)
void? Don't we want to fail if nothing valid was passed?
And wouldn't / shouldn't l_tls_start() also fail in this case?
+{
+ unsigned int i, len;
+ struct tls_cipher_suite **suite;
+
+ l_free(tls->cipher_suite_pref_list);
+
+ if (suite_list)
+ len = l_strv_length((char **) suite_list);
I hate C and its char ** / const char ** thing. Oh well :)
+ else
+ len = L_ARRAY_SIZE(tls_cipher_suite_pref);
You take note of len here, but then...
+
+ tls->cipher_suite_pref_list = l_new(struct tls_cipher_suite *, len + 1);
+
+ if (!suite_list) {
+ /* Use our default cipher suite preference list */
+ for (i = 0; i < L_ARRAY_SIZE(tls_cipher_suite_pref); i++)
use array_size here again. Maybe a nitpick, but using len would be
shorter anyway.
Another question is whether storing a bool default_cipher_pref : 1
instead of doing all this magic dancing would be easier.
+ tls->cipher_suite_pref_list[i] =
+ &tls_cipher_suite_pref[i];
+
+ return;
+ }
+
+ suite = tls->cipher_suite_pref_list;
+
+ for (; *suite_list; suite_list++) {
+ for (i = 0; i < L_ARRAY_SIZE(tls_cipher_suite_pref); i++)
+ if (!strcmp(tls_cipher_suite_pref[i].name, *suite_list))
+ break;
+
+ if (i < L_ARRAY_SIZE(tls_cipher_suite_pref))
+ *suite++ = &tls_cipher_suite_pref[i];
+ else
+ TLS_DEBUG("Cipher suite %s is not supported",
+ *suite_list);
+ }
+}
+
LIB_EXPORT const char *l_tls_alert_to_str(enum l_tls_alert_desc desc)
{
switch (desc) {
Regards,
-Denis
12:12 p.m.
New subject: [PATCH 8/9] tls: Allow user to set custom list of cipher suites
On Fri, 14 Dec 2018 at 17:33, Denis Kenzior <denkenz(a)gmail.com> wrote:
On 12/13/2018 01:57 PM, Andrew Zaborowski wrote:
> TLS "security profiles" define which minimum cipher suites and other
> parameter values are allowed in a specific usage scenario for TLS, for
> example WPA3 in theory requires a specific security profile for its
> TLS-based EAP method. This will also allow the unit tests to test
> individual cipher suites.
Do we want to expose this fine level of granularity in a public API
though? It would seem at least at WPA2 vs WPA3 level that a single
SuiteB selector is all that is needed?
Yes, but there are more TLS profiles out there, like the BCP-195
profile and three profiles called DICOM profiles so we probably don't
want to define APIs for each and instead can have the user read the
spec and copy the cipher suite names?
I note that openssl does let you set/list cipher suites and so do some
https servers.
Unless you have a usecase in mind where one would really want to twiddle
the preference order?
> ---
> ell/ell.sym | 1 +
> ell/tls-private.h | 2 +
> ell/tls.c | 101 ++++++++++++++++++++++++++++++++++++++++++----
> ell/tls.h | 3 ++
> 4 files changed, 99 insertions(+), 8 deletions(-)
>
<snip>
<snip>
> @@ -2790,6 +2836,45 @@ LIB_EXPORT void l_tls_set_version_range(struct l_tls *tls,
> max_version : TLS_MAX_VERSION;
> }
>
> +LIB_EXPORT void l_tls_set_cipher_suites(struct l_tls *tls,
> + const char **suite_list)
void? Don't we want to fail if nothing valid was passed?
And wouldn't / shouldn't l_tls_start() also fail in this case?
l_tls_start() *should* fail so I thought it's easier on the user if
they only need a single conditional statement after setting the
versions and cipher suites instead of checking at every step.
> +{
> + unsigned int i, len;
> + struct tls_cipher_suite **suite;
> +
> + l_free(tls->cipher_suite_pref_list);
> +
> + if (suite_list)
> + len = l_strv_length((char **) suite_list);
I hate C and its char ** / const char ** thing. Oh well :)
> + else
> + len = L_ARRAY_SIZE(tls_cipher_suite_pref);
You take note of len here, but then...
> +
> + tls->cipher_suite_pref_list = l_new(struct tls_cipher_suite *, len + 1);
> +
> + if (!suite_list) {
> + /* Use our default cipher suite preference list */
> + for (i = 0; i < L_ARRAY_SIZE(tls_cipher_suite_pref); i++)
use array_size here again. Maybe a nitpick, but using len would be
shorter anyway.
Ok, good point.
Another question is whether storing a bool default_cipher_pref : 1
instead of doing all this magic dancing would be easier.
So I initially was just setting a NULL tls->cipher_suite_pref_list to
indicate we're using the defaults but I then tried to concentrate the
conditional code in one place as there's this one place where we set
this list but many places that have to read and process it and they
were not easy to move into one function. We already have some rather
convoluted checks when we negotiate the versions and cipher suites.
> + tls->cipher_suite_pref_list[i] =
> + &tls_cipher_suite_pref[i];
> +
> + return;
> + }
> +
> + suite = tls->cipher_suite_pref_list;
> +
> + for (; *suite_list; suite_list++) {
> + for (i = 0; i < L_ARRAY_SIZE(tls_cipher_suite_pref); i++)
> + if (!strcmp(tls_cipher_suite_pref[i].name, *suite_list))
> + break;
> +
> + if (i < L_ARRAY_SIZE(tls_cipher_suite_pref))
> + *suite++ = &tls_cipher_suite_pref[i];
> + else
> + TLS_DEBUG("Cipher suite %s is not supported",
> + *suite_list);
> + }
> +}
> +
> LIB_EXPORT const char *l_tls_alert_to_str(enum l_tls_alert_desc desc)
> {
> switch (desc) {
Regards,
-Denis
_______________________________________________
ell mailing list
ell(a)lists.01.org
https://lists.01.org/mailman/listinfo/ell
12:28 p.m.
New subject: [PATCH 8/9] tls: Allow user to set custom list of cipher suites
Hi Andrew,
On 12/14/2018 01:12 PM, Andrew Zaborowski wrote:
On Fri, 14 Dec 2018 at 17:33, Denis Kenzior <denkenz(a)gmail.com>
wrote:
> On 12/13/2018 01:57 PM, Andrew Zaborowski wrote:
>> TLS "security profiles" define which minimum cipher suites and other
>> parameter values are allowed in a specific usage scenario for TLS, for
>> example WPA3 in theory requires a specific security profile for its
>> TLS-based EAP method. This will also allow the unit tests to test
>> individual cipher suites.
>
> Do we want to expose this fine level of granularity in a public API
> though? It would seem at least at WPA2 vs WPA3 level that a single
> SuiteB selector is all that is needed?
Yes, but there are more TLS profiles out there, like the BCP-195
profile and three profiles called DICOM profiles so we probably don't
want to define APIs for each and instead can have the user read the
spec and copy the cipher suite names?
Yes, but you also have to consider the audience for our TLS
implementation. It isn't browsers. It is just iwd and maybe eventually
ConnMan for the portals stuff. So can we optimize the API for those use
cases ?
I note that openssl does let you set/list cipher suites and so do
some
https servers.
Perhaps you should just keep this private for now.
>> +LIB_EXPORT void l_tls_set_cipher_suites(struct l_tls *tls,
>> + const char **suite_list)
>
> void? Don't we want to fail if nothing valid was passed?
>
> And wouldn't / shouldn't l_tls_start() also fail in this case?
l_tls_start() *should* fail so I thought it's easier on the user if
they only need a single conditional statement after setting the
versions and cipher suites instead of checking at every step.
But the server side at least doesn't? Still, I'd think it would be
useful to have a return value for this or make this private...
>
> Another question is whether storing a bool default_cipher_pref : 1
> instead of doing all this magic dancing would be easier.
So I initially was just setting a NULL tls->cipher_suite_pref_list to
indicate we're using the defaults but I then tried to concentrate the
conditional code in one place as there's this one place where we set
this list but many places that have to read and process it and they
were not easy to move into one function. We already have some rather
convoluted checks when we negotiate the versions and cipher suites.
But you could just assign tls->cipher_suite_pref_list to the global
value and twiddle a bit. Then the bit just has to be checked here and
in l_tls_free when deciding whether to use l_free(). Or use a const
pointer for the rest of the code and a non-const pointer to store the
allocated value (or NULL).
Seems easier to me anyway.
Regards,
Denis
12:49 p.m.
New subject: [PATCH 8/9] tls: Allow user to set custom list of cipher suites
On Fri, 14 Dec 2018 at 20:28, Denis Kenzior <denkenz(a)gmail.com> wrote:
On 12/14/2018 01:12 PM, Andrew Zaborowski wrote:
> On Fri, 14 Dec 2018 at 17:33, Denis Kenzior <denkenz(a)gmail.com> wrote:
>> On 12/13/2018 01:57 PM, Andrew Zaborowski wrote:
>>> TLS "security profiles" define which minimum cipher suites and
other
>>> parameter values are allowed in a specific usage scenario for TLS, for
>>> example WPA3 in theory requires a specific security profile for its
>>> TLS-based EAP method. This will also allow the unit tests to test
>>> individual cipher suites.
>>
>> Do we want to expose this fine level of granularity in a public API
>> though? It would seem at least at WPA2 vs WPA3 level that a single
>> SuiteB selector is all that is needed?
>
> Yes, but there are more TLS profiles out there, like the BCP-195
> profile and three profiles called DICOM profiles so we probably don't
> want to define APIs for each and instead can have the user read the
> spec and copy the cipher suite names?
>
Yes, but you also have to consider the audience for our TLS
implementation. It isn't browsers. It is just iwd and maybe eventually
ConnMan for the portals stuff. So can we optimize the API for those use
cases ?
I don't think we should but ok. BTW we won't be Suite B compliant for
a while still because of the DSA certificates and likely more things.
> I note that openssl does let you set/list cipher suites and so do some
> https servers.
>
Perhaps you should just keep this private for now.
Ok.
>>> +LIB_EXPORT void l_tls_set_cipher_suites(struct l_tls *tls,
>>> + const char **suite_list)
>>
>> void? Don't we want to fail if nothing valid was passed?
>>
>> And wouldn't / shouldn't l_tls_start() also fail in this case?
>
> l_tls_start() *should* fail so I thought it's easier on the user if
> they only need a single conditional statement after setting the
> versions and cipher suites instead of checking at every step.
But the server side at least doesn't? Still, I'd think it would be
useful to have a return value for this or make this private...
Ok, if we're keeping it private let's return a bool.
>>
>> Another question is whether storing a bool default_cipher_pref : 1
>> instead of doing all this magic dancing would be easier.
>
> So I initially was just setting a NULL tls->cipher_suite_pref_list to
> indicate we're using the defaults but I then tried to concentrate the
> conditional code in one place as there's this one place where we set
> this list but many places that have to read and process it and they
> were not easy to move into one function. We already have some rather
> convoluted checks when we negotiate the versions and cipher suites.
But you could just assign tls->cipher_suite_pref_list to the global
value and twiddle a bit. Then the bit just has to be checked here and
in l_tls_free when deciding whether to use l_free(). Or use a const
pointer for the rest of the code and a non-const pointer to store the
allocated value (or NULL).
Seems easier to me anyway.
Yeah, I considered just doing:
if (tls->cipher_suite_pref_list != tls_cipher_suite_pref)
l_free(tls->cipher_suite_pref_list);
but one is just an array of cipher suites and the other is an array of
pointers to cipher suites, perhaps I can convert tls_cipher_suite_pref
to an array of pointers.
BTW since tls.c is pretty big we could split out all the cipher suite
definitions and the functions used by them into tls-suites.c or
similar. For ECDHE we'll need to add extensions support and I'm also
thinking of defining the extensions in a new file tls-extensions.c
(we'll support very few initially but the different specs actually
define many).
Best regards
12:57 p.m.
New subject: [PATCH 8/9] tls: Allow user to set custom list of cipher suites
Hi Andrew,
> Yes, but you also have to consider the audience for our TLS
> implementation. It isn't browsers. It is just iwd and maybe eventually
> ConnMan for the portals stuff. So can we optimize the API for those use
> cases ?
I don't think we should but ok. BTW we won't be Suite B compliant for
a while still because of the DSA certificates and likely more things.
Right, I forgot SuiteB actually implies DSA. But we don't need SuiteB
anyway. WPA3 allows:
- TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA38
ECDHE using the 384-bit prime modulus curve P-384
- TLS_DHE_RSA_WITH_AES_256_GCM_SHA384
So perhaps we just need a 'WPA3' selector. And once we have SuiteB
support, then we can add TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384 to WPA3
selector as well.
BTW since tls.c is pretty big we could split out all the cipher
suite
definitions and the functions used by them into tls-suites.c or
similar. For ECDHE we'll need to add extensions support and I'm also
thinking of defining the extensions in a new file tls-extensions.c
(we'll support very few initially but the different specs actually
define many).
Sure. At 3kloc tls.c isn't too bad, but we can break it up. Propose
something.
Regards,
-Denis
12:57 p.m.
New subject: [PATCH 9/9] unit: Test many TLS cipher suite and version combinations
---
unit/test-tls.c | 147 +++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 139 insertions(+), 8 deletions(-)
diff --git a/unit/test-tls.c b/unit/test-tls.c
index 2b5d16b..8c1bc01 100644
--- a/unit/test-tls.c
+++ b/unit/test-tls.c
@@ -302,11 +302,16 @@ struct tls_conn_test {
const char *server_key_passphrase;
const char *server_ca_cert_path;
const char *server_expect_identity;
+ const char **server_cipher_suites;
const char *client_cert_path;
const char *client_key_path;
const char *client_key_passphrase;
const char *client_ca_cert_path;
const char *client_expect_identity;
+ const char **client_cipher_suites;
+ bool expect_alert;
+ bool expect_client_start_fail;
+ enum l_tls_alert_desc alert_desc;
};
static const struct tls_conn_test tls_conn_test_no_auth = {
@@ -362,6 +367,63 @@ static const struct tls_conn_test tls_conn_test_full_auth = {
.client_expect_identity = "Foo Example Organization",
};
+static const struct tls_conn_test tls_conn_test_bad_server_suite = {
+ .server_cert_path = CERTDIR "cert-server.pem",
+ .server_key_path = CERTDIR "cert-server-key-pkcs8.pem",
+ .server_ca_cert_path = CERTDIR "cert-ca.pem",
+ .server_expect_identity = "Bar Example Organization",
+ .server_cipher_suites = (const char *[]) { "UNKNOWN", NULL },
+ .client_cert_path = CERTDIR "cert-client.pem",
+ .client_key_path = CERTDIR "cert-client-key-pkcs8.pem",
+ .client_ca_cert_path = CERTDIR "cert-ca.pem",
+ .client_expect_identity = "Foo Example Organization",
+ .expect_alert = true,
+ .alert_desc = TLS_ALERT_INTERNAL_ERROR,
+};
+
+static const struct tls_conn_test tls_conn_test_bad_client_suite = {
+ .server_cert_path = CERTDIR "cert-server.pem",
+ .server_key_path = CERTDIR "cert-server-key-pkcs8.pem",
+ .server_ca_cert_path = CERTDIR "cert-ca.pem",
+ .server_expect_identity = "Bar Example Organization",
+ .client_cert_path = CERTDIR "cert-client.pem",
+ .client_key_path = CERTDIR "cert-client-key-pkcs8.pem",
+ .client_ca_cert_path = CERTDIR "cert-ca.pem",
+ .client_expect_identity = "Foo Example Organization",
+ .client_cipher_suites = (const char *[]) { "UNKNOWN", NULL },
+ .expect_client_start_fail = true,
+};
+
+static const struct tls_conn_test tls_conn_test_suite_mismatch = {
+ .server_cert_path = CERTDIR "cert-server.pem",
+ .server_key_path = CERTDIR "cert-server-key-pkcs8.pem",
+ .server_ca_cert_path = CERTDIR "cert-ca.pem",
+ .server_expect_identity = "Bar Example Organization",
+ .server_cipher_suites =
+ (const char *[]) { "TLS_RSA_WITH_AES_128_CBC_SHA256", NULL },
+ .client_cert_path = CERTDIR "cert-client.pem",
+ .client_key_path = CERTDIR "cert-client-key-pkcs8.pem",
+ .client_ca_cert_path = CERTDIR "cert-ca.pem",
+ .client_expect_identity = "Foo Example Organization",
+ .client_cipher_suites =
+ (const char *[]) { "TLS_RSA_WITH_AES_256_CBC_SHA256", NULL },
+ .expect_alert = true,
+ .alert_desc = TLS_ALERT_INSUFFICIENT_SECURITY,
+};
+
+static const struct tls_conn_test tls_conn_test_version_mismatch = {
+ .server_cert_path = CERTDIR "cert-server.pem",
+ .server_key_path = CERTDIR "cert-server-key-pkcs8.pem",
+ .server_ca_cert_path = CERTDIR "cert-ca.pem",
+ .server_expect_identity = "Bar Example Organization",
+ .client_cert_path = CERTDIR "cert-client.pem",
+ .client_key_path = CERTDIR "cert-client-key-pkcs8.pem",
+ .client_ca_cert_path = CERTDIR "cert-ca.pem",
+ .client_expect_identity = "Foo Example Organization",
+ .expect_alert = true,
+ .alert_desc = TLS_ALERT_PROTOCOL_VERSION,
+};
+
#define identity_compare(a, b) ((!(a) && !(b)) || ((a) && (b) &&
!strcmp(a, b)))
struct tls_test_state {
@@ -378,6 +440,9 @@ struct tls_test_state {
const char *send_data;
const char *expect_data;
const char *expect_peer;
+
+ bool expect_alert;
+ enum l_tls_alert_desc alert_desc;
};
static void tls_test_new_data(const uint8_t *data, size_t len, void *user_data)
@@ -411,6 +476,7 @@ static void tls_test_ready(const char *peer_identity, void
*user_data)
struct tls_test_state *s = user_data;
assert(!s->ready);
+ assert(!s->expect_alert);
assert((!s->expect_peer && !peer_identity) ||
(s->expect_peer && peer_identity &&
!strcmp(s->expect_peer, peer_identity)));
@@ -423,7 +489,11 @@ static void tls_test_ready(const char *peer_identity, void
*user_data)
static void tls_test_disconnected(enum l_tls_alert_desc reason, bool remote,
void *user_data)
{
- assert(false);
+ struct tls_test_state *s = user_data;
+
+ assert(s->expect_alert);
+ assert(reason == s->alert_desc);
+ s->success = true;
}
static void tls_debug_cb(const char *str, void *user_data)
@@ -431,11 +501,10 @@ static void tls_debug_cb(const char *str, void *user_data)
l_info("%s %s", (const char *) user_data, str);
}
-static void test_tls_with_ver(const void *data,
+static void test_tls_with_ver(const struct tls_conn_test *test,
uint16_t server_ver, uint16_t client_ver)
{
bool auth_ok;
- const struct tls_conn_test *test = data;
struct tls_test_state s[2] = {
{
.ready = false,
@@ -445,6 +514,8 @@ static void test_tls_with_ver(const void *data,
.send_data = "server to client",
.expect_data = "client to server",
.expect_peer = test->server_expect_identity,
+ .expect_alert = test->expect_alert,
+ .alert_desc = test->alert_desc,
},
{
.ready = false,
@@ -454,6 +525,8 @@ static void test_tls_with_ver(const void *data,
.send_data = "client to server",
.expect_data = "server to client",
.expect_peer = test->client_expect_identity,
+ .expect_alert = test->expect_alert,
+ .alert_desc = test->alert_desc,
},
};
@@ -473,6 +546,12 @@ static void test_tls_with_ver(const void *data,
if (client_ver)
l_tls_set_version_range(s[1].tls, client_ver, client_ver);
+ if (test->server_cipher_suites)
+ l_tls_set_cipher_suites(s[0].tls, test->server_cipher_suites);
+
+ if (test->client_cipher_suites)
+ l_tls_set_cipher_suites(s[1].tls, test->client_cipher_suites);
+
if (getenv("TLS_SERVER_DEBUG"))
l_tls_set_debug(s[0].tls, tls_debug_cb, "server", NULL);
@@ -491,9 +570,12 @@ static void test_tls_with_ver(const void *data,
assert(l_tls_set_cacert(s[1].tls, test->client_ca_cert_path));
assert(l_tls_start(s[0].tls));
- assert(l_tls_start(s[1].tls));
+ assert(!!l_tls_start(s[1].tls) == !test->expect_client_start_fail);
- while (1) {
+ if (test->expect_client_start_fail)
+ goto done;
+
+ while (!(test->expect_alert && s[0].success && s[1].success)) {
if (s[0].raw_buf_len) {
l_tls_handle_rx(s[1].tls, s[0].raw_buf,
s[0].raw_buf_len);
@@ -508,24 +590,60 @@ static void test_tls_with_ver(const void *data,
assert(s[0].success && s[1].success);
+done:
l_tls_free(s[0].tls);
l_tls_free(s[1].tls);
}
static void test_tls_test(const void *data)
{
+ const struct tls_conn_test *test = data;
+
/*
* 1.2 should get negotiated in the first case. If the three
* scenarios succeed that's already good but can be checked with:
* $ TLS_DEBUG=1 unit/test-tls 2>&1 | grep "Negotiated"
*/
- test_tls_with_ver(data, 0, 0);
- test_tls_with_ver(data, 0, L_TLS_V11);
- test_tls_with_ver(data, L_TLS_V10, 0);
+ test_tls_with_ver(test, 0, 0);
+ test_tls_with_ver(test, 0, L_TLS_V11);
+ test_tls_with_ver(test, L_TLS_V10, 0);
+}
+
+static void test_tls_version_mismatch_test(const void *data)
+{
+ test_tls_with_ver(&tls_conn_test_version_mismatch,
+ L_TLS_V11, L_TLS_V12);
+ test_tls_with_ver(&tls_conn_test_version_mismatch,
+ L_TLS_V12, L_TLS_V10);
+ test_tls_with_ver(&tls_conn_test_version_mismatch,
+ L_TLS_V10, L_TLS_V11);
+}
+
+static void test_tls_suite_test(const void *data)
+{
+ const char *suite_name = data;
+ const char *client_cipher_suites[] = { suite_name, NULL };
+ struct tls_conn_test test = tls_conn_test_full_auth;
+
+ test.client_cipher_suites = client_cipher_suites;
+ test_tls_with_ver(&test, 0, 0);
}
int main(int argc, char *argv[])
{
+ const char *suite_names[] = {
+ "TLS_RSA_WITH_RC4_128_MD5",
+ "TLS_RSA_WITH_RC4_128_SHA",
+ "TLS_RSA_WITH_3DES_EDE_CBC_SHA",
+ "TLS_RSA_WITH_AES_128_CBC_SHA",
+ "TLS_RSA_WITH_AES_256_CBC_SHA",
+ "TLS_RSA_WITH_AES_128_CBC_SHA256",
+ "TLS_RSA_WITH_AES_256_CBC_SHA256",
+ "TLS_RSA_WITH_AES_128_GCM_SHA256",
+ "TLS_RSA_WITH_AES_256_GCM_SHA384",
+ NULL
+ }, **name;
+
l_test_init(&argc, &argv);
if (!l_checksum_is_supported(L_CHECKSUM_MD5, false) ||
@@ -584,6 +702,19 @@ int main(int argc, char *argv[])
l_test_add("TLS connection full auth", test_tls_test,
&tls_conn_test_full_auth);
+ l_test_add("TLS connection bad server cipher suite", test_tls_test,
+ &tls_conn_test_bad_server_suite);
+ l_test_add("TLS connection bad client cipher suite", test_tls_test,
+ &tls_conn_test_bad_client_suite);
+ l_test_add("TLS connection cipher suite mismatch", test_tls_test,
+ &tls_conn_test_suite_mismatch);
+
+ l_test_add("TLS connection version mismatch",
+ test_tls_version_mismatch_test, NULL);
+
+ for (name = suite_names; *name; name++)
+ l_test_add(*name, test_tls_suite_test, *name);
+
done:
return l_test_run();
}
--
2.19.1
1295
days inactive
1296
days old
17 comments
2 participants
participants (2)
-
Andrew Zaborowski -
Denis Kenzior