Revert 97925 - Linux: allow linking directly with Kerberos instead of using dlopen.

Revert 97925 - Linux: allow linking directly with Kerberos instead of using dlopen.

dlopen is still the default for Google Chrome. This option
is intended for Linux distro packagers.

BUG=92689
Review URL: http://codereview.chromium.org/7655046

TBR=phajdan.jr@chromium.org
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=98028

[cbentzel, 2011-08-24 12:42:03]

LGTM

Pawel - it looks like this broke on ChromeOS developer machines. It
may make sense to specify USE_KERBEROS=0 there since it doesn't ship
with gssapi anyway.

On Wed, Aug 24, 2011 at 8:38 AM,  <dpolukhin@chromium.org> wrote:
> Reviewers: Paweł Hajdan Jr.,
>
> Description:
> Revert 97925 - Linux: allow linking directly with Kerberos instead of using
> dlopen.
>
> dlopen is still the default for Google Chrome. This option
> is intended for Linux distro packagers.
>
> BUG=92689
>
> Review URL: http://codereview.chromium.org/7655046
>
> TBR=phajdan.jr@chromium.org
>
> Please review this at http://codereview.chromium.org/7719024/
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src/
>
> Affected files:
>  M     build/install-build-deps.sh
>  M     net/http/http_auth_gssapi_posix.h
>  M     net/http/http_auth_gssapi_posix.cc
>  M     net/http/http_auth_gssapi_posix_unittest.cc
>  M     net/http/http_auth_handler_factory.cc
>  M     net/http/http_auth_handler_negotiate_unittest.cc
>  M     net/http/mock_gssapi_library_posix.h
>  M     net/http/mock_gssapi_library_posix.cc
>  M     net/net.gyp
>
>
> Index: build/install-build-deps.sh
> ===================================================================
> --- build/install-build-deps.sh (revision 98027)
> +++ build/install-build-deps.sh (working copy)
> @@ -105,10 +105,10 @@
>           libapache2-mod-php5 libasound2-dev libbz2-dev libcairo2-dev
>           libcups2-dev libdbus-glib-1-dev libgconf2-dev
>           libgl1-mesa-dev libglu1-mesa-dev libglib2.0-dev
> libgnome-keyring-dev
> -          libgtk2.0-dev libjpeg62-dev libkrb5-dev libnspr4-dev libnss3-dev
> -          libpam0g-dev libsctp-dev libsqlite3-dev libxslt1-dev libxss-dev
> -          libxtst-dev mesa-common-dev msttcorefonts patch perl libwww-perl
> -          php5-cgi pkg-config python python-dev rpm subversion
> ttf-dejavu-core
> +          libgtk2.0-dev libjpeg62-dev libnspr4-dev libnss3-dev libpam0g-dev
> +          libsctp-dev libsqlite3-dev libxslt1-dev libxss-dev libxtst-dev
> +          mesa-common-dev msttcorefonts patch perl libwww-perl php5-cgi
> +          pkg-config python python-dev rpm subversion ttf-dejavu-core
>           ttf-kochi-gothic ttf-kochi-mincho wdiff libcurl4-gnutls-dev
>           ttf-indic-fonts ttf-thai-tlwg
>           $chromeos_dev_list"
> Index: net/http/http_auth_gssapi_posix.cc
> ===================================================================
> --- net/http/http_auth_gssapi_posix.cc  (revision 98027)
> +++ net/http/http_auth_gssapi_posix.cc  (working copy)
> @@ -17,7 +17,6 @@
>  #include "net/base/net_errors.h"
>  #include "net/base/net_util.h"
>
> -#if defined(DLOPEN_KERBEROS)
>  // These are defined for the GSSAPI library:
>  // Paraphrasing the comments from gssapi.h:
>  // "The implementation must reserve static storage for a
> @@ -63,7 +62,6 @@
>  gss_OID GSS_C_NT_HOSTBASED_SERVICE = &GSS_C_NT_HOSTBASED_SERVICE_VAL;
>  gss_OID GSS_C_NT_ANONYMOUS = &GSS_C_NT_ANONYMOUS_VAL;
>  gss_OID GSS_C_NT_EXPORT_NAME = &GSS_C_NT_EXPORT_NAME_VAL;
> -#endif  // defined(DLOPEN_KERBEROS)
>
>  namespace net {
>
> @@ -412,11 +410,9 @@
>
>  bool GSSAPISharedLibrary::InitImpl() {
>   DCHECK(!initialized_);
> -#if defined(DLOPEN_KERBEROS)
>   gssapi_library_ = LoadSharedLibrary();
>   if (gssapi_library_ == NULL)
>     return false;
> -#endif  // defined(DLOPEN_KERBEROS)
>   initialized_ = true;
>   return true;
>  }
> @@ -463,20 +459,17 @@
>   return NULL;
>  }
>
> -#if defined(DLOPEN_KERBEROS)
>  #define BIND(lib, x)                                                    \
> -  DCHECK(lib);                                                          \
>   gss_##x##_type x = reinterpret_cast<gss_##x##_type>(                  \
>       base::GetFunctionPointerFromNativeLibrary(lib, "gss_" #x));       \
>   if (x == NULL) {                                                      \
>     LOG(WARNING) << "Unable to bind function \"" << "gss_" #x << "\"";  \
>     return false;                                                       \
>   }
> -#else
> -#define BIND(lib, x) gss_##x##_type x = gss_##x
> -#endif
>
>  bool GSSAPISharedLibrary::BindMethods(base::NativeLibrary lib) {
> +  DCHECK(lib != NULL);
> +
>   BIND(lib, import_name);
>   BIND(lib, release_name);
>   BIND(lib, release_buffer);
> Index: net/http/http_auth_gssapi_posix.h
> ===================================================================
> --- net/http/http_auth_gssapi_posix.h   (revision 98027)
> +++ net/http/http_auth_gssapi_posix.h   (working copy)
> @@ -6,8 +6,6 @@
>  #define NET_HTTP_HTTP_AUTH_GSSAPI_POSIX_H_
>  #pragma once
>
> -#include <gssapi.h>
> -
>  #include <string>
>
>  #include "base/gtest_prod_util.h"
> @@ -16,6 +14,9 @@
>  #include "net/base/net_export.h"
>  #include "net/http/http_auth.h"
>
> +#define GSS_USE_FUNCTION_POINTERS
> +#include "net/third_party/gssapi/gssapi.h"
> +
>  namespace net {
>
>  NET_EXPORT_PRIVATE extern gss_OID CHROME_GSS_C_NT_HOSTBASED_SERVICE_X;
> @@ -167,16 +168,6 @@
>       int* open);
>
>  private:
> -  typedef typeof(&gss_import_name) gss_import_name_type;
> -  typedef typeof(&gss_release_name) gss_release_name_type;
> -  typedef typeof(&gss_release_buffer) gss_release_buffer_type;
> -  typedef typeof(&gss_display_name) gss_display_name_type;
> -  typedef typeof(&gss_display_status) gss_display_status_type;
> -  typedef typeof(&gss_init_sec_context) gss_init_sec_context_type;
> -  typedef typeof(&gss_wrap_size_limit) gss_wrap_size_limit_type;
> -  typedef typeof(&gss_delete_sec_context) gss_delete_sec_context_type;
> -  typedef typeof(&gss_inquire_context) gss_inquire_context_type;
> -
>   FRIEND_TEST_ALL_PREFIXES(HttpAuthGSSAPIPOSIXTest, GSSAPIStartup);
>
>   bool InitImpl();
> Index: net/http/http_auth_gssapi_posix_unittest.cc
> ===================================================================
> --- net/http/http_auth_gssapi_posix_unittest.cc (revision 98027)
> +++ net/http/http_auth_gssapi_posix_unittest.cc (working copy)
> @@ -81,13 +81,11 @@
>   EXPECT_TRUE(gssapi.get()->Init());
>  }
>
> -#if defined(DLOPEN_KERBEROS)
>  TEST(HttpAuthGSSAPIPOSIXTest, GSSAPILoadCustomLibrary) {
>   scoped_ptr<GSSAPILibrary> gssapi(
>       new GSSAPISharedLibrary("/this/library/does/not/exist"));
>   EXPECT_FALSE(gssapi.get()->Init());
>  }
> -#endif  // defined(DLOPEN_KERBEROS)
>
>  TEST(HttpAuthGSSAPIPOSIXTest, GSSAPICycle) {
>   scoped_ptr<test::MockGSSAPILibrary> mock_library(new
> test::MockGSSAPILibrary);
> Index: net/http/http_auth_handler_factory.cc
> ===================================================================
> --- net/http/http_auth_handler_factory.cc       (revision 98027)
> +++ net/http/http_auth_handler_factory.cc       (working copy)
> @@ -10,11 +10,10 @@
>  #include "net/http/http_auth_filter.h"
>  #include "net/http/http_auth_handler_basic.h"
>  #include "net/http/http_auth_handler_digest.h"
> -#include "net/http/http_auth_handler_ntlm.h"
> -
>  #if defined(USE_KERBEROS)
>  #include "net/http/http_auth_handler_negotiate.h"
>  #endif
> +#include "net/http/http_auth_handler_ntlm.h"
>
>  namespace net {
>
> Index: net/http/http_auth_handler_negotiate_unittest.cc
> ===================================================================
> --- net/http/http_auth_handler_negotiate_unittest.cc    (revision 98027)
> +++ net/http/http_auth_handler_negotiate_unittest.cc    (working copy)
> @@ -15,6 +15,7 @@
>  #include "net/http/mock_sspi_library_win.h"
>  #elif defined(OS_POSIX)
>  #include "net/http/mock_gssapi_library_posix.h"
> +#include "net/third_party/gssapi/gssapi.h"
>  #endif
>  #include "testing/gtest/include/gtest/gtest.h"
>  #include "testing/platform_test.h"
> @@ -25,6 +26,7 @@
>  typedef net::test::MockGSSAPILibrary MockAuthLibrary;
>  #endif
>
> +
>  namespace net {
>
>  class HttpAuthHandlerNegotiateTest : public PlatformTest {
> @@ -343,7 +345,6 @@
>   EXPECT_EQ(ERR_MISSING_AUTH_CREDENTIALS, callback.WaitForResult());
>  }
>
> -#if defined(DLOPEN_KERBEROS)
>  TEST_F(HttpAuthHandlerNegotiateTest, MissingGSSAPI) {
>   scoped_ptr<HostResolver> host_resolver(new MockHostResolver());
>   MockAllowURLSecurityManager url_security_manager;
> @@ -365,7 +366,6 @@
>   EXPECT_EQ(ERR_UNSUPPORTED_AUTH_SCHEME, rv);
>   EXPECT_TRUE(generic_handler.get() == NULL);
>  }
> -#endif  // defined(DLOPEN_KERBEROS)
>
>  #endif  // defined(OS_POSIX)
>
> Index: net/http/mock_gssapi_library_posix.cc
> ===================================================================
> --- net/http/mock_gssapi_library_posix.cc       (revision 98027)
> +++ net/http/mock_gssapi_library_posix.cc       (working copy)
> @@ -7,6 +7,7 @@
>  #include "base/logging.h"
>  #include "base/string_util.h"
>  #include "base/stringprintf.h"
> +#include "net/third_party/gssapi/gssapi.h"
>  #include "testing/gtest/include/gtest/gtest.h"
>
>  namespace net {
> @@ -278,7 +279,7 @@
>   // Save the data.
>   output->name = BufferToString(input_name_buffer);
>   CopyOid(&output->name_type, input_name_type);
> -  *output_name = reinterpret_cast<gss_name_t>(output);
> +  *output_name = output;
>
>   return GSS_S_COMPLETE;
>  }
> Index: net/http/mock_gssapi_library_posix.h
> ===================================================================
> --- net/http/mock_gssapi_library_posix.h        (revision 98027)
> +++ net/http/mock_gssapi_library_posix.h        (working copy)
> @@ -6,13 +6,12 @@
>  #define NET_HTTP_MOCK_GSSAPI_LIBRARY_POSIX_H_
>  #pragma once
>
> -#include <gssapi.h>
> -
>  #include <list>
>  #include <string>
>
>  #include "base/gtest_prod_util.h"
>  #include "net/http/http_auth_gssapi_posix.h"
> +#include "net/third_party/gssapi/gssapi.h"
>
>  namespace net {
>
> Index: net/net.gyp
> ===================================================================
> --- net/net.gyp (revision 98027)
> +++ net/net.gyp (working copy)
> @@ -7,7 +7,6 @@
>     'chromium_code': 1,
>
>     'use_kerberos%': 1,
> -    'linux_link_kerberos%': 0,
>   },
>   'targets': [
>     {
> @@ -711,19 +710,6 @@
>           'defines': [
>             'USE_KERBEROS',
>           ],
> -          'conditions': [
> -            ['linux_link_kerberos==1', {
> -              'link_settings': {
> -                'ldflags': [
> -                  '<!@(krb5-config --libs gssapi)',
> -                ],
> -              },
> -            }, { # linux_link_kerberos==0
> -              'defines': [
> -                'DLOPEN_KERBEROS',
> -              ],
> -            }],
> -          ],
>         }, { # use_kerberos == 0
>           'sources!': [
>             'http/http_auth_gssapi_posix.cc',
>
>
>

[Paweł Hajdan Jr., 2011-08-24 17:17:57]

Heh, one ChromeOS guy asked me about this and I told him what to do.

Why don't you just install Kerberos on ChromeOS? Of course disabling it there is
an easy "fix", but I'm not sure if it's good for the project, especially when
targeting Enterprisey people.

I'm now figuring out the plan to reland this. Sorry for breaking your tree once,
I'll do my best not to do it second time.

[cbentzel, 2011-08-24 17:22:43]

On Wed, Aug 24, 2011 at 1:17 PM,  <phajdan.jr@chromium.org> wrote:
> Heh, one ChromeOS guy asked me about this and I told him what to do.
>
> Why don't you just install Kerberos on ChromeOS? Of course disabling it
> there is
> an easy "fix", but I'm not sure if it's good for the project, especially
> when
> targeting Enterprisey people.

The problem with doing this on ChromeOS isn't shipping with a library
- it's providing hooks to configure krb5.conf or equivalent files.

>
> I'm now figuring out the plan to reland this. Sorry for breaking your tree
> once,
> I'll do my best not to do it second time.
>
> http://codereview.chromium.org/7719024/
>