Re: Password identifiers, protocol aging and SCRAM protocol

From: Magnus Hagander <magnus(at)hagander(dot)net>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Robert Haas <robertmhaas(at)gmail(dot)com>, Julian Markwort <julian(dot)markwort(at)uni-muenster(dot)de>, Stephen Frost <sfrost(at)snowman(dot)net>, David Steele <david(at)pgmasters(dot)net>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>, Valery Popov <v(dot)popov(at)postgrespro(dot)ru>
Subject: Re: Password identifiers, protocol aging and SCRAM protocol
Date: 2016-07-05 08:50:07
Message-ID: CABUevExtny+0j7jMV=-66y-_A9iA13zyYWszv9DEhiGtsA0tmw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Jul 5, 2016 at 10:06 AM, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
wrote:

> On Mon, Jul 4, 2016 at 12:54 PM, Michael Paquier
> <michael(dot)paquier(at)gmail(dot)com> wrote:
> > As I am coming back into that, I would as well suggest do the
> > following, that the current set of patches is clearly missing:
> > - Put the HMAC infrastructure stuff of pgcrypto into src/common/. It
> > is a bit a shame to not reuse what is currently available, then I
> > would suggest to reuse that with HMAC_SCRAM_SHAXXX as label.
> > - Move *all* the SHA-related things of pgcrypto to src/common,
> > including SHA1, SHA224 and SHA256. px_memset is a simple wrapper on
> > top of memset, we should clean up that first.
> > Any other things to consider that I am forgetting?
>
> After looking more into that, I have come up with PG-like equivalents
> of things in openssl/sha.h:
> pg_shaXX_init(pg_shaXX_ctx *ctx, data);
> pg_shaXX_update(pg_shaXX_ctx *ctx, uint8 *data, size_t len);
> pg_shaXX_final(uint8 *dest, pg_shaXX_ctx *ctx);
> Then think about shaXX as 1, 224, 256, 384 and 512.
>
> Hence all those functions, moved to src/common, finish with the
> following shape, take an init() one:
> #ifdef USE_SSL
> #define <openssl/sha.h>
> #endif
> void
> pg_shaXX_init(pg_shaXX_ctx *ctx)
> {
> #ifdef USE_SSL
> SHAXX_Init((SHAXX_CTX *) ctx);
> #else
> //Here does the OpenBSD stuff, now part of pgcrypto
> #endif
> }
>
> And that's really ugly, all the OpenBSD things that are used by
> pgcrypto when the code is not built with --with-openssl gather into a
> single place with parts wrapped around USE_SSL. A less ugly solution
> would be to split that into two files, and one or the other gets
> included in OBJS depending on if the build is done with or without
> OpenSSL. We do a rather similar thing with fe/be-secure-openssl.c.
>

FWIW, the main reason for be-secure-openssl.c is that we could have support
for another external SSL library. The idea was never to have a builtin
replacement for it :)

However, is there something that's fundamentally better with the OpenSSL
implementation? Or should we just keep *just* the #else branch in the code,
the part we've imported from OpenBSD?

TLS is complex, we don't want to do that in that case. But just the sha
functions isn't *that* complex, is it?

> Another possibility is that we could say that SCRAM is designed to
> work with TLS, as mentioned a bit upthread via the RFC, so we would
> not support it in builds compiled without OpenSSL. I think that would
> be a shame, but it would simplify all this refactoring juggling.
>
> So, 3 possibilities here:
> 1) Use a single file src/common/sha.c that includes a set of functions
> using USE_SSL
> 2) Have two files in src/common, one when build is used with OpenSSL,
> and the second one when built-in methods are used
> 3) Disable the use of SCRAM when OpenSSL is not present in the build.
>
> Opinions? My heart goes for 2) because 1) is ugly, and 3) is not
> appealing in terms of flexibility.
>
>
I really dislike #3 - we want everybody to start using this...

I'm not sure how common a build without openssl is in the real world
though. RPMs, DEBs, Windows installers etc all build with OpenSSL. But we
probably don't want to make it mandatory, no...

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kyotaro HORIGUCHI 2016-07-05 09:02:02 Re: [CF2016-9] Allow spaces in working path on tap-tests
Previous Message Albe Laurenz 2016-07-05 08:24:55 Re: to_date_valid()