On Thu, Aug 01, 2013 at 02:03:14AM +0100, Federico Schwindt wrote:
> On Wed, Jul 31, 2013 at 2:46 PM, Lasse Karstensen <
> [email protected]> wrote:
> > I've extended the std vmod to include an ip() method, which
> > converts a string into VCL IP. The result can be used for
> > matching against a VCL ACL.
> A few comments:
> - rp leaks if WS_Reserve() fails.
> - WS_Reserve() is cheaper that getaddrinfo(), so I'd check first if there
> is space and then do the getaddrinfo() call. That'd simplify the error path
> too.
> - Missing CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC).
> - You could just check for getaddrinfo() != 0 instead of using s = .. since
> it's not used anywhere else.
Hi Federico.
Thank you for taking the time to review this.
I've implemented the changes you proposed. Updated (full) patch is attached.
> - Using VCL_IP for the fallback parameter restricts what you can use to
> client.ip or server.ip. This might or might not be a problem.
> I wrote a similar function a while ago that was using a STRING parameter as
> suggested by Tollef. Not sure if this is still required.
Yes, we discussed this for a bit in the office.
I don't have strong opinions on either side.
You can of course nest them to get an arbitrary fallback:
std.ip(req.http.X-Forwarded-For, std.ip("127.255.255.255"));
The error handling when the fallback conversion fails doesn't seem to have
any obvious solution. If the user gets to pick a fallback by him/herself,
then at least they know clearly what to check for.
--
With regards,
Lasse Karstensen
Varnish Software AS
diff --git a/bin/varnishtest/tests/m00100.vtc b/bin/varnishtest/tests/m00100.vtc
new file mode 100644
index 0000000..8ed2405
--- /dev/null
+++ b/bin/varnishtest/tests/m00100.vtc
@@ -0,0 +1,32 @@
+varnishtest "test vmod_std.ip conversion"
+
+server s1 {
+ rxreq
+ txresp
+} -start
+
+varnish v1 -vcl+backend {
+ acl fooacl {
+ "192.168.2.11";
+ }
+ import std from "${topbuild}/lib/libvmod_std/.libs/libvmod_std.so" ;
+ sub vcl_recv {
+ if (std.ip(req.http.X-Forwarded-For, server.ip) !~ fooacl) {
+ return(error(403, "Forbidden"));
+ }
+ }
+ sub vcl_deliver {
+ set resp.http.x-ip = std.ip("192.168.2.10", server.ip);
+ set resp.http.x-badip = std.ip("gibberish", server.ip);
+ set resp.http.x-ipv6 = std.ip("2001:db8::10", server.ip);
+ }
+} -start
+
+client c1 {
+ txreq -url "/" -hdr "X-Forwarded-For: 192.168.2.11"
+ rxresp
+ expect resp.status == 200
+ expect resp.http.x-ip == "192.168.2.10"
+ expect resp.http.x-badip == "127.0.0.1"
+ expect resp.http.x-ipv6 == "2001:db8::10"
+} -run
diff --git a/doc/sphinx/reference/vmod_std.rst b/doc/sphinx/reference/vmod_std.rst
index f8c4456..3cf0a3c 100644
--- a/doc/sphinx/reference/vmod_std.rst
+++ b/doc/sphinx/reference/vmod_std.rst
@@ -139,6 +139,19 @@ Description
Example
if (std.integer(beresp.http.x-foo, 0) > 5) { ... }
+ip
+--
+Prototype
+ ip(STRING s, IP fallback)
+Return value
+ IP
+Description
+ Converts the string *s* containing an IPv4 or IPv6 address to an IP.
+ If *s* fails to parse, *fallback* will be used.
+Example
+ if (std.ip(req.http.X-Forwarded-For, server.ip) !~ myacl) { return(error(403, "Forbidden")); }
+
+
collect
-------
Prototype
diff --git a/lib/libvmod_std/vmod.vcc b/lib/libvmod_std/vmod.vcc
index 3238164..1f4e33e 100644
--- a/lib/libvmod_std/vmod.vcc
+++ b/lib/libvmod_std/vmod.vcc
@@ -36,3 +36,4 @@ Function STRING fileread(PRIV_CALL, STRING)
Function DURATION duration(STRING, DURATION)
Function INT integer(STRING, INT)
Function VOID collect(HEADER)
+Function IP ip(STRING, IP)
diff --git a/lib/libvmod_std/vmod_std_conversions.c b/lib/libvmod_std/vmod_std_conversions.c
index c763aa3..539a021 100644
--- a/lib/libvmod_std/vmod_std_conversions.c
+++ b/lib/libvmod_std/vmod_std_conversions.c
@@ -38,6 +38,13 @@
#include "vrt.h"
#include "vcc_if.h"
+#include <sys/socket.h>
+/* Clang 3.0 refuses to parse the gaicb struct in netdb. Works with 3.2. */
+#if __clang_major__ == 3 && __clang_minor__ == 0
+#undef __USE_GNU
+#endif
+#include <netdb.h>
+
VCL_DURATION __match_proto__()
vmod_duration(const struct vrt_ctx *ctx, const char *p, VCL_DURATION d)
{
@@ -117,3 +124,36 @@ vmod_integer(const struct vrt_ctx *ctx, const char *p, VCL_INT i)
return (r);
}
+
+
+VCL_IP __match_proto__()
+vmod_ip(const struct vrt_ctx *ctx, VCL_STRING ipstring, VCL_IP fallback) {
+ struct addrinfo hints;
+ struct addrinfo *rp;
+ struct sockaddr_storage *p;
+ unsigned u;
+
+ CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
+
+ u = WS_Reserve(ctx->ws, sizeof(struct sockaddr_storage));
+ if (u < sizeof(struct sockaddr_storage)) {
+ WS_Release(ctx->ws, 0);
+ return(fallback);
+ }
+
+ memset(&hints, 0, sizeof(struct addrinfo));
+ hints.ai_family = AF_UNSPEC;
+ hints.ai_flags = AI_NUMERICHOST; // Don't attempt DNS resolution.
+ if (getaddrinfo(ipstring, NULL, &hints, &rp) != 0) {
+ WS_Release(ctx->ws, 0);
+ return(fallback);
+ }
+
+ p = (struct sockaddr_storage *) (void*)ctx->ws->f;
+ memcpy(p, rp->ai_addr, sizeof(struct sockaddr_storage));
+ freeaddrinfo(rp);
+
+ WS_Release(ctx->ws, sizeof(struct sockaddr_storage));
+ return(p);
+}
+
_______________________________________________
varnish-dev mailing list
[email protected]
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev
