Commit Graph

323 Commits

Author SHA1 Message Date
Willy Tarreau
94ff03af84 BUG/MEDIUM: ssl: avoid double free when releasing bind_confs
ssl_sock functions don't mark pointers as NULL after freeing them. So
if a "bind" line specifies some SSL settings without the "ssl" keyword,
they will get freed at the end of check_config_validity(), then freed
a second time on exit. Simply mark the pointers as NULL to fix this.
This fix needs to be backported to 1.7 and 1.6.
2016-12-22 22:07:36 +01:00
Willy Tarreau
30fd4bd844 BUG/MEDIUM: ssl: properly reset the reused_sess during a forced handshake
We have a bug when SSL reuse is disabled on the server side : we reset
the context but do not set it to NULL, causing a multiple free of the
same entry. It seems like this bug cannot appear as-is with the current
code (or the conditions to get it are not obvious) but it did definitely
strike when trying to fix another bug with the SNI which forced a new
handshake.

This fix should be backported to 1.7, 1.6 and 1.5.
2016-12-22 21:54:21 +01:00
Willy Tarreau
14e36a101c MEDIUM: cfgparse: move ssl-dh-param-file parsing to ssl_sock
This one was missing an arg count check which was added in the operation.
2016-12-21 23:39:26 +01:00
Willy Tarreau
f22e9683e9 MINOR: cfgparse: move parsing of ssl-default-{bind,server}-ciphers to ssl_sock
These ones are pretty similar, just an strdup. Contrary to ca-base
and crt-base they support being changed.
2016-12-21 23:39:26 +01:00
Willy Tarreau
0bea58d641 MEDIUM: cfgparse: move maxsslconn parsing to ssl_sock
This one simply reuses the existing integer parser. It implicitly
adds a control against negative numbers.
2016-12-21 23:39:26 +01:00
Willy Tarreau
9ceda384e9 MEDIUM: cfgparse: move all tune.ssl.* keywords to ssl_sock
The following keywords were still parsed in cfgparse and were moved
to ssl_sock to remove some #ifdefs :

"tune.ssl.cachesize", "tune.ssl.default-dh-param", "tune.ssl.force-private-cache",
"tune.ssl.lifetime", "tune.ssl.maxrecord", "tune.ssl.ssl-ctx-cache-size".

It's worth mentionning that some of them used to have incorrect sign
checks possibly resulting in some negative values being used. All of
them are now checked for being positive.
2016-12-21 23:39:26 +01:00
Willy Tarreau
8c3b0fd273 MINOR: cfgparse: move parsing of "ca-base" and "crt-base" to ssl_sock
This removes 2 #ifdefs and makes the code much cleaner. The controls
are still there and the two parsers have been merged into a single
function ssl_parse_global_ca_crt_base().

It's worth noting that there's still a check to prevent a change when
the value was already specified. This test seems useless and possibly
counter-productive, it may have to be revisited later, but for now it
was implemented identically.
2016-12-21 23:39:26 +01:00
Willy Tarreau
c2c0b61274 CLEANUP: ssl: use the build options list to report the SSL details
This removes 7 #ifdef from haproxy.c. The message indicating that
openssl is *not* enabled is not there anymore.
2016-12-21 21:30:54 +01:00
Willy Tarreau
f5f26e824a MINOR: appctx/cli: remove the "tlskeys" entry from the appctx union
This one now migrates to the general purpose cli.p0 for the ref pointer,
cli.i0 for the dump_all flag and cli.i1 for the dump_keys_index. A few
comments were added.

The applet.h file doesn't depend on openssl anymore. It's worth noting
that the previous dependency was accidental and only used to work because
all files including this one used to have openssl included prior to
loading this file.
2016-12-16 19:40:14 +01:00
Luca Pizzamiglio
578b169dcb BUILD/MEDIUM: Fixing the build using LibreSSL
Fixing the build using LibreSSL as OpenSSL implementation.
Currently, LibreSSL 2.4.4 provides the same API of OpenSSL 1.0.1x,
but it redefine the OpenSSL version number as 2.0.x, breaking all
checks with OpenSSL 1.1.x.
The patch solves the issue checking the definition of the symbol
LIBRESSL_VERSION_NUMBER when Openssl 1.1.x features are requested.
2016-12-12 22:57:04 +01:00
Willy Tarreau
3067bfa815 BUG/MEDIUM: cli: fix "show stat resolvers" and "show tls-keys"
The recent CLI reorganization managed to break these two commands
by having their parser return 1 (indicating an end of processing)
instead of 0 to indicate new calls to the io handler were needed.

Namely the faulty commits are :
  69e9644 ("REORG: cli: move show stat resolvers to dns.c")
  32af203 ("REORG: cli: move ssl CLI functions to ssl_sock.c")

The fix is trivial and there is no other loss of functionality. Thanks
to Dragan Dosen for reporting the issue and the faulty commits. The
backport is needed in 1.7.
2016-12-05 14:53:37 +01:00
Willy Tarreau
30e5e18bbb CLEANUP: cli: remove assignments to st0 and st2 in keyword parsers
Now it's not needed anymore to set STAT_ST_INIT nor CLI_ST_CALLBACK
in the parsers, remove it in the various places.
2016-11-24 16:59:28 +01:00
Willy Tarreau
3b6e547be8 CLEANUP: cli: rename STAT_CLI_* to CLI_ST_*
These are in CLI states, not stats states anymore. STAT_CLI_O_CUSTOM
was more appropriately renamed CLI_ST_CALLBACK.
2016-11-24 16:59:28 +01:00
William Lallemand
32af203b75 REORG: cli: move ssl CLI functions to ssl_sock.c
Move ssl CLI functions to ssl_sock.c and use the cli keyword API to
register ssl actions on the CLI.
2016-11-24 16:59:27 +01:00
Willy Tarreau
8e0bb0ae16 MINOR: connection: add names for transport and data layers
This makes debugging easier and avoids having to put ugly checks
against certain well-known internal struct pointers.
2016-11-24 16:58:12 +01:00
Bertrand Jacquin
ff13c06a17 CLEANUP: ssl: Fix bind keywords name in comments
Along with a whitespace cleanup and a grammar typo
2016-11-14 18:15:20 +01:00
Bertrand Jacquin
5424ee08de BUG/MINOR: ssl: Print correct filename when error occurs reading OCSP
When Multi-Cert bundle are used, error is throwned regarding certificate
filename without including certifcate type extension.
2016-11-14 18:15:20 +01:00
Bertrand Jacquin
3342309572 BUG/MEDIUM: ssl: Store certificate filename in a variable
Before this change, trash is being used to create certificate filename
to read in care Mutli-Cert are in used. But then ssl_sock_load_ocsp()
modify trash leading to potential wrong information given in later error
message.

This also blocks any further use of certificate filename for other
usage, like ongoing patch to support Certificate Transparency handling
in Multi-Cert bundle.
2016-11-14 18:15:20 +01:00
Dirkjan Bussink
1866d6d8f1 MEDIUM: ssl: Add support for OpenSSL 1.1.0
In the last release a lot of the structures have become opaque for an
end user. This means the code using these needs to be changed to use the
proper functions to interact with these structures instead of trying to
manipulate them directly.

This does not fix any deprecations yet that are part of 1.1.0, it only
ensures that it can be compiled against that version and is still
compatible with older ones.

[wt: openssl-0.9.8 doesn't build with it, there are conflicts on certain
     function prototypes which we declare as inline here and which are
     defined differently there. But openssl-0.9.8 is not supported anymore
     so probably it's OK to go without it for now and we'll see later if
     some users still need it. Emeric has reviewed this change and didn't
     spot anything obvious which requires special care. Let's try it for
     real now]
2016-11-08 20:54:41 +01:00
Thierry FOURNIER / OZON.IO
07c3d78c2c BUG/MINOR: ssl: prevent multiple entries for the same certificate
Today, the certificate are indexed int he SNI tree using their CN and the
list of thier AltNames. So, Some certificates have the same names in the
CN and one of the AltNames entries.

Typically Let's Encrypt duplicate the the DNS name in the CN and the
AltName.

This patch prevents the creation of identical entries in the trees. It
checks the same DNS name and the same SSL context.

If the same certificate is registered two time it will be duplicated.

This patch should be backported in the 1.6 and 1.5 version.
2016-10-24 19:13:12 +02:00
Thierry FOURNIER / OZON.IO
7a3bd3b9dc BUG/MINOR: ssl: Check malloc return code
If malloc() can't allocate memory and return NULL, a segfaut will raises.

This patch should be backported in the 1.6 and 1.5 version.
2016-10-24 19:13:12 +02:00
Thierry FOURNIER / OZON.IO
d44ea3f77c BUILD/CLEANUP: ssl: Check BIO_reset() return code
The BIO_reset function can fails, and the error is not processed.
This patch just take in account the return code of the BIO_reset()
function.
2016-10-24 19:13:12 +02:00
Thierry FOURNIER / OZON.IO
8b068c2993 MINOR: ssl: add debug traces
Add some debug trace when haproxy is configured in debug & verbose mode.
This is useful for openssl tests. Typically, the error "SSL handshake
failure" can be caused by a lot of protocol error. This patch details
the encountered error. For exemple:

   OpenSSL error 0x1408a0c1: ssl3_get_client_hello: no shared cipher

Note that my compilator (gcc-4.7) refuse to considers the function
ssl_sock_dump_errors() as inline. The condition "if" ensure that the
content of the function is not executed in normal case. It should be
a pity to call a function just for testing its execution condition, so
I use the macro "forceinline".
2016-10-24 19:13:12 +02:00
Remi Gacogne
c7e12637df BUG/MINOR: ssl: fix potential memory leak in ssl_sock_load_dh_params()
Roberto Guimaraes reported that Valgrind complains about a leak
in ssl_get_dh_1024().
This is caused caused by an oversight in ssl_sock_load_dh_params(),
where local_dh_1024 is always replaced by a new DH object even if
it already holds one. This patch simply checks whether local_dh_1024
is NULL before calling ssl_get_dh_1024().
2016-07-12 11:48:06 +02:00
Willy Tarreau
90fd35c3a7 Revert "BUG/MINOR: ssl: fix potential memory leak in ssl_sock_load_dh_params()"
This reverts commit 0ea4c23ca7.

Certain very simple confs randomly segfault upon startup with openssl 1.0.2
with this patch, which seems to indicate a use after free. Better drop it
and let valgrind complain about the potential leak.

Also it's worth noting that the man page for SSL_CTX_set_tmp_dh() makes no
mention about whether or not the element should be freed, and the example
provided does not use it either.

This fix should be backported to 1.6 and 1.5 where the patch was just
included.
2016-06-30 20:00:19 +02:00
mildis
16aa0153b5 BUG/MINOR: ssl: close ssl key file on error
Explicitly close the FILE opened to read the ssl key file when parsing
fails to find a valid key.

This fix needs to be backported to 1.6.
2016-06-24 15:27:30 +02:00
Willy Tarreau
24b892f324 BUILD: ssl: fix typo causing a build failure in the multicert patch
I just noticed that SSL wouldn't build anymore since this afternoon's patch :

src/ssl_sock.c: In function 'ssl_sock_load_multi_cert':
src/ssl_sock.c:1982:26: warning: left-hand operand of comma expression has no effect [-Wunused-value]
    for (i = 0; i < fcount, i++)
                          ^
src/ssl_sock.c:1982:31: error: expected ';' before ')' token
    for (i = 0; i < fcount, i++)
                               ^
Makefile:791: recipe for target 'src/ssl_sock.o' failed
make: *** [src/ssl_sock.o] Error 1
2016-06-20 23:02:46 +02:00
Emmanuel Hocdet
5e0e6e409b MINOR: ssl: crt-list parsing factor
LINESIZE and MAX_LINE_ARGS are too low for parsing crt-list.
2016-06-20 17:29:56 +02:00
Emmanuel Hocdet
d294aea605 MEDIUM: ssl: support SNI filters with multicerts
SNI filters used to be ignored with multicerts (eg: those providing
ECDSA and RSA at the same time). This patch makes them work like
other certs.

Note: most of the changes in this patch are due to an extra level of
      indent, read it with "git show -b".
2016-06-20 17:15:17 +02:00
Roberto Guimaraes
0ea4c23ca7 BUG/MINOR: ssl: fix potential memory leak in ssl_sock_load_dh_params()
Valgrind reports that the memory allocated in ssl_get_dh_1024() was leaking. Upon further inspection of openssl code, it seems that SSL_CTX_set_tmp_dh makes a copy of the data, so calling DH_free afterwards makes sense.
2016-06-12 13:12:32 +02:00
Willy Tarreau
bb137a8af7 BUG/MEDIUM: ssl: rewind the BIO when reading certificates
Emeric found that some certificate files that were valid with the old method
(the one with the explicit name involving SSL_CTX_use_PrivateKey_file()) do
not work anymore with the new one (the one trying to load multiple cert types
using PEM_read_bio_PrivateKey()). With the last one, the private key couldn't
be loaded.

The difference was related to the ordering in the PEM file was different. The
old method would always work. The new method only works if the private key is
at the top, or if it appears as an "EC" private key. The cause in fact is that
we never rewind the BIO between the various calls. So this patch moves the
loading of the private key as the first step, then it rewinds the BIO, and
then it loads the cert and the chain. With this everything works.

No backport is needed, this issue came with the recent addition of the
multi-cert support.
2016-04-06 19:02:38 +02:00
Vincent Bernat
02779b6263 CLEANUP: uniformize last argument of malloc/calloc
Instead of repeating the type of the LHS argument (sizeof(struct ...))
in calls to malloc/calloc, we directly use the pointer
name (sizeof(*...)). The following Coccinelle patch was used:

@@
type T;
T *x;
@@

  x = malloc(
- sizeof(T)
+ sizeof(*x)
  )

@@
type T;
T *x;
@@

  x = calloc(1,
- sizeof(T)
+ sizeof(*x)
  )

When the LHS is not just a variable name, no change is made. Moreover,
the following patch was used to ensure that "1" is consistently used as
a first argument of calloc, not the last one:

@@
@@

  calloc(
+ 1,
  ...
- ,1
  )
2016-04-03 14:17:42 +02:00
Vincent Bernat
3c2f2f207f CLEANUP: remove unneeded casts
In C89, "void *" is automatically promoted to any pointer type. Casting
the result of malloc/calloc to the type of the LHS variable is therefore
unneeded.

Most of this patch was built using this Coccinelle patch:

@@
type T;
@@

- (T *)
  (\(lua_touserdata\|malloc\|calloc\|SSL_get_app_data\|hlua_checkudata\|lua_newuserdata\)(...))

@@
type T;
T *x;
void *data;
@@

  x =
- (T *)
  data

@@
type T;
T *x;
T *data;
@@

  x =
- (T *)
  data

Unfortunately, either Coccinelle or I is too limited to detect situation
where a complex RHS expression is of type "void *" and therefore casting
is not needed. Those cases were manually examined and corrected.
2016-04-03 14:17:42 +02:00
Nenad Merdanovic
1789115a52 BUG/MEDIUM: Fix RFC5077 resumption when more than TLS_TICKETS_NO are present
Olivier Doucet reported the issue on the ML and tested that when using
more than TLS_TICKETS_NO keys in the file, the CPU usage is much higeher
than expected.

Lukas Tribus then provided a test case which showed that resumption doesn't
work at all in that case.

This fix needs to be backported to 1.6.

Signed-off-by: Nenad Merdanovic <nmerdan@anine.io>
2016-03-29 16:03:37 +02:00
Willy Tarreau
e237fe1172 BUG/MINOR: ssl: fix usage of the various sample fetch functions
Technically speaking, many SSL sample fetch functions act on the
connection and depend on USE_L5CLI on the client side, which means
they're usable as soon as a handshake is completed on a connection.
This means that the test consisting in refusing to call them when
the stream is NULL will prevent them from working when we implement
the tcp-request session ruleset. Better fix this now. The fix consists
in using smp->sess->origin when they're called for the front connection,
and smp->strm->si[1].end when called for the back connection.

There is currently no known side effect for this issue, though it would
better be backported into 1.6 so that the code base remains consistend.
2016-03-10 17:28:05 +01:00
Willy Tarreau
be508f1580 BUG/MAJOR: samples: check smp->strm before using it
Since commit 6879ad3 ("MEDIUM: sample: fill the struct sample with the
session, proxy and stream pointers") merged in 1.6-dev2, the sample
contains the pointer to the stream and sample fetch functions as well
as converters use it heavily.

The problem is that earlier commit 87b0966 ("REORG/MAJOR: session:
rename the "session" entity to "stream"") had split the session and
stream resulting in the possibility for smp->strm to be NULL before
the stream was initialized. This is what happens in tcp-request
connection rulesets, as discovered by Baptiste.

The sample fetch functions must now check that smp->strm is valid
before using it. An alternative could consist in using a dummy stream
with nothing in it to avoid some checks but it would only result in
deferring them to the next step anyway, and making it harder to detect
that a stream is valid or the dummy one.

There is still an issue with variables which requires a complete
independant fix. They use strm->sess to find the session with strm
possibly NULL and passed as an argument. All call places indirectly
use smp->strm to build strm. So the problem is there but the API needs
to be changed to remove this duplicate argument that makes it much
harder to know what pointer to use.

This fix must be backported to 1.6, as well as the next one fixing
variables.
2016-03-10 16:42:58 +01:00
Cyril Bonté
23d19d669b BUG/MEDIUM: cfgparse: wrong argument offset after parsing server "sni" keyword
Owen Marshall reported an issue depending on the server keywords order in the
configuration.

Working line :
  server dev1 <ip>:<port> check inter 5000 ssl verify none sni req.hdr(Host)

Non working line :
  server dev1 <ip>:<port> check inter 5000 ssl sni req.hdr(Host) verify none

Indeed, both parse_server() and srv_parse_sni() modified the current argument
offset at the same time. To fix the issue, srv_parse_sni() can work on a local
copy ot the offset, leaving parse_server() responsible of the actual value.

This fix must be backported to 1.6.
2016-03-07 23:47:05 +01:00
Willy Tarreau
3724da1261 BUG/MEDIUM: ssl: fix off-by-one in NPN list allocation
After seeing previous ALPN fix, I suspected that NPN code was wrong
as well, and indeed it was since ALPN was copied from it. This fix
must be backported into 1.6 and 1.5.
2016-02-12 17:11:12 +01:00
Marcoen Hirschberg
bef6091cff BUG/MEDIUM: ssl: fix off-by-one in ALPN list allocation
The first time I tried it (1.6.3) I got a segmentation fault :(

After some investigation with gdb and valgrind I found the
problem. memcpy() copies past an allocated buffer in
"bind_parse_alpn". This patch fixes it.

[wt: this fix must be backported into 1.6 and 1.5]
2016-02-12 17:10:52 +01:00
Christopher Faulet
635c0adec2 BUG/MINOR: ssl: Be sure to use unique serial for regenerated certificates
The serial number for a generated certificate was computed using the requested
servername, without any variable/random part. It is not a problem from the
moment it is not regenerated.

But if the cache is disabled or when the certificate is evicted from the cache,
we may need to regenerate it. It is important to not reuse the same serial
number for the new certificate. Else clients (especially browsers) trigger a
warning because 2 certificates issued by the same CA have the same serial
number.

So now, the serial is a static variable initialized with now_ms (internal date
in milliseconds) and incremented at each new certificate generation.

(Ref MPS-2031)
2016-02-09 09:04:53 +01:00
Willy Tarreau
30da7ad809 BUILD: ssl: set SSL_SOCK_NUM_KEYTYPES with openssl < 1.0.2
Last patch unfortunately broke build with openssl older than 1.0.2.
Let's just define a single key type in this case.
2015-12-14 11:28:33 +01:00
yanbzhu
be2774dd35 MEDIUM: ssl: Added support for Multi-Cert OCSP Stapling
Enabled loading of OCSP staple responses (.ocsp files) when processing a
bundled certificate with multiple keytypes.
2015-12-14 11:22:29 +01:00
yanbzhu
63ea846399 MEDIUM: ssl: Added multi cert support for loading crt directories
Loading of multiple certs into shared contexts is now supported if a user
specifies a directory instead of a cert file.
2015-12-14 11:22:29 +01:00
yanbzhu
1b04e5b0e0 MINOR: ssl: Added multi cert support for crt-list config keyword
Added support for loading mutiple certs into shared contexts when they
are specified in a crt-list

Note that it's not practical to support SNI filters with multicerts, so
any SNI filters that's provided to the crt-list is ignored if a
multi-cert opertion is used.
2015-12-14 11:22:29 +01:00
yanbzhu
08ce6ab0c9 MEDIUM: ssl: Added support for creating SSL_CTX with multiple certs
Added ability for users to specify multiple certificates that all relate
a single server. Users do this by specifying certificate "cert_name.pem"
but having "cert_name.pem.rsa", "cert_name.pem.dsa" and/or
"cert_name.pem.ecdsa" in the directory.

HAProxy will now intelligently search for those 3 files and try combine
them into as few SSL_CTX's as possible based on CN/SAN. This will allow
HAProxy to support multiple ciphersuite key algorithms off a single
SSL_CTX.

This change integrates into the existing architecture of SNI lookup and
multiple SNI's can point to the same SSL_CTX, which can support multiple
key_types.
2015-12-14 11:22:29 +01:00
yanbzhu
488a4d2e75 MINOR: ssl: Added cert_key_and_chain struct
Added cert_key_and_chain struct to ssl. This struct will store the
contents of a crt path (from the config file) into memory. This will
allow us to use the data stored in memory instead of reading the file
multiple times.

This will be used to support a later commit to load multiple pkeys/certs
into a single SSL_CTX
2015-12-14 11:22:29 +01:00
Daniel Jakots
54ffb918cb BUILD: check for libressl to be able to build against it
[wt: might be worth backporting it to 1.6]
2015-11-08 07:28:02 +01:00
Christopher Faulet
e7db21693f BUILD: ssl: fix build error introduced in commit 7969a3 with OpenSSL < 1.0.0
The function 'EVP_PKEY_get_default_digest_nid()' was introduced in OpenSSL
1.0.0. So for older version of OpenSSL, compiled with the SNI support, the
HAProxy compilation fails with the following error:

src/ssl_sock.c: In function 'ssl_sock_do_create_cert':
src/ssl_sock.c:1096:7: warning: implicit declaration of function 'EVP_PKEY_get_default_digest_nid'
   if (EVP_PKEY_get_default_digest_nid(capkey, &nid) <= 0)
[...]
src/ssl_sock.c:1096: undefined reference to `EVP_PKEY_get_default_digest_nid'
collect2: error: ld returned 1 exit status
Makefile:760: recipe for target 'haproxy' failed
make: *** [haproxy] Error 1

So we must add a #ifdef to check the OpenSSL version (>= 1.0.0) to use this
function. It is used to get default signature digest associated to the private
key used to sign generated X509 certificates. It is called when the private key
differs than EVP_PKEY_RSA, EVP_PKEY_DSA and EVP_PKEY_EC. It should be enough for
most of cases.
2015-10-22 13:32:34 +02:00
Willy Tarreau
2f63ef4d1c BUG/MAJOR: ssl: free the generated SSL_CTX if the LRU cache is disabled
Kim Seri reported that haproxy 1.6.0 crashes after a few requests
when a bind line has SSL enabled with more than one certificate. This
was caused by an insufficient condition to free generated certs during
ssl_sock_close() which can also catch other certs.

Christopher Faulet analysed the situation like this :

-------
First the LRU tree is only initialized when the SSL certs generation is
configured on a bind line. So, in the most of cases, it is NULL (it is
not the same thing than empty).
When the SSL certs generation is used, if the cache is not NULL, a such
certificate is pushed in the cache and there is no need to release it
when the connection is closed.
But it can be disabled in the configuration. So in that case, we must
free the generated certificate when the connection is closed.

Then here, we have really a bug. Here is the buggy part:

3125)      if (conn->xprt_ctx) {
3126) #ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
3127)              if (!ssl_ctx_lru_tree && objt_listener(conn->target)) {
3128)                      SSL_CTX *ctx = SSL_get_SSL_CTX(conn->xprt_ctx);
3129)                      if (ctx != 3130)
 SSL_CTX_free(ctx);
3131)              }
3133)              SSL_free(conn->xprt_ctx);
3134)              conn->xprt_ctx = NULL;
3135)              sslconns--;
3136)      }

The check on the line 3127 is not enough to determine if this is a
generated certificate or not. Because ssl_ctx_lru_tree is NULL,
generated certificates, if any, must be freed. But here ctx should also
be compared to all SNI certificates and not only to default_ctx. Because
of this bug, when a SNI certificate is used for a connection, it is
erroneously freed when this connection is closed.
-------

Christopher provided this reliable reproducer :

----------
global
    tune.ssl.default-dh-param   2048
    daemon

listen ssl_server
    mode tcp
    bind 127.0.0.1:4443 ssl crt srv1.test.com.pem crt srv2.test.com.pem

    timeout connect 5000
    timeout client  30000
    timeout server  30000

    server srv A.B.C.D:80

You just need to generate 2 SSL certificates with 2 CN (here
srv1.test.com and srv2.test.com).

Then, by doing SSL requests with the first CN, there is no problem. But
with the second CN, it should segfault on the 2nd request.

openssl s_client -connect 127.0.0.1:4443 -servername srv1.test.com // OK
openssl s_client -connect 127.0.0.1:4443 -servername srv1.test.com // OK

But,

openssl s_client -connect 127.0.0.1:4443 -servername srv2.test.com // OK
openssl s_client -connect 127.0.0.1:4443 -servername srv2.test.com // KO
-----------

A long discussion led to the following proposal which this patch implements :

- the cert is generated. It gets a refcount = 1.
- we assign it to the SSL. Its refcount becomes two.
- we try to insert it into the tree. The tree will handle its freeing
  using SSL_CTX_free() during eviction.
- if we can't insert into the tree because the tree is disabled, then
  we have to call SSL_CTX_free() ourselves, then we'd rather do it
  immediately. It will more closely mimmick the case where the cert
  is added to the tree and immediately evicted by concurrent activity
  on the cache.
- we never have to call SSL_CTX_free() during ssl_sock_close() because
  the SSL session only relies on openssl doing the right thing based on
  the refcount only.
- thus we never need to know how the cert was created since the
  SSL_CTX_free() is either guaranteed or already done for generated
  certs, and this protects other ones against any accidental call to
  SSL_CTX_free() without having to track where the cert comes from.

This patch also reduces the inter-dependence between the LRU tree and
the SSL stack, so it should cause less sweating to migrate to threads
later.

This bug is specific to 1.6.0, as it was introduced after dev7 by
this fix :

   d2cab92 ("BUG/MINOR: ssl: fix management of the cache where forged certificates are stored")

Thus a backport to 1.6 is required, but not to 1.5.
2015-10-20 15:29:01 +02:00
Christopher Faulet
85b5a1a781 MINOR: ssl: Add callbacks to set DH/ECDH params for generated certificates
Now, A callback is defined for generated certificates to set DH parameters for
ephemeral key exchange when required.
In same way, when possible, we also defined Elliptic Curve DH (ECDH) parameters.
2015-10-09 12:13:17 +02:00
Christopher Faulet
7969a33a01 MINOR: ssl: Add support for EC for the CA used to sign generated certificates
This is done by adding EVP_PKEY_EC type in supported types for the CA private
key when we get the message digest used to sign a generated X509 certificate.
So now, we support DSA, RSA and EC private keys.

And to be sure, when the type of the private key is not directly supported, we
get its default message digest using the function
'EVP_PKEY_get_default_digest_nid'.

We also use the key of the default certificate instead of generated it. So we
are sure to use the same key type instead of always using a RSA key.
2015-10-09 12:13:12 +02:00
Christopher Faulet
c6f02fb929 MINOR: ssl: Read the file used to generate certificates in any order
the file specified by the SSL option 'ca-sign-file' can now contain the CA
certificate used to dynamically generate certificates and its private key in any
order.
2015-10-09 12:13:08 +02:00
Willy Tarreau
a84c267522 BUILD: ssl: fix build error introduced by recent commit
Commit d2cab92 ("BUG/MINOR: ssl: fix management of the cache where forged
certificates are stored") removed some needed #ifdefs resulting in ssl not
building on older openssl versions where SSL_CTRL_SET_TLSEXT_HOSTNAME is
not defined :

src/ssl_sock.c: In function 'ssl_sock_load_ca':
src/ssl_sock.c:2504: error: 'ssl_ctx_lru_tree' undeclared (first use in this function)
src/ssl_sock.c:2504: error: (Each undeclared identifier is reported only once
src/ssl_sock.c:2504: error: for each function it appears in.)
src/ssl_sock.c:2505: error: 'ssl_ctx_lru_seed' undeclared (first use in this function)
src/ssl_sock.c: In function 'ssl_sock_close':
src/ssl_sock.c:3095: error: 'ssl_ctx_lru_tree' undeclared (first use in this function)
src/ssl_sock.c: In function '__ssl_sock_deinit':
src/ssl_sock.c:5367: error: 'ssl_ctx_lru_tree' undeclared (first use in this function)
make: *** [src/ssl_sock.o] Error 1

Reintroduce the ifdefs around the faulty areas.
2015-10-09 12:13:07 +02:00
Christopher Faulet
77fe80c0b4 MINOR: ssl: Release Servers SSL context when HAProxy is shut down
[wt: could be backported to 1.5 as well]
2015-10-09 10:33:00 +02:00
Christopher Faulet
d2cab92e75 BUG/MINOR: ssl: fix management of the cache where forged certificates are stored
First, the LRU cache must be initialized after the configuration parsing to
correctly set its size.
Next, the function 'ssl_sock_set_generated_cert' returns -1 when an error occurs
(0 if success). In that case, the caller is responsible to free the memory
allocated for the certificate.
Finally, when a SSL certificate is generated by HAProxy but cannot be inserted
in the cache, it must be freed when the SSL connection is closed. This happens
when 'tune.ssl.ssl-ctx-cache-size' is set to 0.
2015-10-09 10:20:53 +02:00
Thierry FOURNIER
bc965348d7 DOC: ssl: missing LF
An error message miss LF
2015-08-27 11:24:23 +02:00
Thierry FOURNIER
136f9d34a9 MINOR: samples: rename union from "data" to "u"
The union name "data" is a little bit heavy while we read the source
code because we can read "data.data.sint". The rename from "data" to "u"
makes the read easiest like "data.u.sint".
2015-08-20 17:13:46 +02:00
Thierry FOURNIER
8c542cac07 MEDIUM: samples: Use the "struct sample_data" in the "struct sample"
This patch remove the struct information stored both in the struct
sample_data and in the striuct sample. Now, only thestruct sample_data
contains data, and the struct sample use the struct sample_data for storing
his own data.
2015-08-20 17:13:46 +02:00
Pradeep Jindal
cc79b003cb BUG/MINOR: ssl: TLS Ticket Key rotation broken via socket command
It seems haproxy was doing wrong pointer arithmetic to update the ticket
ring correctly.
2015-08-20 16:23:22 +02:00
Jérémie Courrèges-Anglas
17c3f6284c BUILD: ssl: Allow building against libssl without SSLv3.
If SSLv3 is explicitely requested but not available, warn the user and
bail out.
2015-07-26 08:11:57 +02:00
Thierry FOURNIER
bf65cd4d77 MAJOR: arg: converts uint and sint in sint
This patch removes the 32 bits unsigned integer and the 32 bit signed
integer. It replaces these types by a unique type 64 bit signed.
2015-07-22 00:48:23 +02:00
Thierry FOURNIER
07ee64ef4d MAJOR: sample: converts uint and sint in 64 bits signed integer
This patch removes the 32 bits unsigned integer and the 32 bit signed
integer. It replaces these types by a unique type 64 bit signed.

This makes easy the usage of integer and clarify signed and unsigned use.
With the previous version, signed and unsigned are used ones in place of
others, and sometimes the converter loose the sign. For example, divisions
are processed with "unsigned", if one entry is negative, the result is
wrong.

Note that the integer pattern matching and dotted version pattern matching
are already working with signed 64 bits integer values.

There is one user-visible change : the "uint()" and "sint()" sample fetch
functions which used to return a constant integer have been replaced with
a new more natural, unified "int()" function. These functions were only
introduced in the latest 1.6-dev2 so there's no impact on regular
deployments.
2015-07-22 00:48:23 +02:00
Willy Tarreau
732eac41f4 MEDIUM: ssl: add sni support on the server lines
The new "sni" server directive takes a sample fetch expression and
uses its return value as a hostname sent as the TLS SNI extension.
A typical use case consists in forwarding the front connection's SNI
value to the server in a bridged HTTPS forwarder :

   sni ssl_fc_sni
2015-07-10 11:43:15 +02:00
Willy Tarreau
630764188b MINOR: ssl: provide ia function to set the SNI extension on a connection
ssl_sock_set_servername() is used to set the SNI hostname on an
outgoing connection. This function comes from code originally
provided by Christopher Faulet of Qualys.
2015-07-10 11:37:29 +02:00
Willy Tarreau
fc017fec48 CLEANUP: ssl: make ssl_sock_generate_certificate() use ssl_sock_generated_cert_serial()
It saves from open-coding the hashing method. Instead all serial
generation is done in a single function.
2015-07-07 18:09:34 +02:00
Willy Tarreau
646b864fe5 CLEANUP: ssl: make ssl_sock_generated_cert_serial() take a const
It doesn't modify the input.
2015-07-07 18:09:15 +02:00
Willy Tarreau
f67214554c MINOR: ssl: make self-generated certs also work with raw IPv6 addresses
The current method of retrieving the incoming connection's destination
address to hash it is not compatible with IPv6 nor the proxy protocol
because it directly tries to get an IPv4 address from the socket. Instead
we must ask the connection. This is only used when no SNI is provided.
2015-07-07 18:04:38 +02:00
Willy Tarreau
745d412758 BUG/MINOR: ssl: fix smp_fetch_ssl_fc_session_id
Dmitry Sivachenko reported the following build warning using Clang
which is a real bug :

src/ssl_sock.c:4104:44: warning: address of 'smp->data.str.len' will always
      evaluate to 'true' [-Wpointer-bool-conversion]
        if (!smp->data.str.str || !&smp->data.str.len)

The impact is very low however, it will return an empty session_id
instead of no session id when none is found.

The fix should be backported to 1.5.
2015-06-17 19:46:14 +02:00
Willy Tarreau
c8ad3beded BUILD: ssl: fix recent build breakage on older SSL libs
Commit 31af49d ("MEDIUM: ssl: Add options to forge SSL certificates")
introduced some dependencies on SSL_CTRL_SET_TLSEXT_HOSTNAME for which
a few checks were missing, breaking the build on openssl 0.9.8.
2015-06-17 15:50:40 +02:00
Christopher Faulet
3054880e09 MINOR: ssl: Export functions to manipulate generated certificates
Following functions are now available in the SSL public API:

  * ssl_sock_create_cert
  * ssl_sock_get_generated_cert
  * ssl_sock_set_generated_cert
  * ssl_sock_generated_cert_serial

These functions could be used to create a certificate by hand, set it in the
cache used to store generated certificates and retrieve it. Here is an example
(pseudo code):

  X509        *cacert     = ...;
  EVP_PKEY    *capkey     = ...;
  char        *servername = ...;
  unsigned int serial;

  serial = ssl_sock_generated_cert_serial(servername, strlen(servername));
  if (!ssl_sock_get_generated_cert(serial, cacert)) {
      SSL_CTX *ctx = ssl_sock_create_cert(servername, serial, cacert, capkey);
      ssl_sock_set_generated_cert(ctx, serial, cacert);
  }
2015-06-12 18:06:59 +02:00
Christopher Faulet
31af49d62b MEDIUM: ssl: Add options to forge SSL certificates
With this patch, it is possible to configure HAProxy to forge the SSL
certificate sent to a client using the SNI servername. We do it in the SNI
callback.

To enable this feature, you must pass following BIND options:

 * ca-sign-file <FILE> : This is the PEM file containing the CA certitifacte and
   the CA private key to create and sign server's certificates.

 * (optionally) ca-sign-pass <PASS>: This is the CA private key passphrase, if
   any.

 * generate-certificates: Enable the dynamic generation of certificates for a
   listener.

Because generating certificates is expensive, there is a LRU cache to store
them. Its size can be customized by setting the global parameter
'tune.ssl.ssl-ctx-cache-size'.
2015-06-12 18:06:59 +02:00
Remi Gacogne
d3a341a96f MEDIUM: ssl: replace standards DH groups with custom ones
It is likely that powerful adversaries have been pre-computing the
standardized DH groups, because being widely used have made them
valuable targets. While users are advised to generate their own
DH parameters, replace the ones we ship by values been randomly
generated for this product only.

[wt: replaced dh1024_p, dh2048_p, and dh4096_p with locally-generated
 ones as recommended by Rémi]
2015-05-31 22:41:28 +02:00
Remi Gacogne
47783ef05b MEDIUM: ssl: add the possibility to use a global DH parameters file
This patch adds the ssl-dh-param-file global setting. It sets the
default DH parameters that will be used during the SSL/TLS handshake when
ephemeral Diffie-Hellman (DHE) key exchange is used, for all "bind" lines
which do not explicitely define theirs.
2015-05-31 22:02:00 +02:00
Remi Gacogne
d3a23c3eb8 MINOR: ssl: add a destructor to free allocated SSL ressources
Using valgrind or another memory leak tracking tool is easier
when the memory internally allocated by OpenSSL is cleanly released
at shutdown.
2015-05-28 18:03:29 +02:00
Remi Gacogne
4f902b8832 BUG/MEDIUM: ssl: fix tune.ssl.default-dh-param value being overwritten
Hervé Commowick reported that the logic used to avoid complaining about
ssl-default-dh-param not being set when static DH params are present
in the certificate file was clearly wrong when more than one sni_ctx
is used.
This patch stores whether static DH params are being used for each
SSL_CTX individually, and does not overwrite the value of
tune.ssl.default-dh-param.
2015-05-28 18:03:27 +02:00
Nenad Merdanovic
26ea822190 MINOR: Add sample fetch which identifies if the SSL session has been resumed
Signed-off-by: Nenad Merdanovic <nmerdan@anine.io>
2015-05-18 07:07:53 +02:00
Nenad Merdanovic
200b0facde MEDIUM: Add support for updating TLS ticket keys via socket
Until now, HAproxy needed to be restarted to change the TLS ticket
keys. With this patch, the TLS keys can be updated on a per-file
basis using the admin socket. Two new socket commands have been
introduced: "show tls-keys" and "set ssl tls-keys".

Signed-off-by: Nenad Merdanovic <nmerdan@anine.io>
2015-05-16 11:28:04 +02:00
Nenad Merdanovic
146defaff4 MINOR: Add TLS ticket keys reference and use it in the listener struct
Within the listener struct we need to use a reference to the TLS
ticket keys which binds the actual keys with the filename. This will
make it possible to update the keys through the socket

Signed-off-by: Nenad Merdanovic <nmerdan@anine.io>
2015-05-16 11:28:04 +02:00
Willy Tarreau
e26bf05115 BUILD/MINOR: ssl: fix build failure introduced by recent patch
Baptiste reported that commit 0a9a2b8 ("MEDIUM: sample change the
prototype of sample-fetches and converters functions") broke the
build of ssl_sock.c when using openssl-1.0.2 because one missed
replacement of sess with smp->sess. No backport is needed.
2015-05-12 10:30:12 +02:00
Thierry FOURNIER
0786d05a04 MEDIUM: sample: change the prototype of sample-fetches functions
This patch removes the "opt" entry from the prototype of the
sample-fetches fucntions. This permits to remove some weight
in the prototype call.
2015-05-11 20:03:08 +02:00
Thierry FOURNIER
0a9a2b8cec MEDIUM: sample change the prototype of sample-fetches and converters functions
This patch removes the structs "session", "stream" and "proxy" from
the sample-fetches and converters function prototypes.

This permits to remove some weight in the prototype call.
2015-05-11 20:01:42 +02:00
Alexander Rigbo
fc65af0c61 BUG/MINOR: ssl: Display correct filename in error message
This patch should be backported to 1.5.
2015-04-11 10:28:59 +02:00
Willy Tarreau
192252e2d8 MAJOR: sample: pass a pointer to the session to each sample fetch function
Many such function need a session, and till now they used to dereference
the stream. Once we remove the stream from the embryonic session, this
will not be possible anymore.

So as of now, sample fetch functions will be called with this :

   - sess = NULL,  strm = NULL                     : never
   - sess = valid, strm = NULL                     : tcp-req connection
   - sess = valid, strm = valid, strm->txn = NULL  : tcp-req content
   - sess = valid, strm = valid, strm->txn = valid : http-req / http-res
2015-04-06 11:37:25 +02:00
Willy Tarreau
15e91e1b36 MAJOR: sample: don't pass l7 anymore to sample fetch functions
All of them can now retrieve the HTTP transaction *if it exists* from
the stream and be sure to get NULL there when called with an embryonic
session.

The patch is a bit large because many locations were touched (all fetch
functions had to have their prototype adjusted). The opportunity was
taken to also uniformize the call names (the stream is now always "strm"
instead of "l4") and to fix indent where it was broken. This way when
we later introduce the session here there will be less confusion.
2015-04-06 11:35:53 +02:00
Willy Tarreau
53c9b4db41 CLEANUP: sample: remove useless tests in fetch functions for l4 != NULL
The stream may never be null given that all these functions are called
from sample_process(). Let's remove this now confusing test which
sometimes happens after a dereference was already done.
2015-04-06 11:35:52 +02:00
Willy Tarreau
9ad7bd48d2 MEDIUM: session: use the pointer to the origin instead of s->si[0].end
When s->si[0].end was dereferenced as a connection or anything in
order to retrieve information about the originating session, we'll
now use sess->origin instead so that when we have to chain multiple
streams in HTTP/2, we'll keep accessing the same origin.
2015-04-06 11:34:29 +02:00
Willy Tarreau
87b09668be REORG/MAJOR: session: rename the "session" entity to "stream"
With HTTP/2, we'll have to support multiplexed streams. A stream is in
fact the largest part of what we currently call a session, it has buffers,
logs, etc.

In order to catch any error, this commit removes any reference to the
struct session and tries to rename most "session" occurrences in function
names to "stream" and "sess" to "strm" when that's related to a session.

The files stream.{c,h} were added and session.{c,h} removed.

The session will be reintroduced later and a few parts of the stream
will progressively be moved overthere. It will more or less contain
only what we need in an embryonic session.

Sample fetch functions and converters will have to change a bit so
that they'll use an L5 (session) instead of what's currently called
"L4" which is in fact L6 for now.

Once all changes are completed, we should see approximately this :

   L7 - http_txn
   L6 - stream
   L5 - session
   L4 - connection | applet

There will be at most one http_txn per stream, and a same session will
possibly be referenced by multiple streams. A connection will point to
a session and to a stream. The session will hold all the information
we need to keep even when we don't yet have a stream.

Some more cleanup is needed because some code was already far from
being clean. The server queue management still refers to sessions at
many places while comments talk about connections. This will have to
be cleaned up once we have a server-side connection pool manager.
Stream flags "SN_*" still need to be renamed, it doesn't seem like
any of them will need to move to the session.
2015-04-06 11:23:56 +02:00
Willy Tarreau
d85c48589a REORG: connection: move conn_drain() to connection.c and rename it
It's now called conn_sock_drain() to make it clear that it only reads
at the sock layer and not at the data layer. The function was too big
to remain inlined and it's used at a few places where size counts.
2015-03-13 00:42:48 +01:00
Janusz Dziemidowicz
2c701b5f3b MEDIUM: ssl: Certificate Transparency support
Adds ability to include Signed Certificate Timestamp List in TLS
extension. File containing SCTL must be present at the same path of
the certificate file, suffixed with '.sctl'. This requires OpenSSL
1.0.2 or later.
2015-03-11 23:27:05 +01:00
Thierry FOURNIER
f41a809dc9 MINOR: sample: add private argument to the struct sample_fetch
The add of this private argument is to prepare the integration
of the lua fetchs.
2015-02-28 23:12:31 +01:00
Willy Tarreau
622317d5e2 BUILD/CLEANUP: ssl: avoid a warning due to mixed code and declaration
Actually it was caused by some dummy code meant to shut gcc up for
an unused argument!
2015-02-28 23:12:30 +01:00
Nenad Merdanovic
05552d4b98 MEDIUM: Add support for configurable TLS ticket keys
Until now, the TLS ticket keys couldn't have been configured and
shared between multiple instances or multiple servers running HAproxy.
The result was that if a request got a TLS ticket from one instance/server
and it hits another one afterwards, it will have to go through the full
SSL handshake and negotation.

This patch enables adding a ticket file to the bind line, which will be
used for all SSL contexts created from that bind line. We can use the
same file on all instances or servers to mitigate this issue and have
consistent TLS tickets assigned. Clients will no longer have to negotiate
every time they change the handling process.

Signed-off-by: Nenad Merdanovic <nmerdan@anine.io>
2015-02-28 23:10:22 +01:00
Willy Tarreau
2a3fb1c8bb MINOR: ssl/server: add the "no-ssl-reuse" server option
This option disables SSL session reuse when SSL is used to communicate with
the server. It will force the server to perform a full handshake for every
new connection. It's probably only useful for benchmarking, troubleshooting,
and for paranoid users.
2015-02-06 18:04:08 +01:00
Cyril Bonté
3180f7b554 MINOR: ssl: load certificates in alphabetical order
As reported by Raphaël Enrici, certificates loaded from a directory are loaded
in a non predictive order. If no certificate was first loaded from a file, it
can result in different behaviours when haproxy is used in cluster.
We can also imagine other cases which weren't met yet.

Instead of using readdir(), we can use scandir() and sort files alphabetically.
This will ensure a predictive behaviour.

This patch should also be backported to 1.5.
2015-01-25 00:48:01 +01:00
Willy Tarreau
d92aa5c44a MINOR: global: report information about the cost of SSL connections
An SSL connection takes some memory when it exists and during handshakes.
We measured up to 16kB for an established endpoint, and up to 76 extra kB
during a handshake. The SSL layer stores these values into the global
struct during initialization. If other SSL libs are used, it's easy to
change these values. Anyway they'll only be used as gross estimates in
order to guess the max number of SSL conns that can be established when
memory is constrained and the limit is not set.
2015-01-15 21:34:39 +01:00
Willy Tarreau
fce03113fa MINOR: global: always export some SSL-specific metrics
We'll need to know the number of SSL connections, their use and their
cost soon. In order to avoid getting tons of ifdefs everywhere, always
export SSL information in the global section. We add two flags to know
whether or not SSL is used in a frontend and in a backend.
2015-01-15 21:32:40 +01:00
Lukas Tribus
e4e30f7d52 BUILD: ssl: use OPENSSL_NO_OCSP to detect OCSP support
Since commit 656c5fa7e8 ("BUILD: ssl: disable OCSP when using
boringssl) the OCSP code is bypassed when OPENSSL_IS_BORINGSSL
is defined. The correct thing to do here is to use OPENSSL_NO_OCSP
instead, which is defined for this exact purpose in
openssl/opensslfeatures.h.

This makes haproxy forward compatible if boringssl ever introduces
full OCSP support with the additional benefit that it links fine
against a OCSP-disabled openssl.

Signed-off-by: Lukas Tribus <luky-37@hotmail.com>
2014-12-09 20:49:22 +01:00
Cyril Bonté
9ce1311ebc BUG/MEDIUM: checks: fix conflicts between agent checks and ssl healthchecks
Lasse Birnbaum Jensen reported an issue when agent checks are used at the same
time as standard healthchecks when SSL is enabled on the server side.

The symptom is that agent checks try to communicate in SSL while it should
manage raw data. This happens because the transport layer is shared between all
kind of checks.

To fix the issue, the transport layer is now stored in each check type,
allowing to use SSL healthchecks when required, while an agent check should
always use the raw_sock implementation.

The fix must be backported to 1.5.
2014-11-16 00:53:12 +01:00
Willy Tarreau
396a186def MEDIUM: ssl: add support for smaller SSL records
There's a very common openssl patch on the net meant to significantly
reduce openssl's memory usage. This patch has been provided for many
versions now, and it makes sense to add support for it given that it
is very simple. It only requires to add an extra SSL_MODE flag. Just
like for other flags, if the flag is unknown, it's unset. About 44kB
of memory may be saved per SSL session with the patch.
2014-11-13 14:08:53 +01:00
Willy Tarreau
fba03cdc5a BUG/MEDIUM: ssl: force a full GC in case of memory shortage
When memory becomes scarce and openssl refuses to allocate a new SSL
session, it is worth freeing the pools and trying again instead of
rejecting all incoming SSL connection. This can happen when some
memory usage limits have been assigned to the haproxy process using
-m or with ulimit -m/-v.

This is mostly an enhancement of previous fix and is worth backporting
to 1.5.
2014-11-13 13:48:58 +01:00