BUG/MEDIUM: stream: don't retry SSL connections which fail the SNI name check
Commits 2ab8867 ("MINOR: ssl: compare server certificate names to the
SNI on outgoing connections") and 96c7b8d ("BUG/MINOR: ssl: Fix check
against SNI during server certificate verification") made it possible
to check that the server's certificate matches the name presented in
the SNI field. While it solves a class of problems, it opens another
one which is that by failing such a connection, we'll retry it and put
more load on the server. It can be a real problem if a user can trigger
this issue, which is what will very often happen when the SNI is forwarded
from the client to the server.
This patch solves this by detecting that this very specific hostname
verification failed and that the hostname was provided using SNI, and
then it simply disables retries and the failure is immediate.
At the time of writing this patch, the previous patches were not backported
(yet), so no backport is needed for this one unless the aforementionned
patches are backported as well. This patch requires previous patches
"BUG/MINOR: ssl: make use of the name in SNI before verifyhost" and
"MINOR: ssl: add a new error code for wrong server certificates".
diff --git a/src/stream.c b/src/stream.c
index 1aa5475..6b7be1e 100644
--- a/src/stream.c
+++ b/src/stream.c
@@ -595,6 +595,7 @@
static int sess_update_st_cer(struct stream *s)
{
struct stream_interface *si = &s->si[1];
+ struct connection *conn = objt_conn(si->end);
/* we probably have to release last stream from the server */
if (objt_server(s->target)) {
@@ -604,6 +605,27 @@
s->flags &= ~SF_CURR_SESS;
objt_server(s->target)->cur_sess--;
}
+
+ if ((si->flags & SI_FL_ERR) &&
+ conn && conn->err_code == CO_ER_SSL_MISMATCH_SNI) {
+ /* We tried to connect to a server which is configured
+ * with "verify required" and which doesn't have the
+ * "verifyhost" directive. The server presented a wrong
+ * certificate (a certificate for an unexpected name),
+ * which implies that we have used SNI in the handshake,
+ * and that the server doesn't have the associated cert
+ * and presented a default one.
+ *
+ * This is a serious enough issue not to retry. It's
+ * especially important because this wrong name might
+ * either be the result of a configuration error, and
+ * retrying will only hammer the server, or is caused
+ * by the use of a wrong SNI value, most likely
+ * provided by the client and we don't want to let the
+ * client provoke retries.
+ */
+ si->conn_retries = 0;
+ }
}
/* ensure that we have enough retries left */