Redux broken krb5.h handling (#1156)

De-duplicate the #include sequence into a libcompat header to
ensure all uses of the hack(s) including autoconf test macros
are using the same logic. This fixes some macros which were
using inconsistent logic for Solaris.

Simplify the if-elif use of __cplusplus detection. All the
extern C hacks apply only when C++ is used.

Wrapping the entire file peer_proxy_negotiate_auth.cc with
'extern C {...}' is unnecessary (it is a .cc) and breaks the
Solaris workaround.

diff --git a/acinclude/krb5.m4 b/acinclude/krb5.m4
index ca06a5a4dbb03168e2844f3820942d2901d9373f..23b23fe0e6b58191983898b5773b48445a43e8a3 100644 (file)
--- a/acinclude/krb5.m4
+++ b/acinclude/krb5.m4
@@ -12,26 +12,27 @@ dnl checks for a broken solaris header file, and sets squid_cv_broken_krb5_h
 dnl to yes if that's the case
 AC_DEFUN([SQUID_CHECK_KRB5_SOLARIS_BROKEN_KRB5_H], [
   AC_CACHE_CHECK([for broken Solaris krb5.h],squid_cv_broken_krb5_h, [
+    SQUID_STATE_SAVE(squid_krb5_solaris_test)
+    CPPFLAGS="-I${srcdir:-.} $CPPFLAGS"
     AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
 #include <krb5.h>
 int i;
 ]])], [ squid_cv_broken_krb5_h=no ], [
     AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
-#if defined(__cplusplus)
-#define KRB5INT_BEGIN_DECLS     extern "C" {
-#define KRB5INT_END_DECLS
-KRB5INT_BEGIN_DECLS
-#endif
-#include <krb5.h>
+#define HAVE_BROKEN_SOLARIS_KRB5_H  1
+#include "compat/krb5.h"
 int i;
 ]])], [ squid_cv_broken_krb5_h=yes ], [ squid_cv_broken_krb5_h=no ])
     ])
+    SQUID_STATE_ROLLBACK(squid_krb5_solaris_test)
   ])
 ]) dnl SQUID_CHECK_KRB5_SOLARIS_BROKEN_KRB5_H
 
 
 AC_DEFUN([SQUID_CHECK_KRB5_HEIMDAL_BROKEN_KRB5_H], [
   AC_CACHE_CHECK([for broken Heimdal krb5.h],squid_cv_broken_heimdal_krb5_h, [
+    SQUID_STATE_SAVE(squid_krb5_heimdal_test)
+    CPPFLAGS="-I${srcdir:-.} $CPPFLAGS"
     AC_RUN_IFELSE([AC_LANG_SOURCE([[
 #include <krb5.h>
 int
@@ -45,13 +46,8 @@ main(void)
 }
 ]])], [ squid_cv_broken_heimdal_krb5_h=no ], [
     AC_RUN_IFELSE([AC_LANG_SOURCE([[
-#if defined(__cplusplus)
-extern "C" {
-#endif
-#include <krb5.h>
-#if defined(__cplusplus)
-}
-#endif
+#define HAVE_BROKEN_HEIMDAL_KRB5_H  1
+#include "compat/krb5.h"
 int
 main(void)
 {
@@ -63,6 +59,7 @@ main(void)
 }
 ]])], [ squid_cv_broken_heimdal_krb5_h=yes ], [ squid_cv_broken_heimdal_krb5_h=no ])
     ])
+    SQUID_STATE_ROLLBACK(squid_krb5_heimdal_test)
   ])
 ]) dnl SQUID_CHECK_KRB5_HEIMDAL_BROKEN_KRB5_H
 
@@ -70,23 +67,16 @@ dnl check the max skew in the krb5 context, and sets squid_cv_max_skew_context
 AC_DEFUN([SQUID_CHECK_MAX_SKEW_IN_KRB5_CONTEXT],[
   AC_CACHE_CHECK([for max_skew in struct krb5_context],
                   squid_cv_max_skew_context, [
+    SQUID_STATE_SAVE(squid_krb5_test)
+    CPPFLAGS="-I${srcdir:-.} $CPPFLAGS"
     AC_COMPILE_IFELSE([
       AC_LANG_PROGRAM([[
-#if HAVE_BROKEN_SOLARIS_KRB5_H
-#if defined(__cplusplus)
-#define KRB5INT_BEGIN_DECLS     extern "C" {
-#define KRB5INT_END_DECLS
-KRB5INT_BEGIN_DECLS
-#endif
-#endif
-#if USE_APPLE_KRB5
-#define KERBEROS_APPLE_DEPRECATED(x)
-#endif
-#include <krb5.h>
+#include "compat/krb5.h"
 krb5_context kc; kc->max_skew = 1;
       ]])
     ],[ squid_cv_max_skew_context=yes ],
     [ squid_cv_max_skew_context=no ])
+    SQUID_STATE_ROLLBACK(squid_krb5_test)
   ])
 ])
 
@@ -94,19 +84,11 @@ dnl check whether the kerberos context has a memory cache. Sets
 dnl squid_cv_memory_cache if that's the case.
 AC_DEFUN([SQUID_CHECK_KRB5_CONTEXT_MEMORY_CACHE],[
   AC_CACHE_CHECK([for memory cache], squid_cv_memory_cache, [
+    SQUID_STATE_SAVE(squid_krb5_test)
+    CPPFLAGS="-I${srcdir:-.} $CPPFLAGS"
     AC_RUN_IFELSE([
       AC_LANG_SOURCE([[
-#if HAVE_BROKEN_SOLARIS_KRB5_H
-#if defined(__cplusplus)
-#define KRB5INT_BEGIN_DECLS     extern "C" {
-#define KRB5INT_END_DECLS
-KRB5INT_BEGIN_DECLS
-#endif
-#endif
-#if USE_APPLE_KRB5
-#define KERBEROS_APPLE_DEPRECATED(x)
-#endif
-#include <krb5.h>
+#include "compat/krb5.h"
 int main(int argc, char *argv[])
 {
     krb5_context context;
@@ -117,6 +99,7 @@ int main(int argc, char *argv[])
 }
       ]])
     ], [ squid_cv_memory_cache=yes ], [ squid_cv_memory_cache=no ], [:])
+    SQUID_STATE_ROLLBACK(squid_krb5_test)
   ])
 ])
 
@@ -124,19 +107,11 @@ dnl check whether the kerberos context has a memory keytab. Sets
 dnl squid_cv_memory_keytab if that's the case.
 AC_DEFUN([SQUID_CHECK_KRB5_CONTEXT_MEMORY_KEYTAB],[
   AC_CACHE_CHECK([for memory keytab], squid_cv_memory_keytab, [
+    SQUID_STATE_SAVE(squid_krb5_test)
+    CPPFLAGS="-I${srcdir:-.} $CPPFLAGS"
     AC_RUN_IFELSE([
       AC_LANG_SOURCE([[
-#if HAVE_BROKEN_SOLARIS_KRB5_H
-#if defined(__cplusplus)
-#define KRB5INT_BEGIN_DECLS     extern "C" {
-#define KRB5INT_END_DECLS
-KRB5INT_BEGIN_DECLS
-#endif
-#endif
-#if USE_APPLE_KRB5
-#define KERBEROS_APPLE_DEPRECATED(x)
-#endif
-#include <krb5.h>
+#include "compat/krb5.h"
 int main(int argc, char *argv[])
 {
     krb5_context context;
@@ -147,6 +122,7 @@ int main(int argc, char *argv[])
 }
       ]])
     ], [ squid_cv_memory_keytab=yes ], [ squid_cv_memory_keytab=no ], [:])
+    SQUID_STATE_ROLLBACK(squid_krb5_test)
   ])
 ])
 
@@ -253,27 +229,10 @@ gss_OID gss_mech_spnego = &_gss_mech_spnego;
 dnl checks that krb5 is functional. Sets squid_cv_working_krb5
 AC_DEFUN([SQUID_CHECK_WORKING_KRB5],[
   AC_CACHE_CHECK([for working krb5], squid_cv_working_krb5, [
+    SQUID_STATE_SAVE(squid_krb5_test)
+    CPPFLAGS="-I${srcdir:-.} $CPPFLAGS"
     AC_RUN_IFELSE([AC_LANG_SOURCE([[
-#if USE_APPLE_KRB5
-#define KERBEROS_APPLE_DEPRECATED(x)
-#endif
-#if HAVE_KRB5_H
-#if HAVE_BROKEN_SOLARIS_KRB5_H
-#if defined(__cplusplus)
-#define KRB5INT_BEGIN_DECLS     extern "C" {
-#define KRB5INT_END_DECLS
-KRB5INT_BEGIN_DECLS
-#endif
-#endif
-#if HAVE_BROKEN_HEIMDAL_KRB5_H
-extern "C" {
-#include <krb5.h>
-}
-#else
-#include <krb5.h>
-#endif
-#endif
-
+#include "compat/krb5.h"
 int
 main(void)
 {
@@ -284,6 +243,7 @@ main(void)
         return 0;
 }
   ]])], [ squid_cv_working_krb5=yes ], [ squid_cv_working_krb5=no ],[:])])
+  SQUID_STATE_ROLLBACK(squid_krb5_test)
   AS_IF([test "x$squid_cv_working_krb5" = "xno" -a `echo $LIBS | grep -i -c "(-)L"` -gt 0],[
     AC_MSG_NOTICE([Check Runtime library path !])
   ])
@@ -353,11 +313,10 @@ AC_DEFUN([SQUID_CHECK_KRB5_FUNCS],[
     AC_DEFINE(HAVE_KRB5_GET_INIT_CREDS_OPT_ALLOC,1,
       [Define to 1 if you have krb5_get_init_creds_opt_alloc]),)
   AC_MSG_CHECKING([for krb5_get_init_creds_free requires krb5_context])
+  SQUID_STATE_SAVE(squid_krb5_test)
+  CPPFLAGS="-I${srcdir:-.} $CPPFLAGS"
   AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
-        #if USE_APPLE_KRB5
-        #define KERBEROS_APPLE_DEPRECATED(x)
-        #endif
-       #include <krb5.h>
+#include "compat/krb5.h"
     ]],[[krb5_context context;
         krb5_get_init_creds_opt *options;
         krb5_get_init_creds_opt_free(context, options)]])],[
@@ -365,6 +324,7 @@ AC_DEFUN([SQUID_CHECK_KRB5_FUNCS],[
                  [Define to 1 if you krb5_get_init_creds_free requires krb5_context])
        AC_MSG_RESULT(yes)
     ],[AC_MSG_RESULT(no)],[AC_MSG_RESULT(no)])
+  SQUID_STATE_ROLLBACK(squid_krb5_test)
 
   AC_CHECK_FUNCS(gss_map_name_to_any,
     AC_DEFINE(HAVE_GSS_MAP_ANY_TO_ANY,1,

diff --git a/compat/Makefile.am b/compat/Makefile.am
index 66d5ed373f7c8a07f36541882b29754403b0c7cb..4a668b61c843af7abbd44d5fd02aefb294b43d7e 100644 (file)
--- a/compat/Makefile.am
+++ b/compat/Makefile.am
@@ -40,6 +40,7 @@ libcompatsquid_la_SOURCES = \
        inet_pton.cc \
        inet_pton.h \
        initgroups.h \
+       krb5.h \
        memrchr.cc \
        memrchr.h \
        mswindows.cc \

diff --git a/compat/krb5.h b/compat/krb5.h
new file mode 100644 (file)
index 0000000..b3e1338
--- /dev/null
+++ b/compat/krb5.h
@@ -0,0 +1,49 @@
+/*
+ * Copyright (C) 1996-2022 The Squid Software Foundation and contributors
+ *
+ * Squid software is distributed under GPLv2+ license and includes
+ * contributions from numerous individuals and organizations.
+ * Please see the COPYING and CONTRIBUTORS files for details.
+ */
+
+#ifndef SQUID_COMPAT_KRB5_H
+#define SQUID_COMPAT_KRB5_H
+
+/*
+ * The Kerberos library krb5.h header file has various major
+ * bugs in some implementations:
+ *
+ * - MacOS marks as deprecated the POSIX compatible APIs.
+ *   Apparently to encourage code to use their internal APIs.
+ *
+ * - Heimdal may lack the extern "C" syntax for building in C++
+ *
+ * - Solaris incorrectly implements its own the extern "C" replacement
+ *   macros and as a result krb5.h contains a trailing '}'.
+ *   see http://bugs.opensolaris.org/bugdatabase/view_bug.do?bug_id=6837512
+ *
+ * This file exists to fix those issues the best we can and to
+ * ensure the logic is identical in Squid code, Squid helpers,
+ * and autoconf tests. See acinclude/krb5.h for the latter.
+ */
+
+#if HAVE_KRB5_H
+#  if USE_APPLE_KRB5
+#    define KERBEROS_APPLE_DEPRECATED(x)
+#  endif
+#  if HAVE_BROKEN_HEIMDAL_KRB5_H && defined(__cplusplus)
+     extern "C" {
+#      include <krb5.h>
+     }
+#  elif HAVE_BROKEN_SOLARIS_KRB5_H && defined(__cplusplus)
+#    define KRB5INT_BEGIN_DECLS extern "C" {
+#    define KRB5INT_END_DECLS
+     extern "C" {
+#      include <krb5.h>
+     /* broken Solaris krb5.h contains the closing } */
+#  else
+#    include <krb5.h>
+#  endif
+#endif /* HAVE_KRB5_H */
+
+#endif /* _SQUID__COMPAT_KRB5_H */

diff --git a/src/acl/external/kerberos_ldap_group/support.h b/src/acl/external/kerberos_ldap_group/support.h
index 846bd84da0e39ebc63948213fee53e2af1447677..c25111cbb7ff7b7a5e0992cf1b7f1f3001ddbdf7 100644 (file)
--- a/src/acl/external/kerberos_ldap_group/support.h
+++ b/src/acl/external/kerberos_ldap_group/support.h
@@ -32,35 +32,12 @@
 
 #define KERBEROS_LDAP_GROUP_VERSION "1.4.0sq"
 
-#include <cstring>
-
-#if USE_APPLE_KRB5
-#define KERBEROS_APPLE_DEPRECATED(x)
-#endif
-
-#if HAVE_KRB5_H
-#if HAVE_BROKEN_SOLARIS_KRB5_H
-#warn "Warning! You have a broken Solaris <krb5.h> system header"
-#warn "http://bugs.opensolaris.org/bugdatabase/view_bug.do?bug_id=6837512"
-#if defined(__cplusplus)
-#define KRB5INT_BEGIN_DECLS     extern "C" {
-#define KRB5INT_END_DECLS
-KRB5INT_BEGIN_DECLS
-#endif
-#endif /* HAVE_BROKEN_SOLARIS_KRB5_H */
-#if HAVE_BROKEN_HEIMDAL_KRB5_H
-extern "C" {
-#include <krb5.h>
-}
-#else
-#include <krb5.h>
-#endif
-#endif /* HAVE_KRB5_H */
+#include "compat/krb5.h"
 
+#include <cstring>
 #if HAVE_COM_ERR_H
 #include <com_err.h>
 #endif /* HAVE_COM_ERR_H */
-
 #define LDAP_DEPRECATED 1
 #if HAVE_LDAP_REBIND_FUNCTION
 #define LDAP_REFERRALS

diff --git a/src/auth/negotiate/kerberos/negotiate_kerberos.h b/src/auth/negotiate/kerberos/negotiate_kerberos.h
index aeb4bf6a309124ee78b6d5b8b964041aeb82cbb6..d50c62fb6732d09d08693a77e304654a3cae4833 100644 (file)
--- a/src/auth/negotiate/kerberos/negotiate_kerberos.h
+++ b/src/auth/negotiate/kerberos/negotiate_kerberos.h
@@ -45,32 +45,13 @@
 #endif
 
 #include "base64.h"
+#include "compat/krb5.h"
 #include "util.h"
 
 #if USE_APPLE_KRB5
-#define KERBEROS_APPLE_DEPRECATED(x)
 #define GSSKRB_APPLE_DEPRECATED(x)
 #endif
 
-#if HAVE_KRB5_H
-#if HAVE_BROKEN_SOLARIS_KRB5_H
-#warn "Warning! You have a broken Solaris <krb5.h> system header"
-#warn "http://bugs.opensolaris.org/bugdatabase/view_bug.do?bug_id=6837512"
-#if defined(__cplusplus)
-#define KRB5INT_BEGIN_DECLS     extern "C" {
-#define KRB5INT_END_DECLS
-KRB5INT_BEGIN_DECLS
-#endif
-#endif /* HAVE_BROKEN_SOLARIS_KRB5_H */
-#if HAVE_BROKEN_HEIMDAL_KRB5_H
-extern "C" {
-#include <krb5.h>
-}
-#else
-#include <krb5.h>
-#endif
-#endif /* HAVE_KRB5_H */
-
 #if USE_HEIMDAL_KRB5
 #if HAVE_GSSAPI_GSSAPI_H
 #include <gssapi/gssapi.h>

diff --git a/src/peer_proxy_negotiate_auth.cc b/src/peer_proxy_negotiate_auth.cc
index 00f3260cb0b75a22014a5a84cc89b0401311c632..177c1f77b7e64d568efd9d0ce414e122c5bacb37 100644 (file)
--- a/src/peer_proxy_negotiate_auth.cc
+++ b/src/peer_proxy_negotiate_auth.cc
@@ -14,31 +14,18 @@
 
 #if HAVE_AUTH_MODULE_NEGOTIATE && HAVE_KRB5 && HAVE_GSSAPI
 #if USE_APPLE_KRB5
-#define KERBEROS_APPLE_DEPRECATED(x)
 #define GSSKRB_APPLE_DEPRECATED(x)
 #endif
 
 #include "base64.h"
+#include "compat/krb5.h"
 #include "debug/Stream.h"
 #include "peer_proxy_negotiate_auth.h"
 
-#ifdef __cplusplus
-extern "C" {
-#endif
-
 #if HAVE_PROFILE_H
 #include <profile.h>
 #endif              /* HAVE_PROFILE_H */
-#if HAVE_KRB5_H
-#if HAVE_BROKEN_SOLARIS_KRB5_H
-#if defined(__cplusplus)
-#define KRB5INT_BEGIN_DECLS     extern "C" {
-#define KRB5INT_END_DECLS
-KRB5INT_BEGIN_DECLS
-#endif
-#endif
-#include <krb5.h>
-#elif HAVE_ET_COM_ERR_H
+#if HAVE_ET_COM_ERR_H && !HAVE_KRB5_H
 #include <et/com_err.h>
 #endif                          /* HAVE_COM_ERR_H */
 #if HAVE_COM_ERR_H
@@ -579,8 +566,5 @@ cleanup:
     return token;
 }
 
-#ifdef __cplusplus
-}
-#endif
 #endif /* HAVE_AUTH_MODULE_NEGOTIATE && HAVE_KRB5 && HAVE_GSSAPI */
 

diff --git a/test-suite/buildtests/layer-01-minimal.opts b/test-suite/buildtests/layer-01-minimal.opts
index 2c06b78f8a0327a047780a30016111678d129aab..046079938523a6e45e394824307bb12b561e7517 100644 (file)
--- a/test-suite/buildtests/layer-01-minimal.opts
+++ b/test-suite/buildtests/layer-01-minimal.opts
@@ -99,8 +99,11 @@ DISTCHECK_CONFIGURE_FLAGS=" \
        --without-aio \
        --without-cap \
        --without-dl \
+       --without-gnugss \
+       --without-heimdal-krb5 \
        --without-large-files \
        --without-ldap \
+       --without-mit-krb5 \
        --without-nettle \
        --without-valgrind-debug \
        --without-ipv6-split-stack \

diff --git a/test-suite/buildtests/layer-02-maximus.opts b/test-suite/buildtests/layer-02-maximus.opts
index fcf1b98db780ac83f497330d72c01a10982c98c5..c24cfd72ce33097000c8c1ab2b93cf3cc879e3b4 100644 (file)
--- a/test-suite/buildtests/layer-02-maximus.opts
+++ b/test-suite/buildtests/layer-02-maximus.opts
@@ -45,6 +45,9 @@ MAKETEST="distcheck"
 #   --with-gnutls \
 #   --with-tdb \
 #   --with-cap \
+#   --with-gnugss \
+#   --with-heimdal-krb5 \
+#   --with-mit-krb5 \
 #
 #   --enable-cpu-profiling \  Requires CPU support.
 #

diff --git a/test-suite/buildtests/layer-04-noauth-everything.opts b/test-suite/buildtests/layer-04-noauth-everything.opts
index 93f74f3d405fdc9d401060d2f4310e5a74349a3e..b01df312edf9fbdbcd4a01debcdd98dae5188cb0 100644 (file)
--- a/test-suite/buildtests/layer-04-noauth-everything.opts
+++ b/test-suite/buildtests/layer-04-noauth-everything.opts
@@ -34,6 +34,9 @@ MAKETEST="distcheck"
 #   --with-po2html=PATH \
 #   --with-tags=TAGS \
 #   --with-cap \
+#   --with-gnugss \
+#   --with-heimdal-krb5 \
+#   --with-mit-krb5 \
 #
 #      Following features require special support from other optional packages.
 #      We can't test them automatically everywhere without detecting those