PATCH: Disallow a netmask of zero unless the IP is also all zeroes

Started by Greg Sabino Mullaneabout 1 year ago5 messageshackers
Jump to latest
#1Greg Sabino Mullane
greg@turnstep.com

I ran into this alarming mistake again the other day. Luckily it was on a
dev system. Someone sees an entry in a pg_hba.conf that looks like this:

host all all 0.0.0.0/0 md5

They are gobsmacked when they learn this means to let everyone in. So they
fix it by adding new entries that look like this:

host all all 10.2.55.4/0 md5
host all all 10.2.55.5/0 md5
host all all 10.2.55.6/0 md5

It should, of course, be:
host all all 10.2.55.4/32 md5

I say "of course" but few people (even tech ones) know the distinction.
(Nor should they have to! But that's for a nearby thread). This patch aims
to prevent this very bad footgun by only allowing a /0 if the IP consists
of only zeroes. It works for ipv4 and ipv6.

Cheers,
Greg

--
Crunchy Data - https://www.crunchydata.com
Enterprise Postgres Software Products & Tech Support

Attachments:

0001-Only-allow-a-CIDR-mask-of-zero-if-the-IP-contains-only-zeroes.patchapplication/x-patch; name=0001-Only-allow-a-CIDR-mask-of-zero-if-the-IP-contains-only-zeroes.patchDownload+20-1
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Greg Sabino Mullane (#1)
Re: PATCH: Disallow a netmask of zero unless the IP is also all zeroes

Greg Sabino Mullane <htamfids@gmail.com> writes:

I say "of course" but few people (even tech ones) know the distinction.
(Nor should they have to! But that's for a nearby thread). This patch aims
to prevent this very bad footgun by only allowing a /0 if the IP consists
of only zeroes. It works for ipv4 and ipv6.

More generally, should we reject if the netmask causes *any* nonzero
IP bits to be ignored? Our CIDR type already imposes that rule:

regression=# select '1.2.3.4/24'::cidr;
ERROR: invalid cidr value: "1.2.3.4/24"
LINE 1: select '1.2.3.4/24'::cidr;
^
DETAIL: Value has bits set to right of mask.

I'm a bit distressed to realize that hba.c isn't using cidr_in.
Maybe we should try to share code instead of duplicating yet more.

regards, tom lane

#3Daniel Gustafsson
daniel@yesql.se
In reply to: Tom Lane (#2)
Re: PATCH: Disallow a netmask of zero unless the IP is also all zeroes

On 11 Feb 2025, at 21:25, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I'm a bit distressed to realize that hba.c isn't using cidr_in.
Maybe we should try to share code instead of duplicating yet more.

+1. I have a note along these lines on my never-shrinking TODO, I think it
would be great if we took a stab at that.

--
Daniel Gustafsson

#4Greg Sabino Mullane
greg@turnstep.com
In reply to: Tom Lane (#2)
Re: PATCH: Disallow a netmask of zero unless the IP is also all zeroes

On Tue, Feb 11, 2025 at 3:25 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

More generally, should we reject if the netmask causes *any* nonzero
IP bits to be ignored? Our CIDR type already imposes that rule:

Yeah, I like that idea a lot. That's a great DETAIL message.

Cheers,
Greg

--
Crunchy Data - https://www.crunchydata.com
Enterprise Postgres Software Products & Tech Support

#5Andreas Karlsson
andreas.karlsson@percona.com
In reply to: Tom Lane (#2)
Re: PATCH: Disallow a netmask of zero unless the IP is also all zeroes

On 2/11/25 9:25 PM, Tom Lane wrote:

Greg Sabino Mullane <htamfids@gmail.com> writes:

I say "of course" but few people (even tech ones) know the distinction.
(Nor should they have to! But that's for a nearby thread). This patch aims
to prevent this very bad footgun by only allowing a /0 if the IP consists
of only zeroes. It works for ipv4 and ipv6.

More generally, should we reject if the netmask causes *any* nonzero
IP bits to be ignored? Our CIDR type already imposes that rule:

+1 From me too. I think we should fix the general issue rather than
special casing /0.

Andreas