caddyserver / caddy

Fast and extensible multi-platform HTTP/1-2-3 web server with automatic HTTPS
https://caddyserver.com
Apache License 2.0
55.48k stars 3.91k forks source link

Client_ip not merged as remote_ip used to in "not" expression #6349

Closed fmarchalemisys closed 1 month ago

fmarchalemisys commented 1 month ago

Hi,

The following host config fragment only works with the IP addresses listed in the first client_ip. Subsequent client_ip are denied access on Caddy 2.8.0:

@webhook {
    path "/webhook"
    not {
        # prod servers
        client_ip 51.138.37.238 20.54.89.16 13.80.70.181 13.80.71.223 13.79.28.70 40.127.253.112/28 51.105.129.192/28
        # demo servers
        client_ip 20.50.240.57 40.74.20.78 94.70.170.65 94.70.174.36 94.70.255.73 94.70.248.18 83.235.24.226 20.13.195.185
        # our own connection for debug purposes
        client_ip 1.2.3.4
    }
}
handle @webhook {
    abort
}

It used to work with remote_ip:

@webhook {
    path "/webhook"
    not {
        # prod servers
        remote_ip forwarded 51.138.37.238 20.54.89.16 13.80.70.181 13.80.71.223 13.79.28.70 40.127.253.112/28 51.105.129.192/28
        # demo servers
        remote_ip 20.50.240.57 40.74.20.78 94.70.170.65 94.70.174.36 94.70.255.73 94.70.248.18 83.235.24.226 20.13.195.185
        # our own connection for debug purposes
        remote_ip 1.2.3.4
    }
}
handle @webhook {
    abort
}

Moving client_ip 1.2.3.4 to the first line of the not expression allows that IP address but not the others.

Trusted proxies are set:

{
    servers {
        trusted_proxies static 173.245.48.0/20 103.21.244.0/22 103.22.200.0/22 103.31.4.0/22 141.101.64.0/18
    }
}

Is there some way around this?

francislavoie commented 1 month ago

Ah whoops, my bad. It's a regression on the matcher parsing. I did a cleanup on code style and accidentally broke the merging.

For now you could write it like this:

        client_ip \
            # prod servers \
            51.138.37.238 20.54.89.16 13.80.70.181 13.80.71.223 13.79.28.70 40.127.253.112/28 51.105.129.192/28 \
            # demo servers \
            20.50.240.57 40.74.20.78 94.70.170.65 94.70.174.36 94.70.255.73 94.70.248.18 83.235.24.226 20.13.195.185 \
            # our own connection for debug purposes \
            1.2.3.4

Kinda goofy, but basically just escaping the newlines to continue specifying more IP ranges on the next line.

I'll fix it for the next release though.

fmarchalemisys commented 1 month ago

Thank you @francislavoie.

This seems to work too (if I'm not mistaken):

@webhook {
    path "/webhook"
    # prod servers
    not remote_ip forwarded 51.138.37.238 20.54.89.16 13.80.70.181 13.80.71.223 13.79.28.70 40.127.253.112/28 51.105.129.192/28
    # demo servers
    not remote_ip 20.50.240.57 40.74.20.78 94.70.170.65 94.70.174.36 94.70.255.73 94.70.248.18 83.235.24.226 20.13.195.185
    # our own connection for debug purposes
    not remote_ip 1.2.3.4
}
francislavoie commented 1 month ago

Yeah that works too, as per https://caddyserver.com/docs/caddyfile/matchers#examples-4 having multiple not in a row is like saying not remote_ip AND not remote_ip AND not remote_ip.