Re: Windows: openssl & gssapi dislike each other

On Tue, Jul 9, 2024 at 2:32 AM Andres Freund <andres@anarazel.de> wrote:
>
> Hi,
>
>
> On 2024-06-13 00:12:51 +0900, Imran Zaheer wrote:
> > I removed the macro from the sslinfo.c as suggested by Andrew. Then I
> > was thinking maybe we can undo some other similar code.
>
> What precisely do you mean by that? Just getting rid of the "ordered include"
> of openssl headers in {fe,be}-secure-openssl.h?
>
Hi

I reordered the includes in {fe,be}-secure-openssl.h as they were also placed
there to resolve similar errors and also were contradicting the
project conventions [1].
But looks like it's better to not touch those as they were for future proofing.

>
> > I rearranged the headers to their previous position in
> > be-secure-openssl.c and in fe-secure-openssl.c. I was able to compile
> > with gssapi and openssl enabled. You can look into the original commits. [1,
> > 2]
> > Is it ok if we undo the changes from these commits?
> >
> > I am attaching two new patches.
> > One with macro guards removed from ssinfo.c.
> > Second patch will additionally rearrange headers for
> > be-secure-openssl.c and in fe-secure-openssl.c to their previous
> > position.
>
> One thing that concerns me with this is that there are other includes of
> gssapi/gssapi.h (e.g. in , which haven't been changed here. ISTM we ought to do apply
> the same change to all of those, otherwise we're just waiting for the problem
> to re-appear.
>

Yes this should be better.

> I wonder if we should add a src/include/libpq/pg-gssapi.h or such, which could
> wrap the entire ifdeferry for gss support. Something like
>
>
> #ifdef ENABLE_GSS
>
> #if defined(HAVE_GSSAPI_H)
> #include <gssapi.h>
> #include <gssapi_ext.h>
> #else
> #include <gssapi/gssapi.h>
> #include <gssapi/gssapi_ext.h>
> #endif
>
> /*
>  * On Windows, <wincrypt.h> includes a #define for X509_NAME, which breaks our
>  * ability to use OpenSSL's version of that symbol if <wincrypt.h> is pulled
>  * in after <openssl/ssl.h> ... and, at least on some builds, it is.  We
>  * can't reliably fix that by re-ordering #includes, because libpq/libpq-be.h
>  * #includes <openssl/ssl.h>.  Instead, just zap the #define again here.
>  */
> #ifdef X509_NAME
> #undef X509_NAME
> #endif
>
> #endif                                                 /* ENABLE_GSS */
>
> Which'd allow the various places using gss (libpq-be.h, be-gssapi-common.h,
> libpq-int.h) to just include pg-gssapi.h and get all of the above without
> redundancy?
>
>
> Another thing that concerns me about this approach is that it seems to assume
> that the only source of such conflicting includes is gssapi. What if some
> other header pulls in wincrypt.h?  But I can't really see a way out of that...
>
> Greetings,
>
> Andres Freund

Creating src/include/libpq/pg-gssapi.h can be another great way of
handling these includes. I compiled successfully but couldn't do
proper testing as there is something wrong with my windows env.

And you are right, the approach we are going with right now only
assumes that it's due to the
gssapi as the bug also appeared when building with gssapi (openssl &
gssapi build). What if openssl clashes with
some other lib too which indirectly includes wincrypt.h

For now maybe we can do the future proofing for gssapi & openssl includes
and do testing if openssl clashes with some other lib too.

Thanks
Imran Zaheer

[1]: https://wiki.postgresql.org/wiki/Committing_checklist#Policies
(3rd last point)