Mailing List Archive

Re: base64 blob.decode() default error handling
On 11/4/19 03:32, pyunyh wrote:
> Hi,
>
> In order to validate client supplied base64 encoded string I have a
> VCL something like the following.
>
> if (blob.length(blob.decode(BASE64, 0, req.http.Sec-WebSocket-Key)) != 16) {
> return (synth(400, "Bad Request"));
> }
>
> If the Sec-WebSocket-Key header has a badly encoded string VCL will
> generate a "503 VCL failed" response. It's somewhat hard to generate
> a 400 repsonse in this case, since very little thing can be done in
> vcl_synth(). As you know resp.status can be overriden as well as
> resp.reason in vcl_synth() but all state tracking made so far is
> completely lost in vcl_synth() so there is no way to know where the
> vcl_synth() is called from which context.
> What is proper way to handle this case?
> If there is a way to override arguments passed to synth() with
> return(fail) during base64 decoding this will address the issue.
> I think this can be easily solved with inline C but it it would be
> better to have a VCL method as vcc_allow_inline_c is disabled by
> default.

@pyunyh, I'm moving this to varnish-misc, which is the more appropriate
list for a user question like this.

This is a general issue with VCL failure -- in vcl_synth, you can tell
from resp.reason=="VCL failed" that it was VCL failure that you got you
there, but there is little to no information about what caused the
failure, so it can be difficult to impossible to implement specific
error handling.

I can see why you need this for the vmod blob use case. But I would
suggest that the Varnish dev team consider a general solution for
identifying the cause of a VCL failure, rather than special-casing a
solution for the vmod.

@pyunyh, as a workaround you might go to the old trick of using a
request header as a variable:

# assuming that req.http.X-Sec-WebSocket-Key-Validated is not set
# at this point, or not set to "true"
if (blob.length(blob.decode(BASE64, 0, req.http.Sec-WebSocket-Key)) != 16) {
return (synth(400, "Bad Request"));
}
set req.http.X-Sec-WebSocket-Key-Validated = "true";

# now in vcl_synth:
if (resp.reason == "VCL failed"
&& req.http.X-Sec-WebSocket-Key-Validated != "true") {
set resp.status = 400;
set resp.reason = "Bad Request";
}


HTH,
Geoff
--
** * * UPLEX - Nils Goroll Systemoptimierung

Scheffelstraße 32
22301 Hamburg

Tel +49 40 2880 5731
Mob +49 176 636 90917
Fax +49 40 42949753

http://uplex.de
Re: base64 blob.decode() default error handling [ In reply to ]
On Mon, Nov 04, 2019 at 01:51:25PM +0100, Geoff Simmons wrote:

[...]

>
> @pyunyh, I'm moving this to varnish-misc, which is the more appropriate
> list for a user question like this.
>

Thanks for correcting me. Subscribed to varnish-misc@

> This is a general issue with VCL failure -- in vcl_synth, you can tell
> from resp.reason=="VCL failed" that it was VCL failure that you got you
> there, but there is little to no information about what caused the
> failure, so it can be difficult to impossible to implement specific
> error handling.
>
> I can see why you need this for the vmod blob use case. But I would
> suggest that the Varnish dev team consider a general solution for
> identifying the cause of a VCL failure, rather than special-casing a
> solution for the vmod.
>
> @pyunyh, as a workaround you might go to the old trick of using a
> request header as a variable:
>
> # assuming that req.http.X-Sec-WebSocket-Key-Validated is not set
> # at this point, or not set to "true"
> if (blob.length(blob.decode(BASE64, 0, req.http.Sec-WebSocket-Key)) != 16) {
> return (synth(400, "Bad Request"));
> }
> set req.http.X-Sec-WebSocket-Key-Validated = "true";
>
> # now in vcl_synth:
> if (resp.reason == "VCL failed"
> && req.http.X-Sec-WebSocket-Key-Validated != "true") {
> set resp.status = 400;
> set resp.reason = "Bad Request";
> }

That approach was the first one in my experiments and it didn't
work. It seems there are two paths here. One is voluntary synth()
call in VCL and the other is indirect call via unconditional
'return (fail)'. Header variable tricks works for the former case
only. If there was an decoding or memory related error in blob it
appears to trigger internal 'return (fail)' which in turn have an
effect of resetting all header variables with std.rollback(). So
there are *NO* header variables available in this case.

The half-working and ugly code I have looks like the following.

sub vcl_synth {

...
if (resp.status == 503 && resp.reason == "VCL failed") {
if (req.method == "GET" &&
req.proto == "HTTP/1.1" &&
req.http.Upgrade &&
... other conditions ...
req.http.Sec-WebSocket-Key &&
req.url ~ "^/path/to/WebSocket") {
set resp.status = 400;
}
}
...
}

Unfortunately this does not work as req.url points to original URL
such that it nullifies URL rewrting rules that were applied.

Thanks.
_______________________________________________
varnish-misc mailing list
varnish-misc@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-misc