Re: Support for NSS as a libpq TLS backend

On Tue, Nov 16, 2021 at 1:26 PM Joshua Brindle
<joshua.brindle@crunchydata.com> wrote:
>
> On Tue, Nov 16, 2021 at 9:45 AM Joshua Brindle
> <joshua.brindle@crunchydata.com> wrote:
> >
> > On Mon, Nov 15, 2021 at 5:37 PM Joshua Brindle
> > <joshua.brindle@crunchydata.com> wrote:
> > >
> > > On Mon, Nov 15, 2021 at 4:44 PM Daniel Gustafsson <daniel@yesql.se> wrote:
> > > >
> > > > > On 15 Nov 2021, at 20:51, Joshua Brindle <joshua.brindle@crunchydata.com> wrote:
> > > >
> > > > > Apologies for the delay, this didn't go to my inbox and I missed it on list.
> > > > >
> > > > > The bitcode generation is still broken, this time for nspr.h:
> > > >
> > > > Interesting, I am unable to replicate that in my tree but I'll investigate
> > > > further tomorrow using your Dockerfile.  For the sake of testing, does
> > > > compilation pass for you in the same place without using --with-llvm?
> > > >
> > >
> > > Yes, it builds and check-world passes. I'll continue testing with this
> > > build. Thank you.
> >
> > The previous Dockerfile had some issues due to a hasty port from RHEL
> > to Fedora, attached is one that works with your patchset, llvm
> > currently disabled, and the llvm deps removed.
> >
> > The service file is also attached since it's referenced in the
> > Dockerfile and you'd have had to reproduce it.
> >
> > After building, run with:
> > docker run --name pg-test -p 5432:5432 --cap-add=SYS_ADMIN -v
> > /sys/fs/cgroup:/sys/fs/cgroup:ro -d <final docker hash>
>
> I think there it a typo in the docs here that prevents them from
> building (this diff seems to fix it):
>
> diff --git a/doc/src/sgml/pgcrypto.sgml b/doc/src/sgml/pgcrypto.sgml
> index 56b73e033c..844aa31e86 100644
> --- a/doc/src/sgml/pgcrypto.sgml
> +++ b/doc/src/sgml/pgcrypto.sgml
> @@ -767,7 +767,7 @@ pgp_sym_encrypt(data, psw, 'compress-algo=1,
> cipher-algo=aes256')
>     <para>
>      Which cipher algorithm to use.  <literal>cast5</literal> is only available
>      if <productname>PostgreSQL</productname> was built with
> -    <productname>OpenSSL</productame>.
> +    <productname>OpenSSL</productname>.
>     </para>
>  <literallayout>
>  Values: bf, aes128, aes192, aes256, 3des, cast5

After a bit more testing, the server is up and running with an nss
database but before configuring the client database I tried connecting
and got a segfault:

#0  PR_Write (fd=0x0, buf=0x141ba60, amount=84) at
io/../../.././nspr/pr/src/io/priometh.c:114
#1  0x00007ff33dfdc62f in pgtls_write (conn=0x13cecb0, ptr=0x141ba60,
len=84) at fe-secure-nss.c:583
#2  0x00007ff33dfd6e18 in pqsecure_write (conn=0x13cecb0,
ptr=0x141ba60, len=84) at fe-secure.c:295
#3  0x00007ff33dfd04dc in pqSendSome (conn=0x13cecb0, len=84) at fe-misc.c:834
#4  0x00007ff33dfd06c8 in pqFlush (conn=0x13cecb0) at fe-misc.c:972
#5  0x00007ff33dfc257c in pqPacketSend (conn=0x13cecb0, pack_type=0
'\000', buf=0x1414c60, buf_len=80) at fe-connect.c:4619
#6  0x00007ff33dfbfadd in PQconnectPoll (conn=0x13cecb0) at fe-connect.c:2986
#7  0x00007ff33dfbe55c in connectDBComplete (conn=0x13cecb0) at
fe-connect.c:2218
#8  0x00007ff33dfbbaef in PQconnectdbParams (keywords=0x1427d10,
values=0x1427e60, expand_dbname=1) at fe-connect.c:668
#9  0x000000000043ebc7 in main (argc=2, argv=0x7ffdccd0e2f8) at startup.c:273

It looks like the ssl connection falls through to attempt a non-ssl
connection but at some point conn->ssl_in_use gets set to true,
despite pr_fd and nss_context being null.

This patch fixes the segfault but I suspect is not the correct fix,
due to the error when connecting saying "Success":

--- a/src/interfaces/libpq/fe-secure-nss.c
+++ b/src/interfaces/libpq/fe-secure-nss.c
@@ -498,6 +498,11 @@ pgtls_read(PGconn *conn, void *ptr, size_t len)
         * for closed connections, while -1 indicates an error within
the ongoing
         * connection.
         */
+       if (!conn->pr_fd) {
+               SOCK_ERRNO_SET(read_errno);
+               return -1;
+       }
+
        nread = PR_Recv(conn->pr_fd, ptr, len, 0, PR_INTERVAL_NO_WAIT);

        if (nread == 0)
@@ -580,6 +585,11 @@ pgtls_write(PGconn *conn, const void *ptr, size_t len)
        PRErrorCode status;
        int                     write_errno = 0;

+       if (!conn->pr_fd) {
+               SOCK_ERRNO_SET(write_errno);
+               return -1;
+       }
+
        n = PR_Write(conn->pr_fd, ptr, len);

        if (n < 0)