Thread: pgcrypto 3des failure, OpenSSL 0.9.8, Solaris 9/sparc
On my Solaris 9/sparc box with OpenSSL 0.9.8-beta6, the pgcrypto regression tests fail the 3des test. I haven't checked against older versions of OpenSSL; I'll do so when I get a chance. I haven't dug into the pgcrypto code yet -- is it doing anything that might be platform-specific? Or is this more likely a problem with OpenSSL? regression.diffs attached. -- Michael Fuhr http://www.fuhr.org/~mfuhr/
Attachment
Michael Fuhr <mike@fuhr.org> writes: > On my Solaris 9/sparc box with OpenSSL 0.9.8-beta6, the pgcrypto > regression tests fail the 3des test. I haven't checked against > older versions of OpenSSL; I'll do so when I get a chance. > I haven't dug into the pgcrypto code yet -- is it doing anything > that might be platform-specific? Or is this more likely a problem > with OpenSSL? I don't see a problem with the pgcrypto regression tests in CVS HEAD, either locally or on the buildfarm board. However probably none of these machines are running a beta version of OpenSSL ... regards, tom lane
On Tue, Jul 05, 2005 at 08:40:08AM -0600, Michael Fuhr wrote: > On my Solaris 9/sparc box with OpenSSL 0.9.8-beta6, the pgcrypto > regression tests fail the 3des test. I haven't checked against > older versions of OpenSSL; I'll do so when I get a chance. > > I haven't dug into the pgcrypto code yet -- is it doing anything > that might be platform-specific? Or is this more likely a problem > with OpenSSL? It is a bug in pgcrypto. I can only excuse it with my strong antipathy towards 3des. Could you test it with newer OpenSSL? -- marko
Attachment
On Tue, Jul 05, 2005 at 07:21:17PM +0300, Marko Kreen wrote: > > It is a bug in pgcrypto. I can only excuse it with my strong antipathy > towards 3des. > > Could you test it with newer OpenSSL? Looks good. After applying the patch, all pgcrypto regression tests pass on my box running Solaris 9/sparc, OpenSSL 0.9.8-beta6, and HEAD. I expect you'll need to submit the patch to pgsql-patches so it'll get put in the queue. Thanks. -- Michael Fuhr http://www.fuhr.org/~mfuhr/
Michael Fuhr <mike@fuhr.org> writes: > On Tue, Jul 05, 2005 at 07:21:17PM +0300, Marko Kreen wrote: >> It is a bug in pgcrypto. I can only excuse it with my strong antipathy >> towards 3des. > Looks good. After applying the patch, all pgcrypto regression tests > pass on my box running Solaris 9/sparc, OpenSSL 0.9.8-beta6, and HEAD. > I expect you'll need to submit the patch to pgsql-patches so it'll > get put in the queue. No need, we'll apply it. regards, tom lane
Marko Kreen <marko@l-t.ee> writes: > On Tue, Jul 05, 2005 at 08:40:08AM -0600, Michael Fuhr wrote: >> On my Solaris 9/sparc box with OpenSSL 0.9.8-beta6, the pgcrypto >> regression tests fail the 3des test. I haven't checked against >> older versions of OpenSSL; I'll do so when I get a chance. > It is a bug in pgcrypto. I can only excuse it with my strong antipathy > towards 3des. Patch applied. Now that I look, the reason the buildfarm failed to find this is of course that the pgcrypto Makefile is configured to never build or test this code. Would it be reasonable to fix the makefile to follow the toplevel --with-openssl choice? regards, tom lane
Tom Lane wrote: > >Now that I look, the reason the buildfarm failed to find this is of >course that the pgcrypto Makefile is configured to never build or test >this code. Would it be reasonable to fix the makefile to follow the >toplevel --with-openssl choice? > > > Yes, please! good catch! andrew
On Tue, Jul 05, 2005 at 02:18:00PM -0400, Tom Lane wrote: > Marko Kreen <marko@l-t.ee> writes: > > On Tue, Jul 05, 2005 at 08:40:08AM -0600, Michael Fuhr wrote: > >> On my Solaris 9/sparc box with OpenSSL 0.9.8-beta6, the pgcrypto > >> regression tests fail the 3des test. I haven't checked against > >> older versions of OpenSSL; I'll do so when I get a chance. > > > It is a bug in pgcrypto. I can only excuse it with my strong antipathy > > towards 3des. > > Patch applied. > > Now that I look, the reason the buildfarm failed to find this is of > course that the pgcrypto Makefile is configured to never build or test > this code. Would it be reasonable to fix the makefile to follow the > toplevel --with-openssl choice? Heh. As it happens, I just researched the same thing. This will be especially imortant with the new PGP code, which simply does not work without OpenSSL. I see 2 variants: 1) put @with_openssl@ and @with_zlib@ variables into contrib/pgcrypto/Makefile.in and let configure process it. 2) put them in some other makefile fragment under src/ and let pgcrypto include it. First I did the simple thing and put them into Makefile.global.in, but this does not work, as it needs to be included _after_ all module variables are set. So 2) needs a new file. -- marko
Marko Kreen <marko@l-t.ee> writes: > I see 2 variants: > 1) put @with_openssl@ and @with_zlib@ variables into > contrib/pgcrypto/Makefile.in and let configure process it. > 2) put them in some other makefile fragment under src/ > and let pgcrypto include it. > First I did the simple thing and put them into Makefile.global.in, > but this does not work, as it needs to be included _after_ all > module variables are set. So 2) needs a new file. Hm ... libpq manages to build code that requires openssl without needing a generated Makefile, so why do we need it here? I'd prefer #1 of these two, but seeing that no other contrib module has a Makefile.in, not having to do either would be even better. This is particularly true if you aren't going to completely eliminate the hand-configuration options in the Makefile, because having to edit a generated Makefile or having to edit Makefile.in and then remember to reconfigure both suck. regards, tom lane
On Tue, Jul 05, 2005 at 02:55:07PM -0400, Tom Lane wrote: > Marko Kreen <marko@l-t.ee> writes: > > I see 2 variants: > > > 1) put @with_openssl@ and @with_zlib@ variables into > > contrib/pgcrypto/Makefile.in and let configure process it. > > 2) put them in some other makefile fragment under src/ > > and let pgcrypto include it. > > > First I did the simple thing and put them into Makefile.global.in, > > but this does not work, as it needs to be included _after_ all > > module variables are set. So 2) needs a new file. > > Hm ... libpq manages to build code that requires openssl without > needing a generated Makefile, so why do we need it here? Now, looking more into it, it indeed does work. But it breaks pgcrypto build for USE_PGXS case. > I'd prefer #1 of these two, but seeing that no other contrib module > has a Makefile.in, not having to do either would be even better. Can I break USE_PGXS? Otherwise I need pre-processed Makefile.in. > This is particularly true if you aren't going to completely eliminate > the hand-configuration options in the Makefile, because having to edit > a generated Makefile or having to edit Makefile.in and then remember > to reconfigure both suck. I'd like to eliminate hand-config. Current code does not need hand-config - you will miss only couple of algorithms without OpenSSL. And there is no code that needs strong randomness. With new PGP code, if you dont have OpenSSL you won't have public-key code anyway - you can't configure bignum support. That leaves randomness for pgp_sym_encrypt - which can be solved with including a strong PRNG with pgcrypto. I think this can be done in 8.1 timeframe, as it will be pretty small piece of code. There is also zlib - no need to configure it if I can get it from main config. -- marko
Marko Kreen <marko@l-t.ee> writes: > On Tue, Jul 05, 2005 at 02:55:07PM -0400, Tom Lane wrote: >> Hm ... libpq manages to build code that requires openssl without >> needing a generated Makefile, so why do we need it here? > Now, looking more into it, it indeed does work. > But it breaks pgcrypto build for USE_PGXS case. How exactly? > Can I break USE_PGXS? Nope, that's right out. If we have to use a Makefile.in we will, but I'd like to understand why we have to. > I'd like to eliminate hand-config. That would be very good. In practice no one is likely to do any manual configuration down inside a contrib module ... certainly none of the RPM distributions do any such thing. regards, tom lane
On Tue, Jul 05, 2005 at 03:36:06PM -0400, Tom Lane wrote: > Marko Kreen <marko@l-t.ee> writes: > > On Tue, Jul 05, 2005 at 02:55:07PM -0400, Tom Lane wrote: > >> Hm ... libpq manages to build code that requires openssl without > >> needing a generated Makefile, so why do we need it here? > > > Now, looking more into it, it indeed does work. > > But it breaks pgcrypto build for USE_PGXS case. > > How exactly? ifdef USE_PGXS PGXS = $(shell pg_config --pgxs) include $(PGXS) else ..... The "include $(PGXS)" includes parts that need initialized local variables, but the variable initializing is the part that needs settings from main makefiles... And including Makefile.global twice gives errors. > > Can I break USE_PGXS? > > Nope, that's right out. If we have to use a Makefile.in we will, > but I'd like to understand why we have to. > > > I'd like to eliminate hand-config. > > That would be very good. In practice no one is likely to do any manual > configuration down inside a contrib module ... certainly none of the RPM > distributions do any such thing. It was unimportant thus far, but now it's life-or-death ;) -- marko
Marko Kreen <marko@l-t.ee> writes: > Oh, ofcourse I would not need to break it, if the interesting > settings can be put into eg. src/Makefile.config (.in), and > I include that one. You mean Makefile.global.in, no? That seems fine to me. > But that case would break if top_srcdir is not ../.. No it wouldn't. regards, tom lane
On Tue, Jul 05, 2005 at 03:58:43PM -0400, Tom Lane wrote: > Marko Kreen <marko@l-t.ee> writes: > > Oh, ofcourse I would not need to break it, if the interesting > > settings can be put into eg. src/Makefile.config (.in), and > > I include that one. > > You mean Makefile.global.in, no? That seems fine to me. No, thats the point - the PGXS include also includes Makefile.global, and including it twice does not work. > > But that case would break if top_srcdir is not ../.. > > No it wouldn't. How can I find the top dir? Hm, actually, I can do $(dir $(PGXS))../Makefile.config So, yes it would work. So here's the variants: 1) contrib/pgcrypto/Makefile.in 2) src/Makefile.config.in I think I like 2) more, thus pgcrypto would not be special among contrib modules. It could be useful for other modules aswell. Also 1) would break postgresql-base build, no? It could be probably avoided with some shell scripting but it would be still inelegant. Comments? -- marko
Marko Kreen <marko@l-t.ee> writes: > On Tue, Jul 05, 2005 at 03:58:43PM -0400, Tom Lane wrote: >> You mean Makefile.global.in, no? That seems fine to me. > No, thats the point - the PGXS include also includes > Makefile.global, and including it twice does not work. So? pgxs.mk includes Makefile.global before doing anything interesting. Seems to me you could do what you need to given make's rules about delayed evaluation of variables. regards, tom lane
On Tue, Jul 05, 2005 at 04:16:04PM -0400, Tom Lane wrote: > Marko Kreen <marko@l-t.ee> writes: > > On Tue, Jul 05, 2005 at 03:58:43PM -0400, Tom Lane wrote: > >> You mean Makefile.global.in, no? That seems fine to me. > > > No, thats the point - the PGXS include also includes > > Makefile.global, and including it twice does not work. > > So? pgxs.mk includes Makefile.global before doing anything interesting. > Seems to me you could do what you need to given make's rules about > delayed evaluation of variables. How? Current config is done using make ifdef/ifeq which are executed immidately. I don't know make that well, maybe there is a way to put if's inside variables, but that would get rather messy, no? -- marko
Marko Kreen <marko@l-t.ee> writes: > Ok Tom, you win. It is indeed possible to make it work, and the > resulting makefile is even cleaner than before. > Following patch thus autoconfigures pgcrypto. It drops the > possibility to use libc's crypt, which was pointless. Applied with a little extra hacking (completely overriding the original values of PG_CPPFLAGS and SHLIB_LINK was unwise, and in fact guaranteed to fail on machines where OpenSSL isn't in the default location). regards, tom lane
On Tue, Jul 05, 2005 at 07:20:48PM -0400, Tom Lane wrote: > Marko Kreen <marko@l-t.ee> writes: > > Ok Tom, you win. It is indeed possible to make it work, and the > > resulting makefile is even cleaner than before. > > Following patch thus autoconfigures pgcrypto. It drops the > > possibility to use libc's crypt, which was pointless. > > Applied with a little extra hacking (completely overriding the original > values of PG_CPPFLAGS and SHLIB_LINK was unwise, and in fact guaranteed > to fail on machines where OpenSSL isn't in the default location). Your PG_CPPFLAGS change does not work - it gets always RAND_SILLY. If I change it from := to =, make complains about recursion. -- marko
Attachment
Marko Kreen <marko@l-t.ee> writes: > ! PG_CPPFLAGS := $(CF_CFLAGS) -I$(srcdir) $(PG_CPPFLAGS) > ... > ! PG_CPPFLAGS = $(CF_CFLAGS) -I$(srcdir) Ah, right. Actually we don't need -I$(srcdir) either, since pgxs.mk adds that automatically. regards, tom lane