sustrik / libmill

Go-style concurrency in C
MIT License
2.73k stars 204 forks source link

TLS Support #152

Open shehzadkhawar opened 8 years ago

shehzadkhawar commented 8 years ago

Hi, I hope all are fine. I want to discuss the possibility whether we can support TLS using libmill or not. By TLS I mean Transport Layer Security and not Thread Local Storage. I want to implement TLS support into the libmill. Any ideas, any clues, any informative reference, etc if you can share, that would be helpful.

Any direction can be proposed e.g. openSSL or gnuTLS or both or any other.

Cheers, K.

skull-squadron commented 8 years ago

libressl would seem the most sensible, but it might be better to just use stunnel / socat / spiped externally to reduce add-a-million-features, kitchen-sinking that got OpenSSL and the TLS standard committee into trouble in the first place by breaking UNIX philosophy commandments 1 & 2.

shehzadkhawar commented 8 years ago

One of my use-cases go like this: Single server, Multiple Clients and communication between them should be TLS based. The stunnel and sisters does support as a wrapper integration but what about when the underlying program has to bootstrap connections dynamically rather than based on configurations as done by external tools.

Now since each connection between server and client can use libmill's facilities of coroutines to provide concurrent tcp connection functionality but what about each ssl_{connect, accept, send, recv} et al. after the libmill-based tcp{listen, accept} call. Would that make them concurrent ssl connections, or there is more to ssl context then meets the eye.

If there is some long standing issue about TLS, then why not it should be resolved. Looking into the libressl might be a good step, its also a better to gather few use cases. For me above is quite important.

johneh commented 8 years ago

I tried openssl with libmill a while back. I have uploaded it here: https://github.com/johneh/millssl

You will need to patch libmill to include a small function or write your own versions of tcpconnect, tcpaccept to return only the file descriptor.

WARNING: this is only a test, and never used/tested in production environment.

shehzadkhawar commented 8 years ago

That seems good. What was your context in which you tried it. Was it working. I am now going to interface it with some application and will comment on my experience. Also It would be good if one can assess which parts of the SSL library (any) libmill can accelerate in terms of performance or other goals. I have cloned it. Do you think if it would be a good idea to fork the libmill and interface your library directly into that..?

sustrik commented 8 years ago

Integrating this with libmill would make sense IMO. However, those having no need for ssl/tls should be able to build libmill without even having openssl installed. Maybe a configure option: "configure --enable-ssl" ?

sustrik commented 8 years ago

John, I've moved your code into libmill itself. I have changed the API so that it's consistent with the rest of libmill API, but otherwise I've left it unchanged.

To use it: ./configure --enable-ssl;make

I'll try to do some polishing. Any help would be welcome.

johneh commented 8 years ago

Great!. I'll try to take another look at the code sometime next week. Many thanks for your efforts.

raedwulf commented 8 years ago

Hi, I'm implementing a simple http2 web server with libmill + nghttp2 c library. I need TLS with ALPN support for that. I've not fully looked into the protocol yet but the API is in openssl/libressl. I'll see today if it needs to be integrated into/exposed from libmill to work or not. My preliminary results show good results from libmill with raw http2 with comparable speeds with lwan (although lwan is still faster for non-streams).

sustrik commented 8 years ago

Have a look at the tip of the repo. ssl.c is the existing state of affairs. It's a bit raw but at least the tests (tests/ssl.c) pass. Maybe you can improve on it. If you do, it would be great if you could contribute the changes back.

johneh commented 8 years ago

@sustrik, consider exposing the SSL object to allow use of the SSL_xx functions needed for ALPN support in openssl.

raedwulf commented 8 years ago

I've thought about that, but that does contaminate the namespace of libmill.h which should be avoided (I think?). I have made a concise extension to the API where you optionally specify the protocol list (null-terminated char **) supported by the server in ssllisten and sslconnect which is sufficient for the usual scenarios for ALPN. No other special functions necessary and if you don't need/want ALPN, then NULL can be specified. As it stands, if libmill(or some fork) wanted to switch SSL implementation, the API/code utilising ALPN would be unchanged with my current proposed API:

MILL_EXPORT struct mill_sslsock *mill_ssllisten_(
    struct mill_ipaddr addr,
    const char *cert_file,
    const char *key_file,
    const char *proto_list[], /* NULL or char *x[] = {"h2", "http/1.1", NULL}; */
    int backlog);
MILL_EXPORT struct mill_sslsock *mill_sslconnect_(
    struct mill_ipaddr addr,
    const char *cert_file,
    const char *key_file,
    const char *server_name, /* NULL or "www.example.com" */
    const char *proto_list[], /* NULL or char *x[] = {"h2", "http/1.1", NULL};*/
    int64_t deadline);

SSL does give the option to specify callbacks to do special protocol matching and selection but protocol behaviour supplied by open/libressl is perfectly adequate. I'll do some testing later and work out the kinks.

sustrik commented 8 years ago

How would you find out which protocol was selected?

raedwulf commented 8 years ago

Oops, I forgot to mention. Just a simple getter from struct mill_sslconn which stores the results of the callback from OpenSSL. This should be automatically set by the callback after the corresponding mill_sslconnect_ and mill_sslaccept_ calls have been made. The behaviour if ALPN is not used or negotiation has failed would be to return an empty null-terminated string. The behaviour if it succeeds is to return a single protocol string.

MILL_EXPORT const char *mill_sslproto_(
    struct mill_sslsock *s);
sustrik commented 8 years ago

Two more questions:

  1. What's server name for? (I have no experience with openssl)
  2. I assume that if multiple protocols are available, one that comes earlier in the list is used, correct?
raedwulf commented 8 years ago
  1. The server name is for Server Name Indication extension. I don't think this is strictly necessary for ALPN but it does allow the client to specify the certificate it wants from the server for servers that have a single IP but multiple domain names and a certificate per domain-name. The use-case for this is if you were to implement a libmill-based web-browser with SSL support that successfully verifies the server certificates. I have normally found all HTTP/2 implementations including both of these extensions - so I guess it is a de facto standard thing to do.
  2. Yes, that's correct. I'm using the behaviour provided by SSL_select_next_proto from the OpenSSL API. OpenSSL does have the API for a developer to implement custom behaviour so that, if wanted, a later protocol in the list can be chosen at the developer's discretion. However, as currently the only existing use case for this extension is for HTTP/2 and HTTP/1.1 compatibility, I think this feature isn't really a requirement any time soon.
    • The API supports SNI but doesn't allow multiple domains/certificates on the server currently. So it is chiefly useful for clients to verify servers.
sustrik commented 8 years ago

Ok, it seems there's a lot of optional features that may or not may be used. So, I guess, johneh@'s suggestion to expose the ssl object (or rather a wrapper) makes sense. It would also make the API extensible in the future.

raedwulf commented 8 years ago

Okay that would work too! Just to note, the client probably should have a separate context per connection to support features like multiple protocols.

sustrik commented 8 years ago

+1

sustrik commented 8 years ago

For inspiration, here's Golang's ssl context structure: https://golang.org/pkg/crypto/tls/#Config

raedwulf commented 8 years ago

API suggestions here too from libtls: https://github.com/libressl-portable/openbsd/blob/master/src/lib/libtls/tls.h Man page: http://man.openbsd.org/OpenBSD-current/man3/tls_init.3

raedwulf commented 8 years ago

@sustrik I'm starting to feel that tls support ought to be in its own library (something like a libmill-enabled libtls library - libmilltls) which is separate. What are your thoughts on this?

Some of the options are:

sustrik commented 8 years ago

In general, I would be for a separate library... However, SSL support is such a basic requirement nowadays that adding it to the core library is perfectly justified.

What about adding support for the basic use case (is there such a thing?) into the core library? We could hard code it in such a way that people interested in just using ssl without desire to understand all the options and subtleties can simply do so.

As for full-fledged access to OpenSSL features, I am not sure. Maybe separate library would be the best option.

sustrik commented 8 years ago

I've added a tutorial step (tutorial/step8.c) to show how SSL support works. However, there seems to be a problem. After starting the server (cd tutorial;./step8) and trying to connect to it via "openssl s_client -connect localhost:5555" I see data coming from the server to the client, but not the other way round.

sustrik commented 8 years ago

Ignore the previous comment. It was cause by openssl s_client using windows line endings instead of unix ones. Tutorial fixed to accomodate for win line endings.

sustrik commented 8 years ago

And here's the tutorial step dealing with TLS/SSL: http://libmill.org/tutorial.html#step8

raedwulf commented 8 years ago

Looks good so far - I'm currently working on a more complete API. Currently, the API only supports TCP whereas there's actually no restriction for supporting non-TCP connections. We should reflect that in the API by being able to pass tcpsock or mfile types to type-specific SSL/TLS connection functions.

I've done some significant global replaces to better reflect what the API does and should support. SSLv2 and SSLv3 are insecure and deprecated by everything and we should only really be using TLS which replaces it. Thus, I propose renaming all the ssl* functions to tls* .

The API isn't complete yet, but should be enough for some discussion. It is largely based on libtls-api (from the developers of libressl). The main adjustments to make it less API-heavy than libtls is to change its dozen(s) of tls_configset* functions that set a single parameter in the config to a flag mask which can be passed to one/two functions.

struct mill_tlsconn;

#define MILL_TLS_PROTO_TLSV1_0          (1 << 1)
#define MILL_TLS_PROTO_TLSV1_1          (1 << 2)
#define MILL_TLS_PROTO_TLSV1_2          (1 << 3)
#define MILL_TLS_PROTO_TLSV1 \
        (MILL_TLS_PROTO_TLSV1_0|MILL_TLS_PROTO_TLSV1_1|MILL_TLS_PROTO_TLSV1_2)

#define MILL_TLS_PROTO_ALL MILL_TLS_PROTO_TLSV1
#define MILL_TLS_PROTO_DEFAULT MILL_TLS_PROTO_TLSV1_2

#define MILL_TLS_FLAGS_EXTENSION_0      (0 << 4) /* future-proofing */
#define MILL_TLS_FLAGS_EXTENSION_1      (0 << 5) /* future-proofing */

#define MILL_TLS_PREFER_CIPHERS_CLIENT  (0 << 6)
#define MILL_TLS_PREFER_CIPHERS_SERVER  (1 << 6)
#define MILL_TLS_VERIFY_CERT            (1 << 7)
#define MILL_TLS_VERIFY_NAME            (1 << 8)
#define MILL_TLS_VERIFY_TIME            (1 << 9)
#define MILL_TLS_VERIFY \
    (MILL_TLS_VERIFY_CERT|MILL_TLS_VERIFY_NAME|MILL_TLS_VERIFY_TIME)
#define MILL_TLS_VERIFY_CLIENT          (1 << 10)
#define MILL_TLS_VERIFY_CLIENT_OPTIONAL (1 << 11)
#define MILL_TLS_CLEAR_KEYS             (1 << 12)

#define MILL_TLS_DHEPARAMS_NONE         (0 << 13)
#define MILL_TLS_DHEPARAMS_AUTO         (1 << 13)
#define MILL_TLS_DHEPARAMS_LEGACY       (2 << 13)

#define MILL_TLS_ECDHECURVE_NONE        (0 << 15)
#define MILL_TLS_ECDHECURVE_AUTO        (1 << 15)
#define MILL_TLS_ECDHECURVE_SECP192R1   (2 << 15)
#define MILL_TLS_ECDHECURVE_SECP224R1   (3 << 15)
#define MILL_TLS_ECDHECURVE_SECP224K1   (4 << 15)
#define MILL_TLS_ECDHECURVE_SECP256R1   (5 << 15)
#define MILL_TLS_ECDHECURVE_SECP256K1   (6 << 15)
#define MILL_TLS_ECDHECURVE_SECP384R1   (7 << 15)
#define MILL_TLS_ECDHECURVE_SECP521R1   (8 << 15)

#define MILL_TLS_CIPHERS_DEFAULT        (0 << 18)
#define MILL_TLS_CIPHERS_SECURE         (1 << 18)
#define MILL_TLS_CIPHERS_COMPAT         (2 << 18)
#define MILL_TLS_CIPHERS_LEGACY         (3 << 18)
#define MILL_TLS_CIPHERS_INSECURE       (4 << 18)

#define MILL_TLS_VERIFY_DEPTH_DEFAULT   (8 << 20)
#define MILL_TLS_VERIFY_DEPTH(X)        ((X-1) << 20) /* 0 not allowed */
#define MILL_TLS_VERIFY_DEPTH_MAX       (1 << 25)

MILL_EXPORT struct mill_tlsconn *mill_tlslisten_(
    struct mill_ipaddr addr,
    const char *ca_file,
    const char *cert_file,
    const char *key_file,
    const char *alpn,
    uint32_t flags,
    int backlog);
MILL_EXPORT int mill_tlsport_(
    struct mill_tlsconn *s);
MILL_EXPORT struct mill_tlsconn *mill_tlsconnect_(
    struct mill_ipaddr addr,
    const char *server_name,
    const char *alpn,
    uint32_t flags,
    int64_t deadline);
MILL_EXPORT struct mill_tlsconn *mill_tlsaccept_(
    struct mill_tlsconn *s,
    int64_t deadline);
MILL_EXPORT struct mill_ipaddr mill_tlsaddr_(
    struct mill_tlsconn *s);
MILL_EXPORT size_t mill_tlsrecv_(
    struct mill_tlsconn *s,
    void *buf,
    int len,
    int64_t deadline);
MILL_EXPORT size_t mill_tlsrecvuntil_(
    struct mill_tlsconn *s,
    void *buf,
    size_t len,
    const char *delims,
    size_t delimcount,
    int64_t deadline);
MILL_EXPORT size_t mill_tlssend_(
    struct mill_tlsconn *s,
    const void *buf,
    int len,
    int64_t deadline);
MILL_EXPORT void mill_tlsflush_(
    struct mill_tlsconn *s,
    int64_t deadline);
MILL_EXPORT void mill_tlsclose_(
    struct mill_tlsconn *s);
MILL_EXPORT int mill_tlspeercertprovided_(
    struct mill_tlsconn *c);
MILL_EXPORT int mill_tlspeercertcontainsname_(
    struct mill_tlsconn *c,
    const char *name);
MILL_EXPORT const char *mill_tlspeercerthash_(
    struct mill_tlsconn *c);
MILL_EXPORT const char *mill_tlspeercertissuer_(
    struct mill_tlsconn *c);
MILL_EXPORT const char *mill_tlspeercertsubject_(
    struct mill_tlsconn *c);
MILL_EXPORT time_t mill_tlspeercertnotbefore_(
    struct mill_tlsconn *c);
MILL_EXPORT time_t mill_tlspeercertnotafter_(
    struct mill_tlsconn *c);
MILL_EXPORT const char *mill_tlsalpnselected_(
    struct mill_tlsconn *c);
MILL_EXPORT const char *mill_tlscipher_(
    struct mill_tlsconn *c);
sustrik commented 8 years ago

Looks good. One thing I like about it is that 1:1 mapping with libtls means we can refer to the libressl documentation instead of writing our own.

raedwulf commented 8 years ago

Excellent, libtls is mainly a wrapper over OpenSSL/LibreSSL API which captures the important parts rather than backwards-compatibility to prehistoric APIs so we know we'd be capturing a good subset of functionality.

There was a contributed patch to libtls that allowed reading/writing callbacks so in theory when that gets accepted into libtls mainline, it would be trivial for any applications to integrate libtls and libmill together. However, I realised that the extra deadline parameter would be very cumbersome/ugly to handle (you'd have to modify a structure rather than pass a parameter). Also, there are a lot of distributions that still use just OpenSSL as opposed to LibreSSL, so libtls would be unavailable.

libtls has a license pretty much the same to libmill. I suggest that we can borrow code from them and acknowledge them in the AUTHORS list to speed up development significantly.

From libtls.h:

/*
 * Copyright (c) 2014 Joel Sing <jsing@openbsd.org>
 *
 * Permission to use, copy, modify, and distribute this software for any
 * purpose with or without fee is hereby granted, provided that the above
 * copyright notice and this permission notice appear in all copies.
 *
 * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
 * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
 * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
 * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
 * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
 * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
 * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 */
sustrik commented 8 years ago

+1

shehzadkhawar commented 8 years ago

Which version of openssl should we support. I have noticed that the API calls used by Johne is somewhat from v1.0.1-stable. Johne what is your comment about version. Version v1.1.0 introduced major changes e.g. automatic initialisation and deinitialisation of algos, cyphers, error strings and few others etc. In other words major simplifications and support of other functions. If you think that we should keep 1.0.1-stable then let me know. Refs: https://www.openssl.org/docs/manmaster/ssl/ https://www.openssl.org/docs/man1.0.1/ssl/ Since the API is at initial stage, we can move IMO. The question is which minimum version to support.

raedwulf commented 8 years ago

v1.0.2 is required for ALPN support so I do recommend it. Without v1.0.2, the API could be made to still work but ignores the ALPN feature.

Support for OpenSSL v1.0.2 should come fairly soon because most distributions are playing catchup as Google Chrome now only supports HTTP/2 over ALPN and disables HTTP/2 otherwise

EDIT: I guess v1.1.0 would be fine as well, but I haven't looked into the details. I assume >= 1.0.2 is good. EDIT2: A quick look shows that v1.1.0 breaks the API significantly. I would suggest going with v1.0.2 if possible right now because this has the best common set of features between libressl & openssl.

raedwulf commented 8 years ago

Just a note - I won't have time to work on this until the start of September when I'll have significantly more free time.

raedwulf commented 8 years ago

I've been working on this for the last few days and I've come to the conclusion that this is not the best investment in time. There's a few challenges integrating SSL/TLS directly into libmill:

  1. libmill has a simple, small, set of error codes that use errno to convey this information. The SSL/TLS library has a significantly larger set of errors that can occur. This could either be rectified by making the number of API functions larger (and as such the average number of errors possible per function goes down) or introducing a new set of error functions to handle SSL/TLS specific errors. In the former case, it makes the API heavier and I don't think it fully solve the issue. In the latter case, it breaks the uniformity of the API by having a separate set of functions required to handle errors.
  2. Kitchen-sink philosophy: adding this library in adds a set of potentially buggy/incomplete features because the sheer number of mishandling issues of the OpenSSL library can occur. This also adds a maintenance burden which I'm currently of the belief is unnecessary.
  3. Investment in time: libtls recently committed to their HEAD/tip of their repository to allow custom read/write functions as callbacks. Granted it's not a pure libmill API, but arguably 1. isn't either. As a library user, I think dealing with non-uniformity due to integrating libraries that do their own tasks well is better than dealing with non-uniformity due to a incomplete feature within the same library. libtls is well-documented and very simple to use compared to OpenSSL.

libmill is pretty stable, I think in the future having TLS/SSL support with an API that hasn't quite solidified would be better in dsock for libdill. OpenSSL, LibreSSL and libtls all intend the user to utilise their version of read and write which goes against the intention of libmill to deal with the I/O instead. Thus, it may be of interest of a dedicated person in the future to re-implement the SSL protocol from scratch or port an embedded tls library with a liberal license so that it doesn't carry all the additional I/O cruft into a new dsock/dtls implementation.

In the meantime, libtls is perfectly adequate to do the job required. There is a downside currently with libtls, however, libtls doesn't compile with OpenSSL straight away I believe and isn't a package in any of the major distributions. It does not require too much effort to port to OpenSSL, however. It also is not that large; sloccount reports 2468 lines of code. It would be reasonably easy to embed into any project - but maybe slightly too large for libmill? @sustrik what do you think?

@shehzadkhawar if this is still a requirement, I suggest porting to use libtls the most viable course of action and either embedding or making your own package/installing locally.

raedwulf commented 8 years ago

@shehzadkhawar Hi, an update - I'm currently working new support for TLS in dsock. It's still WIP, but you can find it here https://github.com/sustrik/dsock/pull/17