tls: set selfsigned Elliptic Curve (EC) function #17

cHuberCoffee · 2020-08-26T10:07:31Z

A new function to generate a self signed cert using elliptic curve cryptographic primitives.
The used elliptic curve is hardcoded with NID_secp521r1.
One can use this function in the barsip dtls_srtp module.

sreimers · 2020-08-26T10:27:35Z

src/tls/openssl/tls.c

+		X509_free(cert);
+
+
+	return err = 0;


This return assignment looks a bit strange should it return err;?

yes u are right. should be a normal return err;

alfredh · 2020-08-28T17:04:25Z

src/tls/openssl/tls.c


 	if (!X509_set_pubkey(cert, key))
 		goto out;

-	if (!X509_sign(cert, key, EVP_sha1()))
+	if (!X509_sign(cert, key, EVP_sha256()))


is this change done on purpose ?

not on purpose for this PR.
but on purpose in general. Since the SHA1 algo. is broken. This effects the cert. signatures with SHA1 as well. Using SHA2 alogs. is a small but effective change.

I agree to the change, but I think it should be a separate PR.
also very important to test that it works.

alfredh · 2020-08-28T17:05:33Z

have you done any interop testing with WebRTC browsers ?
(i.e. using DTLS-SRTP with EC certificate).

cHuberCoffee · 2020-09-02T11:49:22Z

I'm working on a setup baresip-asterisk-sipML5 to test the dtls-srtp with the EC certificates.
It's a good idea in general to work through such a testsetup, to get a deeper understanding of the underlying structures.

I will give you more information of my results asap.

cHuberCoffee · 2020-09-10T09:53:50Z

The setup is now baresip<->asterisk<->sipML5. The communication happens between these points. The key exchange via DTLSv1.2 is working with the cert. signed via ECDSA-SHA256 version. The media stream is established and audio data is transmitted.

alfredh · 2020-09-10T09:59:27Z

did you try to call in both directions ? the DTLS server is normally the caller.

also please test with Chrome and Firefox.

alfredh · 2020-09-29T08:19:46Z

I have tested this patch with Chrome and CtxSip. The calling direction is from baresip to CtxSip.

The DTLS handshake fails:

dtls_srtp: incoming DTLS connect from 10.0.1.10:51789 
tls: 140736434426816:error:1417A0C1:SSL routines:tls_post_process_client_hello:no shared cipher:ssl/statem/statem_srvr.c:2284:
dtls: accept error: 1
dtls_srtp: dtls_accept failed (Protocol error)

alfredh · 2020-09-29T08:23:09Z

at Wire I implemented this function to generate ECDSA cert:

https://github.com/wireapp/wire-audio-video-signaling/blob/master/src/cert/cert.c#L37

perhaps this missing part will make it work:

	EC_KEY_set_asn1_flag(ec_key, OPENSSL_EC_NAMED_CURVE);

cHuberCoffee · 2020-10-14T08:46:25Z

The test-setup is running with the original RSA method. I tested RSA and EC signed certificates against CtxSip with Firefox and Chrome. Both (RSA and EC) worked for both browsers. Call direction was barsip->CtxSip.

alfredh · 2020-10-21T09:38:05Z

I tested the original code (no patches) between Safari and baresip-webrtc:

dtls_srtp: media=audio -- start DTLS client
dtls_srtp: component start: RTP [raddr=192.168.88.36:53019]
dtls_srtp: 'audio,RTP' dtls connect to 192.168.88.36:53019
    >> dtls cipher: ECDHE-ECDSA-CHACHA20-POLY1305
dtls_srtp: verified sha-256 fingerprint OK
dtls_srtp: ---> DTLS-SRTP complete (video/RTP) Profile=AES_CM_128_HMAC_SHA1_80
rtcsession: mediaenc event 'Secure' (video,RTP)

please note that in most cases, the DTLS Server is normally the same as the caller.

the negotiated cipher depends heavily on the calling direction.

try to add this in your code:

	info("    >> dtls cipher: %s\n", tls_cipher_name(comp->tls_conn));

and try to call in both directions, and then check if the cipher is different.

alfredh · 2020-10-21T15:35:23Z

additional test with ctxSip, calling both directions:

chrome -> baresip:
.... dtls cipher: ECDHE-ECDSA-CHACHA20-POLY1305

baresip -> chrome:
.... dtls cipher: ECDHE-RSA-AES128-GCM-SHA256

alfredh · 2020-10-21T15:41:54Z

with your patch and _ec function used in baresip dtls_srtp module, DTLS handshake fails:

dtls_srtp: incoming DTLS connect from 10.0.1.10:55045
tls: 140736434426816:error:1417A0C1:SSL routines:tls_post_process_client_hello:no shared cipher:ssl/statem/statem_srvr.c:2284:
dtls: accept error: 1
dtls_srtp: dtls_accept failed (Protocol error)
stream: update 'audio'
stream: audio: starting mediaenc 'dtls_srtp' (wait_secure=1)
sip:[email protected]: session closed: Connection reset by peer
sip:[email protected]: Call with sip:[email protected] terminated (duration: 9 secs)

--- System info: ---
 Machine:  x86_64/darwin
 Version:  1.0.0 (libre v1.1.0)
 Build:    64-bit little endian
 Kernel:   Darwin Johns-MacBook-Pro.local 16.7.0 Darwin Kernel Version 16.7.0: Wed Apr 24 20:50:53 PDT 2019; root:xnu-3789.73.49~1/RELEASE_X86_64 x86_64
 Uptime:   1 min 48 secs
 Started:  Wed Oct 21 17:38:21 2020
 Compiler: 4.2.1 Compatible Apple LLVM 9.0.0 (clang-900.0.39.2)
 OpenSSL:  OpenSSL 1.1.1h  22 Sep 2020

calling direction is from baresip to chrome/ctxsip.

alfredh · 2020-10-21T17:22:03Z

src/tls/openssl/tls.c

 	int r, err = ENOMEM;

 	if (!tls || !cn)
 		return EINVAL;

-	rsa = RSA_new();
-	if (!rsa)
+	eckey = EC_KEY_new_by_curve_name(NID_secp521r1);


perhaps the curve name can be added as a function argument.
then the application can choose which one to use.

alfredh · 2020-10-21T17:24:05Z

perhaps this curve will make it work: "prime256v1"

alfredh · 2020-11-02T11:42:43Z

please have a look at this specification:

https://tools.ietf.org/html/draft-ietf-rtcweb-security-arch-20#section-6.5

   All Implementations MUST support DTLS 1.2 with the
   TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256 cipher suite and the P-256
   curve [FIPS186].

if I change the curve name in your patch to use "prime256v1", it works fine.

suggest using this curve or make it configurable via argument string.

alfredh · 2020-11-05T12:32:36Z

this looks good.

a small ccheck warning must be fixed before merging:

./include/re_tls.h:39: has trailing space(s)

sreimers · 2020-11-09T11:08:15Z

@cHuberCoffee please prevent force-push, its easier to review normal commits and the changes. for cleanup we can squash the pull request afterwards (if necessary).

alfredh · 2020-11-09T12:31:58Z

looks good now.

ready to merge to master, I would say ..

cHuberCoffee force-pushed the tls_selfsigned_EC branch from 315b5e0 to cef92be Compare August 26, 2020 10:11

sreimers reviewed Aug 26, 2020

View reviewed changes

cHuberCoffee force-pushed the tls_selfsigned_EC branch from cef92be to 47ac5b5 Compare August 26, 2020 11:15

alfredh reviewed Aug 28, 2020

View reviewed changes

cHuberCoffee marked this pull request as draft October 7, 2020 08:00

cHuberCoffee force-pushed the tls_selfsigned_EC branch from 47ac5b5 to 0a50d64 Compare October 14, 2020 08:42

cHuberCoffee marked this pull request as ready for review October 14, 2020 09:29

alfredh reviewed Oct 21, 2020

View reviewed changes

cHuberCoffee marked this pull request as draft October 22, 2020 07:10

cHuberCoffee force-pushed the tls_selfsigned_EC branch 2 times, most recently from 77d76b5 to f573e27 Compare November 4, 2020 14:25

tls: set selfsigned Elliptic Curve (EC) function

c963268

cHuberCoffee force-pushed the tls_selfsigned_EC branch from f573e27 to c963268 Compare November 9, 2020 11:00

sreimers marked this pull request as ready for review November 9, 2020 12:32

sreimers merged commit e40b2f4 into baresip:master Nov 9, 2020

cHuberCoffee deleted the tls_selfsigned_EC branch November 11, 2020 12:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tls: set selfsigned Elliptic Curve (EC) function #17

tls: set selfsigned Elliptic Curve (EC) function #17

cHuberCoffee commented Aug 26, 2020

sreimers Aug 26, 2020

cHuberCoffee Aug 26, 2020

alfredh Aug 28, 2020

cHuberCoffee Sep 10, 2020

alfredh Sep 10, 2020

alfredh commented Aug 28, 2020

cHuberCoffee commented Sep 2, 2020

cHuberCoffee commented Sep 10, 2020

alfredh commented Sep 10, 2020

alfredh commented Sep 29, 2020

alfredh commented Sep 29, 2020

cHuberCoffee commented Oct 14, 2020

alfredh commented Oct 21, 2020

alfredh commented Oct 21, 2020

alfredh commented Oct 21, 2020

alfredh Oct 21, 2020

alfredh commented Oct 21, 2020

alfredh commented Nov 2, 2020

alfredh commented Nov 5, 2020

sreimers commented Nov 9, 2020

alfredh commented Nov 9, 2020

tls: set selfsigned Elliptic Curve (EC) function #17

tls: set selfsigned Elliptic Curve (EC) function #17

Conversation

cHuberCoffee commented Aug 26, 2020

sreimers Aug 26, 2020

Choose a reason for hiding this comment

cHuberCoffee Aug 26, 2020

Choose a reason for hiding this comment

alfredh Aug 28, 2020

Choose a reason for hiding this comment

cHuberCoffee Sep 10, 2020

Choose a reason for hiding this comment

alfredh Sep 10, 2020

Choose a reason for hiding this comment

alfredh commented Aug 28, 2020

cHuberCoffee commented Sep 2, 2020

cHuberCoffee commented Sep 10, 2020

alfredh commented Sep 10, 2020

alfredh commented Sep 29, 2020

alfredh commented Sep 29, 2020

cHuberCoffee commented Oct 14, 2020

alfredh commented Oct 21, 2020

alfredh commented Oct 21, 2020

alfredh commented Oct 21, 2020

alfredh Oct 21, 2020

Choose a reason for hiding this comment

alfredh commented Oct 21, 2020

alfredh commented Nov 2, 2020

alfredh commented Nov 5, 2020

sreimers commented Nov 9, 2020

alfredh commented Nov 9, 2020