BUG/MINOR: ssl: Fix update of default certificate

The default SSL_CTX used by a specific frontend is the one of the first
ckch instance created for this frontend. If this instance has SNIs, then
the SSL context is linked to the instance through the list of SNIs
contained in it. If the instance does not have any SNIs though, then the
SSL_CTX is only referenced by the bind_conf structure and the instance
itself has no link to it.
When trying to update a certificate used by the default instance through
a cli command, a new version of the default instance was rebuilt but the
default SSL context referenced in the bind_conf structure would not be
changed, resulting in a buggy behavior in which depending on the SNI
used by the client, he could either use the new version of the updated
certificate or the original one.

This patch adds a reference to the default SSL context in the default
ckch instances so that it can be hot swapped during a certificate
update.

This should fix GitHub issue #1143.

It can be backported as far as 2.2.
This commit is contained in:
Remi Tricot-Le Breton 2021-03-17 14:56:54 +01:00 committed by William Lallemand
parent 62592ad967
commit 8218aed90e
5 changed files with 175 additions and 12 deletions

View File

@ -86,7 +86,7 @@ struct ckch_inst {
struct ckch_store *ckch_store; /* pointer to the store used to generate this inst */
struct crtlist_entry *crtlist_entry; /* pointer to the crtlist_entry used, or NULL */
struct server *server; /* pointer to the server if is_server_instance is set, NULL otherwise */
SSL_CTX *ctx; /* pointer to the SSL context used by this instance if it is a server one (is_server_instance set) */
SSL_CTX *ctx; /* pointer to the SSL context used by this instance */
unsigned int is_default:1; /* This instance is used as the default ctx for this bind_conf */
unsigned int is_server_instance:1; /* This instance is used by a backend server */
/* space for more flag there */

View File

@ -0,0 +1,2 @@
set_default_cert.pem !*
set_default_cert.pem www.test1.com

View File

@ -0,0 +1,52 @@
-----BEGIN CERTIFICATE-----
MIIENjCCAh4CAQEwDQYJKoZIhvcNAQELBQAwWzELMAkGA1UEBhMCRlIxDjAMBgNV
BAgMBVBhcmlzMQ4wDAYDVQQHDAVQYXJpczEVMBMGA1UECgwMSEFQcm94eSBUZWNo
MRUwEwYDVQQDDAxIQVByb3h5IFRlY2gwHhcNMjEwMzAyMTcxODUwWhcNMjIwMzAy
MTcxODUwWjBnMQswCQYDVQQGEwJGUjETMBEGA1UECAwKU29tZS1TdGF0ZTEOMAwG
A1UEBwwFUGFyaXMxHTAbBgNVBAoMFEhBUHJveHkgVGVjaG5vbG9naWVzMRQwEgYD
VQQDDAsqLnRlc3QxLmNvbTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEB
APjmyWLJ1olKg/EOarln7oQB7pdUrF6kS1YG+Nz0sgFzxnU0PHn/IeARCprHyEZ4
eBOrQ0CHhM5hdEFDX8iq32rektcQqwfH83iwD9zXxFBJ7ItoWG6YAN6WLXjEDYEI
hxLJMlW3kfYODKhNMvoqXyZi2wTyAJI+aLJI7pbeD+YNb0AwOnSH5ag5ohZIr3QU
99UD/VUhndv4OP8JZwBiV6Qy79jVDVbPFGaOc70VkMQSCHytyudQicUZrYQdIw1E
981JF/UpbnECLWyB3V+4t1KtWOW90vkUoBEj8Nxe6kYnMaNSjQhfKSF6zGmUOXYp
oHPCgms8v4JaovQygo02Qi8CAwEAATANBgkqhkiG9w0BAQsFAAOCAgEAAz8IntYc
zrbIqweHfD9CZTNIQiobhQmgykT0KQ23Gm2y/e3o63XOqxDv0bEctg4zE83w3g7d
mJlEyCB0N0qC8UGGsbRm5Cny7H//g3u06NqSMBYbdU+BgZBBj16I5Kcw/kSBb9dA
wslLlrUjBj6dK83EB1cpyqpyZHIXkR/E424ggfc45rmD60AtU0SvzVNZfIK0PmB0
3YKiUlO7cl5CzTeTg2BooRvqwblya62SRkhfFL7NCRX1/S9tO/XiaYzgP7J6J09x
yYs2XhQqJFgtS+1vDp8rHKhcANFVXBJ6rDSbp1qBv7qZkQhlFf8hQtd5iBXvCb0a
KtN9L4o6t1wvyo0BbERroGU7rkPPUNiMc3gWEf/mgwGLsNNOYqY5eYoeAF7arX5f
c4LCHiAYMWa/bEY29zmm51GH5ddxFSu1j95Hfd+HlNcX8Oyfed2oCoSamochmbzA
Kktk0QfCYIv4LlaG5pUliLa6DCLK7yMfT5RC5GGb350p3uDobVj/taY2cVwXOBQb
MjXK32K9CFrnqKQptPV1ohlWgNiqhvxiGp3Yx17Cn54WL9ksO+8TlwWAttazKVlT
40tHqGOu6ld90xGZitxL2oA9kBg9Nkxas/f9+9p6sJe5wj09dj/cqRjyiKv7nek1
TIPtsNbJghDRDQ3uPEYHdX0h490qGMyGARw=
-----END CERTIFICATE-----
-----BEGIN RSA PRIVATE KEY-----
MIIEpgIBAAKCAQEA+ObJYsnWiUqD8Q5quWfuhAHul1SsXqRLVgb43PSyAXPGdTQ8
ef8h4BEKmsfIRnh4E6tDQIeEzmF0QUNfyKrfat6S1xCrB8fzeLAP3NfEUEnsi2hY
bpgA3pYteMQNgQiHEskyVbeR9g4MqE0y+ipfJmLbBPIAkj5oskjult4P5g1vQDA6
dIflqDmiFkivdBT31QP9VSGd2/g4/wlnAGJXpDLv2NUNVs8UZo5zvRWQxBIIfK3K
51CJxRmthB0jDUT3zUkX9SlucQItbIHdX7i3Uq1Y5b3S+RSgESPw3F7qRicxo1KN
CF8pIXrMaZQ5dimgc8KCazy/glqi9DKCjTZCLwIDAQABAoIBAQC/arWb7L+56/2W
iFDZb62GBfpYlXzOeCmb6la/jsvKxB/vCRItfGGv8Usnh9dlIsat0bsxyEcBdP80
Jb1nFMonZS6miSIPJN4Ahd5dJ+7JFGD/QWso+mtIw1QLGTONdWJztxmnxDpTcbCY
Sm6W57kvSz1HC1oXHjnkSqR6kCLH9y6/i7ox6IPYyDA1t/TKJMnKFOPkxKJ8A96v
1avPrCWfXWYdn6Og5ERd8FJF2L5BYImmmkPpoUeWPyMBfAYqdK5FRijO6JMn/h5k
XkJm+2bru+cRwcNYUNPuDIa+ZBWhjFfZfSOhOoECeKLe+lhfcFPC7cCSeDJAjGtR
dakm15ohAoGBAP4+rVBeSCBhPH27T3HWp74qMWkYJzkdqTV0wUUJ1wtuWZFDg/RP
OYKC+6cM0nW3K+j/9pTWMS1eM61x/VNyFQGUq/rMJGEWFH08NXnV8AxCtwKUV/rP
Uq3MB4QWfSYGMo9QL+9lu23fMWYpBLo+KIcqPjLb+8FEJGmaC9JCIYQfAoGBAPqe
qp7NzMmX2b1VR2XXm1CZzwTEFXb4NeDIxSfNbsqXCmws3jgBX3Lx7dQ9k8ymvaA5
ucYLU3ppozeB//8Ir9lSA1A4w3VN9a+l1ZdQpKQ4SuHtqDwkmKAT85vmGHCPhwlq
Er9ests3wQ4T/8HPG92QWs+Gg34F+x9U6h2FMv/xAoGBAOM6h1HWAeaWoSbKWvWm
YKNQOHryMFQW011IbVfTtJOt23U9/1hB2mdvw5SInCzDOgZzhiF90dP3Zn5063FB
+84+3vo2q6jtwAAx6KVsdK+wjLpMdNlfpEhamrkOFGoAjf2SMFVo+fv3x8HDlUsT
NMuhEJgKDlasHVMYb8pKeoQHAoGBAMAF7ij6+lvD03tz6d6oUkJxduLp8qBTEcUH
T7hteOQU0lGMFz/GHYIOx/EEtUfqwgQP9r09VFrIsdwH6UNZPpM+eXdv5qLsdsB8
SalEisGguA9fbrWWPLL6Vn8uz67+6bJW6cJjJps8ntjQjffLXkhnII09PWbD4mNh
RngT5L2hAoGBANqa+yYSvEGNAxvdfxE0u3U/4OtjCl168nNwHXmyaCKZ1e4XYflz
wGI4J1ngcCKN37RkCgfu/XRKrc82XhAhV+YYjAUqQYrTyh26b4v9Dp9tBUWiv7bk
6L+ZlCms+HpsuYmsCAu/od41OWSSpdg+R3VOE0t3rp0r1QdAGYd1nwQC
-----END RSA PRIVATE KEY-----

View File

@ -3,11 +3,19 @@
# This reg-test uses the "set ssl cert" command to update a certificate over the CLI.
# It requires socat to upload the certificate
#
# this check does 3 requests, the first one will use "www.test1.com" as SNI,
# This check has two separate parts.
# In the first part, there are 3 requests, the first one will use "www.test1.com" as SNI,
# the second one with the same but that must fail and the third one will use
# "localhost". Since vtest can't do SSL, we use haproxy as an SSL client with 2
# chained listen section.
#
# In the second part, we check the update of a default certificate in a crt-list.
# This corresponds to a bug raised in https://github.com/haproxy/haproxy/issues/1143.
# A certificate is used as default certificate as well as regular one, and during the update
# the default certificate would not be properly updated if the default instance did not have
# any SNI. The test consists in checking that the used certificate is the right one after
# updating it via a "set ssl cert" call.
#
# If this test does not work anymore:
# - Check that you have socat
@ -17,7 +25,7 @@ varnishtest "Test the 'set ssl cert' feature of the CLI"
#REQUIRE_BINARIES=socat
feature ignore_unknown_macro
server s1 -repeat 3 {
server s1 -repeat 9 {
rxreq
txresp
} -start
@ -27,6 +35,7 @@ haproxy h1 -conf {
tune.ssl.default-dh-param 2048
tune.ssl.capture-cipherlist-size 1
stats socket "${tmpdir}/h1/stats" level admin
crt-base ${testdir}
defaults
mode http
@ -41,15 +50,30 @@ haproxy h1 -conf {
listen clear-lst
bind "fd@${clearlst}"
balance roundrobin
http-response set-header X-SSL-Server-SHA1 %[ssl_s_sha1,hex]
retries 0 # 2nd SSL connection must fail so skip the retry
server s1 "${tmpdir}/ssl.sock" ssl verify none sni str(www.test1.com)
server s2 "${tmpdir}/ssl.sock" ssl verify none sni str(www.test1.com)
server s3 "${tmpdir}/ssl.sock" ssl verify none sni str(localhost)
server s4 "${tmpdir}/other-ssl.sock" ssl verify none sni str(www.test1.com)
server s5 "${tmpdir}/other-ssl.sock" ssl verify none sni str(other.test1.com) # uses the default certificate
server s6 "${tmpdir}/other-ssl.sock" ssl verify none sni str(www.test1.com)
server s7 "${tmpdir}/other-ssl.sock" ssl verify none sni str(other.test1.com) # uses the default certificate
server s8 "${tmpdir}/other-ssl.sock" ssl verify none sni str(www.test1.com)
server s9 "${tmpdir}/other-ssl.sock" ssl verify none sni str(other.test1.com) # uses the default certificate
listen ssl-lst
bind "${tmpdir}/ssl.sock" ssl crt ${testdir}/common.pem strict-sni
server s1 ${s1_addr}:${s1_port}
listen other-ssl-lst
bind "${tmpdir}/other-ssl.sock" ssl crt-list ${testdir}/set_default_cert.crt-list
server s1 ${s1_addr}:${s1_port}
} -start
@ -97,3 +121,84 @@ haproxy h1 -cli {
expect ~ ".*SHA1 FingerPrint: A490D069DBAFBEE66DE434BEC34030ADE8BCCBF1"
}
# The following requests are aimed at a backend that uses the set_default_cert.crt-list file
# Uses the www.test1.com sni
client c1 -connect ${h1_clearlst_sock} {
txreq
rxresp
expect resp.http.X-SSL-Server-SHA1 == "9DC18799428875976DDE706E9956035EE88A4CB3"
expect resp.status == 200
} -run
# Uses the other.test1.com sni and the default line of the crt-list
client c1 -connect ${h1_clearlst_sock} {
txreq
rxresp
expect resp.http.X-SSL-Server-SHA1 == "9DC18799428875976DDE706E9956035EE88A4CB3"
expect resp.status == 200
} -run
shell {
printf "set ssl cert ${testdir}/set_default_cert.pem <<\n$(cat ${testdir}/common.pem)\n\n" | socat "${tmpdir}/h1/stats" -
}
# Certificate should not have changed yet
haproxy h1 -cli {
send "show ssl cert ${testdir}/set_default_cert.pem"
expect ~ ".*SHA1 FingerPrint: 9DC18799428875976DDE706E9956035EE88A4CB3"
}
shell {
echo "commit ssl cert ${testdir}/set_default_cert.pem" | socat "${tmpdir}/h1/stats" -
}
haproxy h1 -cli {
send "show ssl cert ${testdir}/set_default_cert.pem"
expect ~ ".*SHA1 FingerPrint: 2195C9F0FD58470313013FC27C1B9CF9864BD1C6"
}
# Uses the www.test1.com sni
client c1 -connect ${h1_clearlst_sock} {
txreq
rxresp
expect resp.http.X-SSL-Server-SHA1 == "2195C9F0FD58470313013FC27C1B9CF9864BD1C6"
expect resp.status == 200
} -run
# Uses the other.test1.com sni and the default line of the crt-list
client c1 -connect ${h1_clearlst_sock} {
txreq
rxresp
expect resp.http.X-SSL-Server-SHA1 == "2195C9F0FD58470313013FC27C1B9CF9864BD1C6"
expect resp.status == 200
} -run
# Restore original certificate
shell {
printf "set ssl cert ${testdir}/set_default_cert.pem <<\n$(cat ${testdir}/set_default_cert.pem)\n\n" | socat "${tmpdir}/h1/stats" -
echo "commit ssl cert ${testdir}/set_default_cert.pem" | socat "${tmpdir}/h1/stats" -
}
haproxy h1 -cli {
send "show ssl cert ${testdir}/set_default_cert.pem"
expect ~ ".*SHA1 FingerPrint: 9DC18799428875976DDE706E9956035EE88A4CB"
}
# Uses the www.test1.com sni
client c1 -connect ${h1_clearlst_sock} {
txreq
rxresp
expect resp.http.X-SSL-Server-SHA1 == "9DC18799428875976DDE706E9956035EE88A4CB3"
expect resp.status == 200
} -run
# Uses the other.test1.com sni and the default line of the crt-list
client c1 -connect ${h1_clearlst_sock} {
txreq
rxresp
expect resp.http.X-SSL-Server-SHA1 == "9DC18799428875976DDE706E9956035EE88A4CB3"
expect resp.status == 200
} -run

View File

@ -2942,7 +2942,6 @@ void ssl_sock_load_cert_sni(struct ckch_inst *ckch_inst, struct bind_conf *bind_
struct sni_ctx *sc0, *sc0b, *sc1;
struct ebmb_node *node;
int def = 0;
list_for_each_entry_safe(sc0, sc0b, &ckch_inst->sni_ctx, by_ckch_inst) {
@ -2976,14 +2975,13 @@ void ssl_sock_load_cert_sni(struct ckch_inst *ckch_inst, struct bind_conf *bind_
ebst_insert(&bind_conf->sni_w_ctx, &sc0->name);
else
ebst_insert(&bind_conf->sni_ctx, &sc0->name);
}
/* replace the default_ctx if required with the first ctx */
if (ckch_inst->is_default && !def) {
SSL_CTX_free(bind_conf->default_ctx);
SSL_CTX_up_ref(sc0->ctx);
bind_conf->default_ctx = sc0->ctx;
def = 1;
}
/* replace the default_ctx if required with the instance's ctx. */
if (ckch_inst->is_default) {
SSL_CTX_free(bind_conf->default_ctx);
SSL_CTX_up_ref(ckch_inst->ctx);
bind_conf->default_ctx = ckch_inst->ctx;
}
}
@ -3421,6 +3419,12 @@ int ckch_inst_new_load_store(const char *path, struct ckch_store *ckchs, struct
SSL_CTX_up_ref(ctx);
}
/* Always keep a reference to the newly constructed SSL_CTX in the
* instance. This way if the instance has no SNIs, the SSL_CTX will
* still be linked. */
SSL_CTX_up_ref(ctx);
ckch_inst->ctx = ctx;
/* everything succeed, the ckch instance can be used */
ckch_inst->bind_conf = bind_conf;
ckch_inst->ssl_conf = ssl_conf;