Skip to content

Improve varnish config, add support for URIBAN by regex #495

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Rade333 merged 8 commits into from
Aug 26, 2025
Merged

Improve varnish config, add support for URIBAN by regex #495

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ead552d
(drupal/varnish) Don't use cookies to vary cache for static files
MarttiR Aug 15, 2025
11cbd62
(drupal/varnish) Fix URIBAN behavior, add support for URIBAN by regex
MarttiR Aug 15, 2025
b8ef26d
(frontend/varnish) Fix URIBAN behavior, add support for URIBAN by regex
MarttiR Aug 15, 2025
9e62b7f
(drupal/varnish) Sort static file extensions alphabetically
MarttiR Aug 18, 2025
6859a11
(drupal/varnish) Move Accept-Encoding block before the static files b…
MarttiR Aug 18, 2025
a3e28a4
(drupal/varnish) Use non-capturing groups in regexes
MarttiR Aug 25, 2025
2294948
(frontend/varnish) Use non-capturing groups in regexes
MarttiR Aug 25, 2025
cba1254
(drupal/varnish) Simplify session cookie regex
MarttiR Aug 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 28 additions & 13 deletions drupal/templates/varnish-configmap-vcl.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,8 +304,8 @@ data:
return(pipe);
}

# Only allow BAN requests from IP addresses in the 'purge' ACL.
if (req.method == "BAN" || req.method == "URIBAN") {
# Only allow BAN requests from IP addressees in the 'internal' ACL.
if (req.method == "BAN") {
# Admin port is only exposed to internal network
if (!client.ip ~ purge) {
return (synth(403, "Not allowed."));
Expand All @@ -319,11 +319,30 @@ data:
elseif (req.http.Cache-Tags) {
ban("obj.http.Cache-Tags ~ " + req.http.Cache-Tags);
}
elseif (req.method == "URIBAN") {
ban("req.http.host == " + req.http.host + " && req.url == " + req.url);
else {
# If there are no cache tags headers in a BAN request,
# it is a bad request, so indicate that to the client.
return (synth(400, "Cache tags headers not present."));
}
# Throw a synthetic page so the request won't go to the backend.
return (synth(200, "Ban added."));
}

# Only allow URIBAN requests from IP addressees in the 'internal' ACL.
if (req.method == "URIBAN") {
# Admin port is only exposed to internal network
if (!client.ip ~ purge) {
return (synth(403, "Not allowed."));
Copy link
Copy Markdown
Contributor

@ragnarkurmwunder ragnarkurmwunder Aug 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider "IP not allowed".
Reasoning: suppose someone gets the error, and has no clue of this code here, it could be a guesswork why the error.

Copy link
Copy Markdown
Contributor Author

@MarttiR MarttiR Aug 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK, the message set here is only logged in varnishlog, not transmitted via HTTP.

}

# If x-url-invalidate-pattern header is present,
# use it to match URLs in stored objects. (ban by regex pattern)
if (req.http.x-url-invalidate-pattern) {
ban("obj.http.x-url ~ " + req.http.x-url-invalidate-pattern);
Copy link

Copilot AI Aug 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ban statements reference obj.http.x-url but there's no evidence that this header is set during cache storage. The original code used req.url for URIBAN matching. If x-url is not properly set on cached objects, these ban statements will not match any cached entries.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

@MarttiR MarttiR Aug 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

obj.http.x-url is stored in the cached objects here

}
# Without pattern, ban by matching host and URL exactly.
else {
return (synth(403, "Cache tags headers not present."));
ban("obj.http.host == " + req.http.host + " && obj.http.x-url == " + req.url);
Comment thread
MarttiR marked this conversation as resolved.
}
# Throw a synthetic page so the request won't go to the backend.
return (synth(200, "Ban added."));
Expand Down Expand Up @@ -420,8 +439,9 @@ data:
return (pass);
}

if (req.url ~ "\.(png|gif|jpg|tif|tiff|ico|webp|swf|css|js|pdf|doc|xls|ppt|zip)(\?.*)?$") {
// Forcing a lookup with static file requests
if (req.url ~ "\.(png|gif|jpg|svg|tif|tiff|ico|webp|swf|css|js|pdf|doc|xls|ppt|zip|woff|eot|ttf|bmp|bz2)$") {
Copy link
Copy Markdown
Contributor

@ragnarkurmwunder ragnarkurmwunder Aug 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sort the extensions alphabetically?

Copy link
Copy Markdown
Contributor Author

@MarttiR MarttiR Aug 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 9e62b7f.

Also noticed that the Accept-Encoding block tried to skip compressing already compressed files, but as there is significant overlap with the static files handling and the block was after the static files block, it was actually not doing anything for many types of static files. 6859a11 moves it to before the static files block.

# Static file request do not vary on cookies
Copy link
Copy Markdown
Contributor

@ragnarkurmwunder ragnarkurmwunder Aug 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I like this.

unset req.http.Cookie;
return (hash);
}

Expand All @@ -445,12 +465,7 @@ data:
}

if (req.http.Cookie) {
if (req.url ~ "\.(png|gif|jpg|svg|tif|tiff|ico|webp|swf|css|js|pdf|doc|xls|ppt|zip|woff|eot|ttf|bmp|bz2)$") {
# Static file request do not vary on cookies
unset req.http.Cookie;
return (hash);
}
elseif (req.http.Cookie ~ "(SESS[a-z0-9]+|SSESS[a-z0-9]+)") {
if (req.http.Cookie ~ "(SESS[a-z0-9]+|SSESS[a-z0-9]+)") {
Copy link
Copy Markdown
Contributor

@ragnarkurmwunder ragnarkurmwunder Aug 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

simplify?
``"S?SESS[a-z0-9]+="`

Copy link
Copy Markdown
Contributor Author

@MarttiR MarttiR Aug 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, done in cba1254.

# Authenticated users should not be cached
return (pass);
}
Expand Down
60 changes: 39 additions & 21 deletions frontend/templates/varnish-configmap-vcl.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,45 @@ data:
return (synth(400));
}

# Only allow BAN requests from IP addressees in the 'internal' ACL.
if (req.method == "BAN") {
# Admin port is only exposed to internal network
if (!client.ip ~ internal) {
return (synth(403, "Not allowed."));
}

# Logic for the ban, using the cache tags headers.
if (req.http.Cache-Tags) {
ban("obj.http.Cache-Tags ~ " + req.http.Cache-Tags);
}
else {
# If there are no cache tags headers in a BAN request,
# it is a bad request, so indicate that to the client.
return (synth(400, "Cache tags headers not present."));
}
# Throw a synthetic page so the request won't go to the backend.
return (synth(200, "Ban added."));
}

# Only allow URIBAN requests from IP addressees in the 'internal' ACL.
if (req.method == "URIBAN") {
# Admin port is only exposed to internal network
if (!client.ip ~ internal) {
return (synth(403, "Not allowed."));
}

# If x-url-invalidate-pattern header is present,
# use it to match URLs in stored objects. (ban by regex pattern)
if (req.http.x-url-invalidate-pattern) {
ban("obj.http.x-url ~ " + req.http.x-url-invalidate-pattern);
Comment thread
MarttiR marked this conversation as resolved.
}
else {
ban("obj.http.host == " + req.http.host + " && obj.http.x-url == " + req.url);
Comment thread
MarttiR marked this conversation as resolved.
}
# Throw a synthetic page so the request won't go to the backend.
return (synth(200, "Ban added."));
}

# Match upstream allowlist to supply headers containing real ip
if (client.ip ~ upstream_proxy && req.http.X-Envoy-External-Address) {
set req.http.X-Forwarded-For = req.http.X-Envoy-External-Address;
Expand Down Expand Up @@ -100,27 +139,6 @@ data:
return (pass);
}

# Only allow BAN requests from IP addresses in the 'purge' ACL.
if (req.method == "BAN" || req.method == "URIBAN") {
# Admin port is only exposed to internal network
if (!client.ip ~ internal) {
return (synth(403, "Not allowed."));
}

# Logic for the ban, using the cache tags headers.
if (req.http.Cache-Tags) {
ban("obj.http.Cache-Tags ~ " + req.http.Cache-Tags);
}
elseif (req.method == "URIBAN") {
ban("req.http.host == " + req.http.host + " && req.url == " + req.url);
}
else {
return (synth(403, "Cache tags headers not present."));
}
# Throw a synthetic page so the request won't go to the backend.
return (synth(200, "Ban added."));
}

# Implementing websocket support (https://www.varnish-cache.org/docs/4.0/users-guide/vcl-example-websockets.html)
if (req.http.Upgrade ~ "(?i)websocket") {
return (pipe);
Expand Down
Loading