Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TLS Support #152

Open
shehzadkhawar opened this issue Jul 18, 2016 · 34 comments
Open

TLS Support #152

shehzadkhawar opened this issue Jul 18, 2016 · 34 comments

Comments

@shehzadkhawar
Copy link

shehzadkhawar commented Jul 18, 2016

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
Copy link

skull-squadron commented Jul 19, 2016

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
Copy link
Author

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
Copy link
Contributor

johneh commented Jul 20, 2016

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
Copy link
Author

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
Copy link
Owner

sustrik commented Jul 21, 2016

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
Copy link
Owner

sustrik commented Jul 22, 2016

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
Copy link
Contributor

johneh commented Jul 22, 2016

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

@raedwulf
Copy link
Contributor

raedwulf commented Jul 23, 2016

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
Copy link
Owner

sustrik commented Jul 23, 2016

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
Copy link
Contributor

johneh commented Jul 23, 2016

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

@raedwulf
Copy link
Contributor

raedwulf commented Jul 23, 2016

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
Copy link
Owner

sustrik commented Jul 24, 2016

How would you find out which protocol was selected?

@raedwulf
Copy link
Contributor

raedwulf commented Jul 24, 2016

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
Copy link
Owner

sustrik commented Jul 24, 2016

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
Copy link
Contributor

raedwulf commented Jul 24, 2016

  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
Copy link
Owner

sustrik commented Jul 24, 2016

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
Copy link
Contributor

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
Copy link
Owner

sustrik commented Jul 24, 2016

+1

@sustrik
Copy link
Owner

sustrik commented Jul 24, 2016

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

@raedwulf
Copy link
Contributor

raedwulf commented Jul 25, 2016

@raedwulf
Copy link
Contributor

raedwulf commented Jul 25, 2016

@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:

  • Thin-shim interface between libmill and OpenSSL inside libmill (what we have currently). Requires OpenSSL code to do more advanced features and thus exposing the SSL contexts. Adds OpenSSL dependency to standard libmill distribution.
  • Wrapping interface in libmill. Much more complicated and has the problems above except exposing OpenSSL interface.
  • Separate wrapping library like libtls but using libmill internally. More effort, separate library.

@sustrik
Copy link
Owner

sustrik commented Jul 26, 2016

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
Copy link
Owner

sustrik commented Jul 28, 2016

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
Copy link
Owner

sustrik commented Jul 28, 2016

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
Copy link
Owner

sustrik commented Jul 28, 2016

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

@raedwulf
Copy link
Contributor

raedwulf commented Jul 29, 2016

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_config_set_* 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
Copy link
Owner

sustrik commented Aug 1, 2016

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
Copy link
Contributor

raedwulf commented Aug 1, 2016

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 <[email protected]>
 *
 * 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
Copy link
Owner

sustrik commented Aug 1, 2016

+1

@shehzadkhawar
Copy link
Author

shehzadkhawar commented Aug 5, 2016

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
Copy link
Contributor

raedwulf commented Aug 5, 2016

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
Copy link
Contributor

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
Copy link
Contributor

raedwulf commented Sep 20, 2016

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
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants