Message-ID: <b2f8b8e9-1c9a-57b2-0c8f-1f3c7fbdb5e2@enterprisedb.com>
Date: Wed, 30 Mar 2022 13:37:31 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.7.0
Subject: Re: [PATCH] Accept IP addresses in server certificate SANs
Content-Language: en-US
To: Jacob Champion <pchampion@vmware.com>, "daniel@yesql.se"
 <daniel@yesql.se>, "tgl@sss.pgh.pa.us" <tgl@sss.pgh.pa.us>
Cc: "stark@mit.edu" <stark@mit.edu>,
 "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>,
 "horikyota.ntt@gmail.com" <horikyota.ntt@gmail.com>
References: <20220318.163857.1357392368287571138.horikyota.ntt@gmail.com>
 <20220322.133202.1699013732440256188.horikyota.ntt@gmail.com>
 <5878cc4f261dfac70999be7c10d80e898449e388.camel@vmware.com>
 <20220323.142007.1816604521514641243.horikyota.ntt@gmail.com>
 <2da38935f528904f62dd5846767dc53585d8a309.camel@vmware.com>
 <552429.1648415967@sss.pgh.pa.us>
 <3F1A8748-DEF8-454C-B7ED-F536CCF7F115@yesql.se>
 <5EA8D400-9705-463C-B30F-E7A78EE5C8FB@yesql.se>
 <27dd89aa5bd8fac8fdfdbd33dd460422bcb77d6b.camel@vmware.com>
From: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
In-Reply-To: <27dd89aa5bd8fac8fdfdbd33dd460422bcb77d6b.camel@vmware.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/b2f8b8e9-1c9a-57b2-0c8f-1f3c7fbdb5e2%40enterprisedb.com>
Precedence: bulk

On 28.03.22 22:21, Jacob Champion wrote:
> On Mon, 2022-03-28 at 11:17 +0200, Daniel Gustafsson wrote:
>> Fixing up the switch_server_cert() calls and using default_ssl_connstr makes
>> the test pass for me.  The required fixes are in the supplied 0004 diff, I kept
>> them separate to allow the original author to incorporate them without having
>> to dig them out to see what changed (named to match the git format-patch output
>> since I think the CFBot just applies the patches in alphabetical order).
> 
> Thanks! Those changes look good to me; I've folded them into v11. This
> is rebased on a newer HEAD so it should fix the apply failures that
> Greg pointed out.

I'm not happy about how inet_net_pton.o is repurposed here.  That code 
is clearly meant to support backend data types with specifically.  Code like

+ /*
+  * 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.
+  */

indicates that we are fighting against the API.  Also, if someone ever 
wants to change how those backend data types work, we then have to check 
a bunch of other code as well.

I think we should be using inet_ntop()/inet_pton() directly here.  We 
can throw substitute implementations into libpgport if necessary, that's 
not so difficult.