Set tls_ctx->vhost to NULL if no SNI extension present

Review fixes #2 - fuse domain fronting ans strict host checking frang validations together - show full SNI in "unknown host" message
tempesta-tech · Jun 16, 2023 · da18ccd · da18ccd
1 parent 6841156
commit da18ccd
Show file tree

Hide file tree

Showing 5 changed files with 43 additions and 73 deletions.
diff --git a/fw/http_limits.c b/fw/http_limits.c
@@ -627,6 +627,40 @@ frang_assert_host_header(const TfwStr *l, const TfwStr *r)
 	return tfw_strcmp(l, r);
 }
 
+static int
+frang_http_domain_fronting_check(const TfwHttpReq *req, FrangAcc *ra)
+{
+	TlsCtx *tctx;
+
+	if ((TFW_CONN_TYPE(req->conn) & TFW_FSM_HTTPS) == 0)
+		return TFW_PASS;
+
+	tctx = tfw_tls_context(req->conn);
+
+	if (tctx->vhost != req->vhost) {
+		TfwVhost *tls_vhost = tctx->vhost;
+		BasicStr tls_name, req_name;
+		static BasicStr null_name = {"NULL", 4};
+
+		/* An exotic case where TLS connection hasn't assigned
+		 * any vhost to the TlsCtx */
+		if (unlikely(tls_vhost == NULL))
+			return TFW_PASS;
+		/* Special case of default vhosts */
+		if (req->vhost == NULL
+		    && tfw_vhost_is_default(tls_vhost))
+			return TFW_PASS;
+
+		tls_name = tctx->vhost ? tls_vhost->name : null_name;
+		req_name = req->vhost ? req->vhost->name : null_name;
+		frang_msg("vhost by SNI doesn't match vhost by authority",
+		          &FRANG_ACC2CLI(ra)->addr, " ('%.*s' vs '%.*s')\n",
+		          PR_TFW_STR(&tls_name), PR_TFW_STR(&req_name));
+		return TFW_BLOCK;
+	}
+	return TFW_PASS;
+}
+
 /**
  * Require host header in HTTP request (RFC 7230 5.4).
  * Block HTTP/1.1 requests w/o host header,
@@ -735,41 +769,7 @@ frang_http_host_check(const TfwHttpReq *req, FrangAcc *ra)
 		return TFW_BLOCK;
 	}
 
-	return TFW_PASS;
-}
-
-static int
-frang_http_domain_fronting_check(const TfwHttpReq *req, FrangAcc *ra)
-{
-	TlsCtx *tctx;
-
-	if ((TFW_CONN_TYPE(req->conn) & TFW_FSM_HTTPS) == 0)
-		return TFW_PASS;
-
-	tctx = tfw_tls_context(req->conn);
-
-	if (tctx->vhost != req->vhost) {
-		TfwVhost *tls_vhost = tctx->vhost;
-		BasicStr tls_name, req_name;
-		static BasicStr null_name = {"NULL", 4};
-
-		/* An exotic case where TLS connection hasn't assigned
-		 * any vhost to the TlsCtx */
-		if (unlikely(tls_vhost == NULL))
-			return TFW_PASS;
-		/* Special case of default vhosts */
-		if (req->vhost == NULL
-		    && tfw_vhost_is_default(tls_vhost))
-			return TFW_PASS;
-
-		tls_name = tctx->vhost ? tls_vhost->name : null_name;
-		req_name = req->vhost ? req->vhost->name : null_name;
-		frang_msg("vhost by SNI doesn't match vhost by authority",
-		          &FRANG_ACC2CLI(ra)->addr, " ('%.*s' vs '%.*s')\n",
-		          PR_TFW_STR(&tls_name), PR_TFW_STR(&req_name));
-		return TFW_BLOCK;
-	}
-	return TFW_PASS;
+	return frang_http_domain_fronting_check(req, ra);
 }
 
 /*
@@ -1133,12 +1133,6 @@ frang_http_req_process(FrangAcc *ra, TfwConn *conn, TfwFsmData *data,
 		{
 			T_FSM_EXIT();
 		}
-		/* Optional protection against domain fronting attacks */
-		if (!f_cfg->http_allow_domain_fronting
-		    && (r = frang_http_domain_fronting_check(req, ra)))
-		{
-			T_FSM_EXIT();
-		}
 		/* Ensure overridden HTTP method suits restrictions. */
 		r = frang_http_methods_override(req, ra, f_cfg);
 		if (r)

diff --git a/fw/http_limits.h b/fw/http_limits.h
@@ -169,7 +169,6 @@ struct frang_global_cfg_t {
  * @http_trailer_split  - Allow the same header appear in both
  *			  request header part and chunked trailer part;
  * @http_method_override - Allow method override in request headers.
- * @http_allow_domain_fronting - Allow SNI/authority mismatch
  */
 struct frang_vhost_cfg_t {
 	unsigned long		http_methods_mask;
@@ -183,7 +182,6 @@ struct frang_vhost_cfg_t {
 
 	bool			http_ct_required;
 	bool			http_strict_host_checking;
-	bool			http_allow_domain_fronting;
 	bool			http_trailer_split;
 	bool			http_method_override;
 };

diff --git a/fw/tls.c b/fw/tls.c
@@ -817,9 +817,14 @@ tfw_tls_sni(TlsCtx *ctx, const unsigned char *data, size_t len)
 
 		if (unlikely(!vhost && !tfw_tls.allow_any_sni)) {
 			SNI_WARN("unknown server name '%.*s', reject connection.\n",
-				 PR_TFW_STR(&srv_name));
+				 (int)len, data);
 			return -ENOENT;
 		}
+		/* Save vhost that had been selected by SNI */
+		ctx->vhost = vhost;
+	} else {
+		/* No SNI => no vhost for later frang checks */
+		ctx->vhost = NULL;
 	}
 	/*
 	 * If accurate vhost is not found or client doesn't send sni extension,
@@ -851,8 +856,6 @@ tfw_tls_sni(TlsCtx *ctx, const unsigned char *data, size_t len)
 	}
 	/* Save processed server name as hash. */
 	ctx->sni_hash = len ? hash_calc(data, len) : 0;
-	/* Save vhost that had been selected by SNI */
-	ctx->vhost = vhost;
 
 	return 0;
 }

diff --git a/fw/vhost.c b/fw/vhost.c
@@ -2219,20 +2219,6 @@ tfw_cfgop_frang_strict_host_checking(TfwCfgSpec *cs, TfwCfgEntry *ce)
 	return r;
 }
 
-static int
-tfw_cfgop_frang_allow_domain_fronting(TfwCfgSpec *cs, TfwCfgEntry *ce)
-{
-	int r;
-	FrangVhostCfg *cfg = tfw_cfgop_frang_get_cfg();
-
-	if (ce->dflt_value && cfg->http_allow_domain_fronting)
-		return 0;
-	cs->dest = &cfg->http_allow_domain_fronting;
-	r = tfw_cfg_set_bool(cs, ce);
-	cs->dest = NULL;
-	return r;
-}
-
 static int
 tfw_cfgop_frang_ct_required(TfwCfgSpec *cs, TfwCfgEntry *ce)
 {
@@ -2826,12 +2812,6 @@ static TfwCfgSpec tfw_global_frang_specs[] = {
 		.handler = tfw_cfgop_frang_strict_host_checking,
 		.allow_reconfig = true,
 	},
-	{
-		.name = "http_allow_domain_fronting",
-		.deflt = "true",
-		.handler = tfw_cfgop_frang_allow_domain_fronting,
-		.allow_reconfig = true,
-	},
 	{
 		.name = "http_ct_required",
 		.deflt = "false",
@@ -2984,12 +2964,6 @@ static TfwCfgSpec tfw_vhost_frang_specs[] = {
 		.handler = tfw_cfgop_frang_strict_host_checking,
 		.allow_reconfig = true,
 	},
-	{
-		.name = "http_allow_domain_fronting",
-		.deflt = "true",
-		.handler = tfw_cfgop_frang_allow_domain_fronting,
-		.allow_reconfig = true,
-	},
 	{
 		.name = "http_ct_required",
 		.deflt = "false",

diff --git a/tls/ttls.h b/tls/ttls.h
@@ -540,7 +540,8 @@ typedef struct tls_handshake_t TlsHandshake;
  * @nb_zero	-  # of 0-length encrypted messages;
  * @client_auth	- flag for client authentication (client side only);
  * @hostname	- expected peer CN for verification (and SNI if available);
- * @vhost	- vhost selected by SNI (TfwVhost)
+ * @vhost	- vhost selected by SNI (TfwVhost*), NULL if no SNI extension
+ *		  had been sent by the client
  */
 typedef struct ttls_context {
 	struct sock		*sk;