Re: [PATCH] Accept IP addresses in server certificate SANs

On Fri, 2021-12-17 at 16:54 +0900, Kyotaro Horiguchi wrote:
> Sorry for the silly mistake.
>
> At Fri, 17 Dec 2021 15:40:10 +0900 (JST), Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> wrote in
> > > NSS departs slightly from the spec and will additionally try to match
> > > an IP address against the CN, but only if there are no iPAddresses in
> > > the SAN. It roughly matches the logic for DNS names.
> >
> > OpenSSL seems different. X509_check_host() tries SAN then CN iff SAN
> > doesn't exist. X509_check_ip() tries SAN and completely ignores
> > iPAdress and CN.
>
> OpenSSL seems different. X509_check_host() tries SAN then CN iff SAN
> doesn't exist. X509_check_ip() tries iPAddress and completely ignores
> CN.

Right.

On Fri, 2021-12-17 at 15:40 +0900, Kyotaro Horiguchi wrote:
>
> > + * pg_inet_net_pton() will accept CIDR masks, which we don't want to
> > > + * match, so skip the comparison if the host string contains a slash.
> > > + */
> > > + if (!strchr(host, '/')
> > > + && pg_inet_net_pton(PGSQL_AF_INET6, host, addr, -1) == 128)
> > >
> > > If a cidr is given, pg_inet_net_pton returns a number less than 128 so
> > > we don't need to check '/' explicity? (I'm not sure '/128' is
> > > sensible but doesn't harm..)
> >
> > Personally I think that, if someone wants your libpq to connect to a
> > server with a hostname of "some:ipv6::address/128", then they are
> > trying to pull something (evading a poorly coded blocklist, perhaps?)
> > and we should not allow that to match an IP. Thoughts?
>
> If the client could connect to the network-address, it could be said
> that we can assume that address is the name:p Just kidding.
>
> As the name suggests, the function reads a network address. And the
> only user is network_in(). I think we should provide pg_inet_pton()
> instead of abusing pg_inet_net_pton(). inet_net_pton_*() functions
> can be modified to reject /cidr part without regression so we are able
> to have pg_inet_pton() with a small amount of change.
>
> - inet_net_pton_ipv4(const char *src, u_char *dst)
> + inet_net_pton_ipv4_internal(const char *src, u_char *dst, bool netaddr)
>
> + inet_net_pton_ipv4(const char *src, u_char *dst)
> (calls inet_net_pton_ipv4_internal(src, dst, true))
> + inet_pton_ipv4(const char *src, u_char *dst)
> (calls inet_net_pton_ipv4_internal(src, dst, false))

Sounds good, I will make that change. Thanks for the feedback!

--Jacob