On Tue, Jul 5, 2016 at 10:06 AM, Michael Paquier <michael.paqu...@gmail.com> wrote:
> On Mon, Jul 4, 2016 at 12:54 PM, Michael Paquier > <michael.paqu...@gmail.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/