Client cert renegotiation in http #687

fAuernigg · 2023-02-08T14:16:46Z

Add server support for certificate renegotiation.
Fix tls state machine on client and server for tls1.2 renegotiation.

Feature Commits:
tls: handle client hello/finished state during tls 1.2 renegotiation
tls: handle server done state during tls 1.2 renegotiation

Commits with new Apis:
tls, http: add tls and http apis for renegotiation
Add tls_set_conn_verify_client_handler,
tls_renegotiate, tls_disable_session_on_reneg,
tls_set_posthandshake_auth, tls_conn_verify_client_post_handshake,
tls_conn_version
Add http_sock_tls

Commit to fix retest on win32:
http: fix read_file on win32 (incorrect get fsize)

sreimers · 2023-02-09T15:00:07Z

include/re_tls.h

+int tls_renegotiate(const struct tls_conn *tc);
+int tls_disable_session_on_reneg(struct tls_conn *tc);
+int tls_set_posthandshake_auth(struct tls *tls, int enabled);
+#endif


We should try to avoid ifdef in public APIs, its fine if libre.so is build with Libressl and a application is build with openssl.

BTW: any chance to make these new apis LibreSSL compatible?

sreimers · 2023-02-09T15:06:08Z

include/re_tls.h

@@ -8,6 +8,10 @@ struct tls;
 struct tls_conn;
 struct tcp_conn;
 struct udp_sock;
+#if !defined(LIBRESSL_VERSION_NUMBER)
+struct x509_store_ctx_st;
+typedef struct x509_store_ctx_st X509_STORE_CTX;


The TLS api should be generic to not to be bound to a specific ssl library.

cspiel1 · 2023-02-16T16:18:31Z

include/re_tls.h

@@ -28,6 +28,10 @@ enum tls_keytype {
 	TLS_KEYTYPE_EC,
 };

+struct tls_conn_d {
+	int (*verify_cb) (int ok, void *d);
+	void *verify_d;


rename to

struct tls_conn_d { int (*verifyh) (int ok, void *arg); void *arg;

cspiel1 · 2023-02-16T16:27:09Z

include/re_tls.h

@@ -46,6 +50,10 @@ int tls_set_certificate_der(struct tls *tls, enum tls_keytype keytype,
 			    const uint8_t *key, size_t len_key);
 int tls_set_certificate(struct tls *tls, const char *cert, size_t len);
 void tls_set_verify_client(struct tls *tls);
+void tls_set_verify_client_trust_all(struct tls *tls);
+int tls_set_conn_verify_client_handler(struct tls_conn *tc, int depth,
+	int (*cb) (int ok, void *d), void *d);


rename to

int tls_set_verify_client_handler(struct tls_conn *tc, int depth, int (*verifyh) (int ok, void *arg), void *arg);

cspiel1 · 2023-02-16T16:29:34Z

include/re_tls.h


 /* TCP */

 int tls_conn_change_cert(struct tls_conn *tc, const char *file);
 int tls_start_tcp(struct tls_conn **ptc, struct tls *tls,
 		  struct tcp_conn *tcp, int layer);

+int tls_conn_verify_client_post_handshake(struct tls_conn *tc);


rename

int tls_verify_client_post_handshake(struct tls_conn *tc);

cspiel1 · 2023-02-16T16:30:56Z

include/re_tls.h


 /* TCP */

 int tls_conn_change_cert(struct tls_conn *tc, const char *file);
 int tls_start_tcp(struct tls_conn **ptc, struct tls *tls,
 		  struct tcp_conn *tcp, int layer);

+int tls_conn_verify_client_post_handshake(struct tls_conn *tc);
+const char* tls_conn_version(struct tls_conn *tc);


rename to

const char* tls_version(struct tls_conn *tc);

cspiel1 · 2023-02-16T17:01:02Z

src/http/client.c

-	if (n < (size_t)s) {
-		mem_deref(buf);
-		return EIO;
+	while ((n=fread(buf+idx, 1, PEMBUF_SIZE, f)) == PEMBUF_SIZE) {


maybe we should use the portable function fs_fopen()

cspiel1 · 2023-02-16T17:03:18Z

src/http/client.c

+			return ENOMEM;
+		}
+		buf = nbuf;
+		memset(buf+idx, 0, PEMBUF_SIZE);


We should use mbuf:

struct mbuf *cert; struct mbuf *key;

Please try this patch:
http_client_read_mbuf.patch.zip

cspiel1 · 2023-02-16T17:06:22Z

src/tls/openssl/tls.c

@@ -39,6 +40,7 @@ struct session_reuse {
 	struct hash *ht_sessions;
 };

+


cspiel1 · 2023-02-16T17:06:38Z

src/tls/openssl/tls.c

@@ -61,6 +63,7 @@ struct tls_cert {
 	char *host;
 };

+


cspiel1 · 2023-02-16T17:08:46Z

src/tls/openssl/tls.c

+ * Set TLS server context to request certificate from peer
+ * and set trust all certificates of peer.
+ *
+ * @deprecated Use tls_set_verify_peer_trust_all instead


please prepare also a PR for baresip to replace the deprecated function:

./modules/dtls_srtp/dtls_srtp.c:534: tls_set_verify_client(tls);

baresip/baresip#2461

cspiel1 · 2023-02-16T17:11:11Z

src/tls/openssl/tls_tcp.c

@@ -28,6 +28,7 @@
 struct tls_conn {
 	SSL *ssl;             /* inheritance */
 	struct tls *tls;      /* inheritance */
+	struct tls_conn_d cd; /* inheritance */


inheritance?

"struct tls_conn_d cd;" is inherited in tls.c and defined in tls_*.c simliar to tls and ssl.

cspiel1 · 2023-02-16T17:11:34Z

src/tls/openssl/tls_udp.c

@@ -60,6 +60,7 @@ struct dtls_sock {
 struct tls_conn {
 	SSL *ssl;             /* inheritance */
 	struct tls *tls;      /* inheritance */
+	struct tls_conn_d cd; /* inheritance */


inheritance?

Add tls_set_conn_verify_client_handler, tls_renegotiate, tls_disable_session_on_reneg, tls_set_posthandshake_auth, tls_conn_verify_client_post_handshake, tls_conn_version Add http_sock_tls

alfredh · 2023-02-26T09:45:03Z

this patch is quite large, around 1100 lines

is it possible to split it up into smaller parts, for easier review ?

fAuernigg · 2023-02-27T14:26:05Z

this patch is quite large, around 1100 lines

is it possible to split it up into smaller parts, for easier review ?

I added PR #711, #712, #713.

The libre test added in this PR #713 requires new api's to test renegotiation in the http server.

This test requires #712 to fix renegotiation in the tls state machine under TLS v1.2
This test requires #711 to fix reading certificate files during this test on Win32.

So what I wanted to say is PR #713 will never have the green light for test ok until TLS #712 / #711 was merged.
This PR (#713) is to test the tls state update of PR #712.

cspiel1 · 2023-02-27T14:51:45Z

Is replaced by #711, #712, #713.

fAuernigg force-pushed the client_cert_renegotiation_in_http branch from 121ef13 to 6476e0d Compare February 9, 2023 14:27

sreimers reviewed Feb 9, 2023

View reviewed changes

fAuernigg force-pushed the client_cert_renegotiation_in_http branch 12 times, most recently from adb661a to 811e5de Compare February 15, 2023 16:03

cspiel1 reviewed Feb 16, 2023

View reviewed changes

src/tls/openssl/tls.c Outdated

@@ -39,6 +40,7 @@ struct session_reuse {

struct hash *ht_sessions;

};

Copy link

Collaborator

cspiel1 Feb 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unrelated

cspiel1 reviewed Feb 16, 2023

View reviewed changes

src/tls/openssl/tls.c Outdated

@@ -61,6 +63,7 @@ struct tls_cert {

char *host;

};

Copy link

Collaborator

cspiel1 Feb 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unrelated

cspiel1 reviewed Feb 16, 2023

View reviewed changes

fAuernigg force-pushed the client_cert_renegotiation_in_http branch from 811e5de to 98c6d9a Compare February 20, 2023 07:56

fAuernigg added 3 commits February 21, 2023 10:06

tls, http: add tls and http apis for renegotiation

cd16ae1

Add tls_set_conn_verify_client_handler, tls_renegotiate, tls_disable_session_on_reneg, tls_set_posthandshake_auth, tls_conn_verify_client_post_handshake, tls_conn_version Add http_sock_tls

tls: handle client hello/finished state during tls 1.2 renegotiation

001df3a

tls: handle server done state during tls 1.2 renegotiation

2fd3875

fAuernigg added 2 commits February 21, 2023 10:06

http: fix read_file on win32 (wrong filesize) and use mbuf

8b18658

test/http: add test http renegotation with client cert validation

e240a55

fAuernigg force-pushed the client_cert_renegotiation_in_http branch from 98c6d9a to e240a55 Compare February 21, 2023 09:12

fAuernigg marked this pull request as ready for review February 21, 2023 14:26

cspiel1 closed this Feb 27, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Client cert renegotiation in http #687

Client cert renegotiation in http #687

fAuernigg commented Feb 8, 2023 •

edited

Loading

sreimers Feb 9, 2023

sreimers Feb 9, 2023

sreimers Feb 9, 2023 •

edited

Loading

cspiel1 Feb 16, 2023 •

edited

Loading

cspiel1 Feb 16, 2023

cspiel1 Feb 16, 2023

cspiel1 Feb 16, 2023

cspiel1 Feb 16, 2023

cspiel1 Feb 16, 2023 •

edited

Loading

cspiel1 Feb 16, 2023

cspiel1 Feb 16, 2023

cspiel1 Feb 16, 2023

fAuernigg Feb 20, 2023

cspiel1 Feb 16, 2023

fAuernigg Feb 20, 2023

cspiel1 Feb 16, 2023

alfredh commented Feb 26, 2023

fAuernigg commented Feb 27, 2023 •

edited

Loading

cspiel1 commented Feb 27, 2023

		@@ -39,6 +40,7 @@ struct session_reuse {
		struct hash *ht_sessions;
		};

Client cert renegotiation in http #687

Client cert renegotiation in http #687

Conversation

fAuernigg commented Feb 8, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sreimers Feb 9, 2023 • edited Loading

Choose a reason for hiding this comment

cspiel1 Feb 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cspiel1 Feb 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alfredh commented Feb 26, 2023

fAuernigg commented Feb 27, 2023 • edited Loading

cspiel1 commented Feb 27, 2023

fAuernigg commented Feb 8, 2023 •

edited

Loading

sreimers Feb 9, 2023 •

edited

Loading

cspiel1 Feb 16, 2023 •

edited

Loading

cspiel1 Feb 16, 2023 •

edited

Loading

fAuernigg commented Feb 27, 2023 •

edited

Loading