[racket-dev] [patch] OpenSSL ECDH(E) + DHE support.

OK, here's a finalized version of my patch, that:
  - Fixes a few typos.
  - Uses Racket's I/O system to load the DH parameters.
  - Adds two functions to enable ECDH(E) + DHE
  - TLS 1.1/1.2 server/client contexts
  - Embeds reasonable defaults for ECDH(E) and DHE mode.
  - Has the standard TLS curves as defined
    https://tools.ietf.org/html/rfc4492#section-5.1.1

Are there any other things I should address?

On Tue, Feb 11, 2014 at 10:34:53AM -0500, Edward Lee wrote:
> Here's an updated patch that that:
>   - Fixes a few typos.
>   - Uses Racket's I/O system to read the Diffie-Hellman parameters.
> 
> Things that are not yet added in yet:
>   - All the other elliptic curve names used in TLS.
> 
> On Mon, Feb 10, 2014 at 08:36:02PM -0500, Edward Lee wrote:
> > On Mon, Feb 10, 2014 at 06:37:56PM -0500, Ryan Culpepper wrote:
> > > On 02/08/2014 05:52 PM, Edward Lee wrote:
> > > >Hi.
> > > >
> > > >Racket's OpenSSL bindings do not currently enable the ECDH(E) and the
> > > >DHE ciphers, which are needed for perfect forward secrecy.
> > > >
> > > >I've attached a patch that:
> > > >   - Embeds reasonable defaults for DHE mode.
> > > >   - Adds two functions, ssl-server-context-enable-dhe! and
> > > >     ssl-server-context-enable-ecdhe! that when given
> > > >     DHE/ECDHE setup arguments (for DHE, a DH parameter file path,
> > > >     for ECDHE, the name of one of the built-in OpenSSL elliptic curves
> > > >     [currently, only secp521r1])
> > > >   - (unrelated, but also useful) Adds bindings for TLS 1.1/1.2-only
> > > >     server/client contexts.
> > > >
> > > >This patch is currently a work in progress (it currently only supports
> > > >one elliptic curve name) that works well enough for what I am using it
> > > >for, but I'm interested in getting this patch upstream.
> > > 
> > > Here are a few comments:
> > > 
> > > What is SSL_CTRL_SET_ECDH_AUTO? I couldn't find it in the openssl
> > > headers, and I searched a few recent versions. It seems unused here,
> > > anyway.
> > It's not defined in OpenSSL 1.0.1, I think.  It can be used for enabling
> > ECDH(E) in OpenSSL >= 1.0.2, with OpenSSL automatically selecting the
> > curve - <https://www.openssl.org/news/changelog.html>, "Support for
> > automatic EC temporary key parameter selection..."
> > > 
> > > I think it would be better to read the DH params file into memory
> > > using Racket file operations and then use a memory BIO for
> > > PEM_read_bio_DHparams. Two reasons: the file access will then
> > > automatically go through Racket's security-guard checks, and the
> > > file read will go through Racket's IO system, so if the read blocks
> > > it won't block all Racket threads.
> > I will investigate this, and time permitting, will change my patch to
> > use Racket's IO system for loading the parameters.
> > > 
> > > In ...enable-ecdhe!, the 'case' form doesn't need 'quote'. It should
> > > be this instead:
> > > 
> > >   (case name
> > >     [(secp521r1) NID_secp521r1]
> > >     [else ___])
> > > 
> > > As it is, the function will accept 'quote as the name of a curve.
> > Thanks for pointing this out.
> > > 
> > > >What's the exact process for this?
> > > 
> > > If you have a github account, you can fork the repo there and submit
> > > a pull request. But we can certainly handle patches like this too.
> > > 
> > > Ryan
> > Great, thanks.
> > 
> > --Edward
> > _________________________
> >   Racket Developers list:
> >   http://lists.racket-lang.org/dev

> diff --git a/racket/collects/openssl/dh4096.pem b/racket/collects/openssl/dh4096.pem
> new file mode 100644
> index 0000000..1b35ad8
> --- /dev/null
> +++ b/racket/collects/openssl/dh4096.pem
> @@ -0,0 +1,18 @@
> +-----BEGIN DH PARAMETERS-----
> +MIICCAKCAgEA+hRyUsFN4VpJ1O8JLcCo/VWr19k3BCgJ4uk+d+KhehjdRqNDNyOQ
> +l/MOyQNQfWXPeGKmOmIig6Ev/nm6Nf9Z2B1h3R4hExf+zTiHnvVPeRBhjdQi81rt
> +Xeoh6TNrSBIKIHfUJWBh3va0TxxjQIs6IZOLeVNRLMqzeylWqMf49HsIXqbcokUS
> +Vt1BkvLdW48j8PPv5DsKRN3tloTxqDJGo9tKvj1Fuk74A+Xda1kNhB7KFlqMyN98
> +VETEJ6c7KpfOo30mnK30wqw3S8OtaIR/maYX72tGOno2ehFDkq3pnPtEbD2CScxc
> +alJC+EL7RPk5c/tgeTvCngvc1KZn92Y//EI7G9tPZtylj2b56sHtMftIoYJ9+ODM
> +sccD5Piz/rejE3Ome8EOOceUSCYAhXn8b3qvxVI1ddd1pED6FHRhFvLrZxFvBEM9
> +ERRMp5QqOaHJkM+Dxv8Cj6MqrCbfC4u+ZErxodzuusgDgvZiLF22uxMZbobFWyte
> +OvOzKGtwcTqO/1wV5gKkzu1ZVswVUQd5Gg8lJicwqRWyyNRczDDoG9jVDxmogKTH
> +AaqLulO7R8Ifa1SwF2DteSGVtgWEN8gDpN3RBmmPTDngyF2DHb5qmpnznwtFKdTL
> +KWbuHn491xNO25CQWMtem80uKw+pTnisBRF/454n1Jnhub144YRBoN8CAQI=
> +-----END DH PARAMETERS-----
> +
> +These are the 4096 bit DH parameters from "Assigned Number for SKIP Protocols"
> +(http://www.skip-vpn.org/spec/numbers.html).
> +See there for how they were generated.
> +Note that g is not a generator, but this is not a problem since p is a safe prime.
> diff --git a/racket/collects/openssl/mzssl.rkt b/racket/collects/openssl/mzssl.rkt
> index 2f16517..e6ffedb 100644
> --- a/racket/collects/openssl/mzssl.rkt
> +++ b/racket/collects/openssl/mzssl.rkt
> @@ -34,6 +34,7 @@ TO DO:
>           racket/tcp
>           racket/string
>           racket/lazy-require
> +         racket/runtime-path
>           "libcrypto.rkt"
>           "libssl.rkt")
>  (lazy-require
> @@ -41,7 +42,9 @@ TO DO:
>   ["private/macosx.rkt" (load-macosx-keychain)])
>  
>  (define protocol-symbol/c
> -  (or/c 'sslv2-or-v3 'sslv2 'sslv3 'tls))
> +  (or/c 'sslv2-or-v3 'sslv2 'sslv3 'tls 'tls11 'tls12))
> +(define curves/c
> +  (or/c 'secp521r1))
>  
>  (define verify-source/c
>    (or/c path-string?
> @@ -50,6 +53,7 @@ TO DO:
>          (list/c 'macosx-keychain path-string?)))
>  
>  (provide
> + ssl-dh-param-path
>   (contract-out
>    [ssl-available? boolean?]
>    [ssl-load-fail-reason (or/c #f string?)]
> @@ -59,6 +63,10 @@ TO DO:
>     (c-> ssl-client-context?)]
>    [ssl-make-server-context
>     (->* () (protocol-symbol/c) ssl-server-context?)]
> +  [ssl-server-context-enable-dhe!
> +   (->* (ssl-server-context?) (path-string?) void?)]
> +  [ssl-server-context-enable-ecdhe!
> +   (->* (ssl-server-context?) (curves/c) void?)]
>    [ssl-client-context?
>     (c-> any/c boolean?)]
>    [ssl-server-context?
> @@ -185,6 +193,8 @@ TO DO:
>  (define-cpointer-type _X509*)
>  (define-cpointer-type _ASN1_STRING*)
>  (define-cpointer-type _STACK*)
> +(define-cpointer-type _DH*)
> +(define-cpointer-type _EC_KEY*)
>  (define-cstruct _GENERAL_NAME ([type _int] [d _ASN1_STRING*]))
>  
>  (define-ssl SSLv2_client_method (_fun -> _SSL_METHOD*))
> @@ -195,9 +205,19 @@ TO DO:
>  (define-ssl SSLv23_server_method (_fun -> _SSL_METHOD*))
>  (define-ssl TLSv1_client_method (_fun -> _SSL_METHOD*))
>  (define-ssl TLSv1_server_method (_fun -> _SSL_METHOD*))
> +(define-ssl TLSv1_1_client_method (_fun -> _SSL_METHOD*))
> +(define-ssl TLSv1_1_server_method (_fun -> _SSL_METHOD*))
> +(define-ssl TLSv1_2_client_method (_fun -> _SSL_METHOD*))
> +(define-ssl TLSv1_2_server_method (_fun -> _SSL_METHOD*))
> +
> +(define-crypto DH_free (_fun _DH* -> _void) #:wrap (deallocator))
> +(define-crypto EC_KEY_free (_fun _EC_KEY* -> _void) #:wrap (deallocator))
> +
> +(define-crypto EC_KEY_new_by_curve_name (_fun _int -> _EC_KEY*) #:wrap (allocator EC_KEY_free))
>  
>  (define-crypto BIO_s_mem (_fun -> _BIO_METHOD*))
>  (define-crypto BIO_new (_fun _BIO_METHOD* -> _BIO*/null))
> +(define-crypto BIO_new_mem_buf (_fun _pointer _int -> _BIO*))
>  (define-crypto BIO_free (_fun _BIO* -> _void))
>  
>  (define-crypto BIO_read (_fun _BIO* _bytes _int -> _int))
> @@ -259,6 +279,7 @@ TO DO:
>  (define-ssl SSL_load_error_strings (_fun -> _void))
>  
>  (define-crypto GENERAL_NAME_free _fpointer)
> +(define-crypto PEM_read_bio_DHparams (_fun _BIO* _pointer _pointer _pointer -> _DH*) #:wrap (allocator DH_free))
>  (define-crypto ASN1_STRING_length (_fun _ASN1_STRING* -> _int))
>  (define-crypto ASN1_STRING_data (_fun _ASN1_STRING* -> _pointer))
>  (define-crypto X509_NAME_get_index_by_NID (_fun _X509_NAME* _int _int -> _int))
> @@ -331,8 +352,20 @@ TO DO:
>  (define NID_commonName 13)
>  (define GEN_DNS 2)
>  
> +(define SSL_CTRL_SET_ECDH_AUTO 94)
> +(define SSL_CTRL_OPTIONS 32)
> +(define SSL_CTRL_SET_TMP_DH 3)
> +(define SSL_CTRL_SET_TMP_ECDH 4)
> +
> +(define SSL_OP_SINGLE_ECDH_USE #x00080000)
> +(define SSL_OP_SINGLE_DH_USE #x00100000)
> +
> +(define NID_secp521r1 716)
> +
>  (define-mzscheme scheme_make_custodian (_fun _pointer -> _scheme))
>  
> +(define-runtime-path ssl-dh-param-path "dh4096.pem")
> +
>  ;; Make this bigger than 4096 to accommodate at least
>  ;; 4096 of unencrypted data
>  (define BUFFER-SIZE 8000)
> @@ -467,6 +500,10 @@ TO DO:
>        (if client? SSLv3_client_method SSLv3_server_method)]
>       [(tls)
>        (if client? TLSv1_client_method TLSv1_server_method)]
> +     [(tls11)
> +      (if client? TLSv1_1_client_method TLSv1_1_server_method)]
> +     [(tls12)
> +      (if client? TLSv1_2_client_method TLSv1_2_server_method)]
>       [else
>        (error 'encrypt->method "internal error, unknown encrypt: ~e" e)])))
>  
> @@ -515,6 +552,34 @@ TO DO:
>  (define (ssl-seal-context! mzctx)
>    (set-ssl-context-sealed?! mzctx #t))
>  
> +(define (ssl-server-context-enable-ecdhe! context [name 'secp521r1])
> +  (define (symbol->nid name)
> +    (case name
> +      [(secp521r1) NID_secp521r1]
> +      [else NID_secp521r1]))
> +  (define ctx (extract-ctx 'ssl-server-context-enable-ecdhe! #t context))
> +  (define key (EC_KEY_new_by_curve_name (symbol->nid name)))
> +  (check-valid key 'ssl-server-context-enable-ecdhe! "Could not enable ECDH(E)")
> +  (unless (= 1 (SSL_CTX_ctrl ctx SSL_CTRL_SET_TMP_ECDH 0 key))
> +    (error 'ssl-server-context-enable-ecdhe! "Could not enable ECDH(E)"))
> +  (SSL_CTX_ctrl ctx SSL_CTRL_OPTIONS SSL_OP_SINGLE_ECDH_USE #f)
> +  (void))
> +
> +(define (ssl-server-context-enable-dhe! context [path ssl-dh-param-path])
> +  (define params (call-with-input-file path port->bytes))
> +  (define params-bio (BIO_new_mem_buf params (bytes-length params)))
> +  (check-valid params-bio 'ssl-server-context-enable-dhe! "Diffie-Hellman parameters")
> +  (with-failure
> +    (lambda ()
> +      (BIO_free params-bio))
> +    (define ctx (extract-ctx 'ssl-server-context-enable-dhe! #t context))
> +    (define dh (PEM_read_bio_DHparams params-bio #f #f #f))
> +    (check-valid dh 'ssl-server-context-enable-dhe "Diffie-Hellman parameters")
> +    (unless (= 1 (SSL_CTX_ctrl ctx SSL_CTRL_SET_TMP_DH 0 dh))
> +      (error 'ssl-server-context-enable-dhe "Could not enable DHE"))
> +    (SSL_CTX_ctrl ctx SSL_CTRL_OPTIONS SSL_OP_SINGLE_DH_USE #f)
> +    (void)))
> +
>  (define (ssl-load-... who load-it ssl-context-or-listener pathname
>                        #:try? [try? #f])
>    (let ([ctx (get-context/listener who ssl-context-or-listener

> _________________________
>   Racket Developers list:
>   http://lists.racket-lang.org/dev