Skip to content

Commit

Permalink
BUG/MINOR: ssl/cli: can't find ".crt" files when replacing a certificate
Browse files Browse the repository at this point in the history
Bug was introduced by commit 26654 ("MINOR: ssl: add "crt" in the
cert_exts array").

When looking for a .crt directly in the cert_exts array, the
ssl_sock_load_pem_into_ckch() function will be called with a argument
which does not have its ".crt" extensions anymore.

If "ssl-load-extra-del-ext" is used this is not a problem since we try
to add the ".crt" when doing the lookup in the tree.

However when using directly a ".crt" without this option it will failed
looking for the file in the tree.

The fix removes the "crt" entry from the array since it does not seem to
be really useful without a rework of all the lookups.

Should fix issue haproxy#2265

Must be backported as far as 2.6.

(cherry picked from commit e7d9082)
Signed-off-by: Christopher Faulet <[email protected]>
  • Loading branch information
wlallemand authored and capflam committed Sep 6, 2023
1 parent aa15db9 commit d513644
Show file tree
Hide file tree
Showing 4 changed files with 91 additions and 2 deletions.
1 change: 0 additions & 1 deletion include/haproxy/ssl_ckch-t.h
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,6 @@ struct cafile_entry {

enum {
CERT_TYPE_PEM = 0,
CERT_TYPE_CRT,
CERT_TYPE_KEY,
#if ((defined SSL_CTRL_SET_TLSEXT_STATUS_REQ_CB && !defined OPENSSL_NO_OCSP) || defined OPENSSL_IS_BORINGSSL)
CERT_TYPE_OCSP,
Expand Down
1 change: 1 addition & 0 deletions reg-tests/ssl/bug-2265.crt
90 changes: 90 additions & 0 deletions reg-tests/ssl/set_ssl_bug_2265.vtc
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
#REGTEST_TYPE=devel

# 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,
# 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.
#
# This is the same as "set_ssl_cert_noext.vtc" but the .crt contains both the certificate and the key.
#
# If this test does not work anymore:
# - Check that you have socat

varnishtest "Test the 'set ssl cert' feature of the CLI with separate key and crt"
#REQUIRE_VERSION=2.2
#REQUIRE_OPTIONS=OPENSSL
feature cmd "command -v socat"
feature ignore_unknown_macro

server s1 -repeat 3 {
rxreq
txresp
} -start

haproxy h1 -conf {
global
tune.ssl.default-dh-param 2048
tune.ssl.capture-buffer-size 1
stats socket "${tmpdir}/h1/stats" level admin

defaults
mode http
option httplog
retries 0
log stderr local0 debug err
option logasap
timeout connect "${HAPROXY_TEST_TIMEOUT-5s}"
timeout client "${HAPROXY_TEST_TIMEOUT-5s}"
timeout server "${HAPROXY_TEST_TIMEOUT-5s}"

listen clear-lst
bind "fd@${clearlst}"
balance roundrobin
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)

listen ssl-lst
bind "${tmpdir}/ssl.sock" ssl crt ${testdir}/bug-2265.crt strict-sni

server s1 ${s1_addr}:${s1_port}
} -start


haproxy h1 -cli {
send "show ssl cert ${testdir}/bug-2265.crt"
expect ~ ".*SHA1 FingerPrint: 2195C9F0FD58470313013FC27C1B9CF9864BD1C6"
}

client c1 -connect ${h1_clearlst_sock} {
txreq
rxresp
expect resp.status == 200
} -run

shell {
printf "set ssl cert ${testdir}/bug-2265.crt <<\n$(cat ${testdir}/ecdsa.pem)\n\n" | socat "${tmpdir}/h1/stats" -
echo "commit ssl cert ${testdir}/bug-2265.crt" | socat "${tmpdir}/h1/stats" -
}

haproxy h1 -cli {
send "show ssl cert ${testdir}/bug-2265.crt"
expect ~ ".*SHA1 FingerPrint: A490D069DBAFBEE66DE434BEC34030ADE8BCCBF1"
}

# check that the "www.test1.com" SNI was removed
client c1 -connect ${h1_clearlst_sock} {
txreq
rxresp
expect resp.status == 503
} -run

client c1 -connect ${h1_clearlst_sock} {
txreq
rxresp
expect resp.status == 200
} -run
1 change: 0 additions & 1 deletion src/ssl_ckch.c
Original file line number Diff line number Diff line change
Expand Up @@ -1414,7 +1414,6 @@ int ssl_store_load_locations_file(char *path, int create_if_none, enum cafile_ty

struct cert_exts cert_exts[] = {
{ "", CERT_TYPE_PEM, &ssl_sock_load_pem_into_ckch }, /* default mode, no extensions */
{ "crt", CERT_TYPE_CRT, &ssl_sock_load_pem_into_ckch },
{ "key", CERT_TYPE_KEY, &ssl_sock_load_key_into_ckch },
#if ((defined SSL_CTRL_SET_TLSEXT_STATUS_REQ_CB && !defined OPENSSL_NO_OCSP) || defined OPENSSL_IS_BORINGSSL)
{ "ocsp", CERT_TYPE_OCSP, &ssl_sock_load_ocsp_response_from_file },
Expand Down

0 comments on commit d513644

Please sign in to comment.