Skip to content
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.

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

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

Closed
fmarchalemisys opened this issue May 30, 2024 · 3 comments · Fixed by #6350
Closed

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

fmarchalemisys opened this issue May 30, 2024 · 3 comments · Fixed by #6350

Comments

@fmarchalemisys
Copy link

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
Copy link
Member

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
Copy link
Author

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
Copy link
Member

francislavoie commented May 30, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants