MINOR: ssl: Add "abort ssl ca-file" CLI command

The "abort" command aborts an ongoing transaction started by a "set ssl
ca-file" command. Since the updated CA file data is not pushed into the
cafile tree until a "commit ssl ca-file" call is performed, the abort
command simply clears the new cafile_entry that was stored in the
cafile_transaction.

This fixes a subpart of GitHub issue #1057.

diff --git a/reg-tests/ssl/set_ssl_cafile.vtc b/reg-tests/ssl/set_ssl_cafile.vtc
index fb34531a80654c60c546a8579b9b1365cc2651ac..0cfed1dec87c75f23beab67e3800f67c9948a169 100644 (file)
--- a/reg-tests/ssl/set_ssl_cafile.vtc
+++ b/reg-tests/ssl/set_ssl_cafile.vtc
@@ -1,6 +1,7 @@
 #REGTEST_TYPE=devel
 
 # This reg-test uses the "set ssl ca-file" command to update a CA file over the CLI.
+# It also tests the "abort ssl ca-file" command.
 #
 # It is based on two CA certificates, set_cafile_interCA1.crt and set_cafile_interCA2.crt,
 # and a client certificate that was signed with set_cafile_interCA1.crt (set_cafile_client.pem)
@@ -19,7 +20,7 @@ varnishtest "Test the 'set ssl ca-file' feature of the CLI"
 #REQUIRE_BINARIES=socat
 feature ignore_unknown_macro
 
-server s1 -repeat 3 {
+server s1 -repeat 4 {
   rxreq
   txresp
 } -start
@@ -67,6 +68,27 @@ client c1 -connect ${h1_clearlst_sock} {
     expect resp.http.X-SSL-Client-Verify == 21
 } -run
 
+# Set a new ca-file without committing it and check that the new ca-file is not taken into account
+shell {
+    printf "set ssl ca-file ${testdir}/set_cafile_ca2.crt <<\n$(cat ${testdir}/set_cafile_ca1.crt)\n\n" | socat "${tmpdir}/h1/stats" -
+}
+
+# This connection should still fail for the same reasons as previously
+client c1 -connect ${h1_clearlst_sock} {
+    txreq
+    rxresp
+    expect resp.status == 200
+    # unable to verify the client certificate
+    expect resp.http.X-SSL-Client-Verify == 21
+} -run
+
+haproxy h1 -cli {
+    send "abort ssl ca-file ${testdir}/set_cafile_ca2.crt"
+    expect ~ "Transaction aborted for certificate '${testdir}/set_cafile_ca2.crt'!"
+    send "commit ssl ca-file ${testdir}/set_cafile_ca2.crt"
+    expect ~ "No ongoing transaction!"
+}
+
 
 # Update the bind line's ca-file in order to accept the client certificate
 shell {

diff --git a/src/ssl_ckch.c b/src/ssl_ckch.c
index 5b659c9bd331433f3cabd544630f08a8bc4f59b4..b178d85361896390cd5fecfcf37b2cda3126c15b 100644 (file)
--- a/src/ssl_ckch.c
+++ b/src/ssl_ckch.c
@@ -2510,6 +2510,50 @@ error:
        return 1;
 }
 
+
+/* parsing function of 'abort ssl ca-file' */
+static int cli_parse_abort_cafile(char **args, char *payload, struct appctx *appctx, void *private)
+{
+       char *err = NULL;
+
+       if (!cli_has_level(appctx, ACCESS_LVL_ADMIN))
+               return 1;
+
+       if (!*args[3])
+               return cli_err(appctx, "'abort ssl ca-file' expects a filename\n");
+
+       /* The operations on the CKCH architecture are locked so we can
+        * manipulate ckch_store and ckch_inst */
+       if (HA_SPIN_TRYLOCK(CKCH_LOCK, &ckch_lock))
+               return cli_err(appctx, "Can't abort!\nOperations on certificates are currently locked!\n");
+
+       if (!cafile_transaction.path) {
+               memprintf(&err, "No ongoing transaction!\n");
+               goto error;
+       }
+
+       if (strcmp(cafile_transaction.path, args[3]) != 0) {
+               memprintf(&err, "The ongoing transaction is about '%s' but you are trying to abort a transaction for '%s'\n", cafile_transaction.path, args[3]);
+               goto error;
+       }
+
+       /* Only free the uncommitted cafile_entry here, because the SNI and instances were not generated yet */
+       ssl_store_delete_cafile_entry(cafile_transaction.new_cafile_entry);
+       cafile_transaction.new_cafile_entry = NULL;
+       cafile_transaction.old_cafile_entry = NULL;
+       ha_free(&cafile_transaction.path);
+
+       HA_SPIN_UNLOCK(CKCH_LOCK, &ckch_lock);
+
+       err = memprintf(&err, "Transaction aborted for certificate '%s'!\n", args[3]);
+       return cli_dynmsg(appctx, LOG_NOTICE, err);
+
+error:
+       HA_SPIN_UNLOCK(CKCH_LOCK, &ckch_lock);
+
+       return cli_dynerr(appctx, err);
+}
+
 /* release function of the `commit ssl ca-file' command, free things and unlock the spinlock */
 static void cli_release_commit_cafile(struct appctx *appctx)
 {
@@ -2549,6 +2593,7 @@ static struct cli_kw_list cli_kws = {{ },{
 
        { { "set", "ssl", "ca-file", NULL },    "set ssl ca-file <cafile> <payload>      : replace a CA file",                                                     cli_parse_set_cafile, NULL, NULL },
        { { "commit", "ssl", "ca-file", NULL }, "commit ssl ca-file <cafile>             : commit a CA file",                                                      cli_parse_commit_cafile, cli_io_handler_commit_cafile, cli_release_commit_cafile },
+       { { "abort", "ssl", "ca-file", NULL },  "abort ssl ca-file <cafile>              : abort a transaction for a CA file",                                     cli_parse_abort_cafile, NULL, NULL },
        { { NULL }, NULL, NULL, NULL }
 }};