[PATCH] kerberos: Fix compilation on systems where SONAME_LIBKRB5 is not defined.

[Dmitry Timoshkov]

Signed-off-by: Dmitry Timoshkov 
---
 dlls/kerberos/krb5_ap.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/dlls/kerberos/krb5_ap.c b/dlls/kerberos/krb5_ap.c
index 3827e5ccbc..0a6463c5f3 100644
--- a/dlls/kerberos/krb5_ap.c
+++ b/dlls/kerberos/krb5_ap.c
@@ -842,6 +842,8 @@ static int get_buffer_index( SecBufferDesc *desc, DWORD type )
     return -1;
 }

+#ifdef SONAME_LIBKRB5
+
 static char *get_user_at_domain( const WCHAR *user, ULONG user_len, const WCHAR *domain, ULONG domain_len )
 {
     int len_user, len_domain;
@@ -924,6 +926,16 @@ done:
     return krb5_error_to_status( err );
 }

+#else /* SONAME_LIBKRB5 */
+
+static NTSTATUS init_creds( const SEC_WINNT_AUTH_IDENTITY_W *id )
+{
+    FIXME( "Kerberos support was not provided at compile time\n" );
+    return SEC_E_UNKNOWN_CREDENTIALS;
+}
+
+#endif /* SONAME_LIBKRB5 */
+
 static NTSTATUS acquire_credentials_handle( UNICODE_STRING *principal_us, gss_cred_usage_t cred_usage,
     LSA_SEC_HANDLE *credential, TimeStamp *ts_expiry )
 {

[Hans Leidekker]

On Tue, 2019-07-02 at 11:05 +0800, Dmitry Timoshkov wrote:
> Signed-off-by: Dmitry Timoshkov
>
> dlls/kerberos/krb5_ap.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/dlls/kerberos/krb5_ap.c b/dlls/kerberos/krb5_ap.c
> index 3827e5ccbc..0a6463c5f3 100644
> --- a/dlls/kerberos/krb5_ap.c
> +++ b/dlls/kerberos/krb5_ap.c
> @@ -842,6 +842,8 @@ static int get_buffer_index( SecBufferDesc *desc, DWORD type )
>      return -1;
>  }
>
> +#ifdef SONAME_LIBKRB5

Where do you see this? Note that this code is inside an #ifdef SONAME_LIBGSSAPI_KRB5
block and configure should disable gssapi when it can't detect krb5. This means that
SONAME_LIBGSSAPI_KRB5 should not be set when SONAME_LIBKRB5 isn't.

[Dmitry Timoshkov]

Hans Leidekker  wrote:
> On Tue, 2019-07-02 at 11:05 +0800, Dmitry Timoshkov wrote:
> > Signed-off-by: Dmitry Timoshkov
> >
> > dlls/kerberos/krb5_ap.c | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/dlls/kerberos/krb5_ap.c b/dlls/kerberos/krb5_ap.c
> > index 3827e5ccbc..0a6463c5f3 100644
> > --- a/dlls/kerberos/krb5_ap.c
> > +++ b/dlls/kerberos/krb5_ap.c
> > @@ -842,6 +842,8 @@ static int get_buffer_index( SecBufferDesc *desc, DWORD type )
> >      return -1;
> >  }
> >
> > +#ifdef SONAME_LIBKRB5
>
> Where do you see this? Note that this code is inside an #ifdef SONAME_LIBGSSAPI_KRB5
> block and configure should disable gssapi when it can't detect krb5. This means that
> SONAME_LIBGSSAPI_KRB5 should not be set when SONAME_LIBKRB5 isn't.

I can confirm that on my system configure undefs SONAME_LIBGSSAPI_KRB5.

Unfortunately that's not on one of my machines. According to the reporter,
configure fails to find libkrb55.so but successfully locates libgssapi.so,
and configure log confirms that.

[Hans Leidekker]

On Tue, 2019-07-02 at 17:02 +0800, Dmitry Timoshkov wrote:
> Where do you see this? Note that this code is inside an #ifdef SONAME_LIBGSSAPI_KRB5
> > block and configure should disable gssapi when it can't detect krb5. This means that
> > SONAME_LIBGSSAPI_KRB5 should not be set when SONAME_LIBKRB5 isn't.
>
> I can confirm that on my system configure undefs SONAME_LIBGSSAPI_KRB5.
>
> Unfortunately that's not on one of my machines. According to the reporter,
> configure fails to find libkrb55.so but successfully locates libgssapi.so,
> and configure log confirms that.

Sounds like a broken install. To fix this I think it would be better to improve
our configure check than to add more (nested) ifdefs. AcquireCredentialsHandle
won't be very useful without these krb5 functions.

[Dmitry Timoshkov]

Hans Leidekker  wrote:
> On Tue, 2019-07-02 at 17:02 +0800, Dmitry Timoshkov wrote:
> > Where do you see this? Note that this code is inside an #ifdef SONAME_LIBGSSAPI_KRB5
> > > block and configure should disable gssapi when it can't detect krb5. This means that
> > > SONAME_LIBGSSAPI_KRB5 should not be set when SONAME_LIBKRB5 isn't.
> >
> > I can confirm that on my system configure undefs SONAME_LIBGSSAPI_KRB5.
> >
> > Unfortunately that's not on one of my machines. According to the reporter,
> > configure fails to find libkrb55.so but successfully locates libgssapi.so,
> > and configure log confirms that.
>
> Sounds like a broken install. To fix this I think it would be better to improve
> our configure check than to add more (nested) ifdefs.

I believe that the added ifdefs are actually necessary, and they logically
separate libkrb5 functionality from libgssapi one. Without libkrb5 the only
things that won't be supported are custom credentials and the ticket cache,
everything else works just fine.
> AcquireCredentialsHandle won't be very useful without these krb5 functions.

It works just fine with cached system credentials.

[Hans Leidekker]

On Tue, 2019-07-02 at 18:14 +0800, Dmitry Timoshkov wrote:
> Sounds like a broken install. To fix this I think it would be better to improve
> > our configure check than to add more (nested) ifdefs.
>
> I believe that the added ifdefs are actually necessary, and they logically
> separate libkrb5 functionality from libgssapi one. Without libkrb5 the only
> things that won't be supported are custom credentials and the ticket cache,
> everything else works just fine.
> > AcquireCredentialsHandle won't be very useful without these krb5 functions.
>
> It works just fine with cached system credentials.

gssapi itself depends on krb5, so there's no reason to build with partial support.
The user will have it installed but can't make use of it, and the developer will
not be warned that support is missing.

[Dmitry Timoshkov]

Hans Leidekker  wrote:
> On Tue, 2019-07-02 at 18:14 +0800, Dmitry Timoshkov wrote:
> > Sounds like a broken install. To fix this I think it would be better to improve
> > > our configure check than to add more (nested) ifdefs.
> >
> > I believe that the added ifdefs are actually necessary, and they logically
> > separate libkrb5 functionality from libgssapi one. Without libkrb5 the only
> > things that won't be supported are custom credentials and the ticket cache,
> > everything else works just fine.
> > > AcquireCredentialsHandle won't be very useful without these krb5 functions.
> >
> > It works just fine with cached system credentials.
>
> gssapi itself depends on krb5, so there's no reason to build with partial support.

Apparently there are configurations where that's not the case, and supporting them
isn't that hard as the patch shows.
> The user will have it installed but can't make use of it, and the developer will
> not be warned that support is missing.

I won't argue that whether it's a useful configuration, but the patch solves
both a compilation bug and and makes kerberos.dll at least partially working.

[Alexandre Julliard]

Dmitry Timoshkov  writes:
> Hans Leidekker  wrote:
> > On Tue, 2019-07-02 at 18:14 +0800, Dmitry Timoshkov wrote:
> > > Sounds like a broken install. To fix this I think it would be better to improve
> > > > our configure check than to add more (nested) ifdefs.
> > >
> > > I believe that the added ifdefs are actually necessary, and they logically
> > > separate libkrb5 functionality from libgssapi one. Without libkrb5 the only
> > > things that won't be supported are custom credentials and the ticket cache,
> > > everything else works just fine.
> > > > AcquireCredentialsHandle won't be very useful without these krb5 functions.
> > >
> > > It works just fine with cached system credentials.
> >
> > gssapi itself depends on krb5, so there's no reason to build with partial support.
>
> Apparently there are configurations where that's not the case, and supporting them
> isn't that hard as the patch shows.

Have you verified that the library is truly missing, or is it only the
.so symlink from the dev package?

[Dmitry Timoshkov]

Alexandre Julliard  wrote:
> > Hans Leidekker  wrote:
> > > On Tue, 2019-07-02 at 18:14 +0800, Dmitry Timoshkov wrote:
> > > > Sounds like a broken install. To fix this I think it would be better to improve
> > > > > our configure check than to add more (nested) ifdefs.
> > > >
> > > > I believe that the added ifdefs are actually necessary, and they logically
> > > > separate libkrb5 functionality from libgssapi one. Without libkrb5 the only
> > > > things that won't be supported are custom credentials and the ticket cache,
> > > > everything else works just fine.
> > > > > AcquireCredentialsHandle won't be very useful without these krb5 functions.
> > > >
> > > > It works just fine with cached system credentials.
> > >
> > > gssapi itself depends on krb5, so there's no reason to build with partial support.
> >
> > Apparently there are configurations where that's not the case, and supporting them
> > isn't that hard as the patch shows.
>
> Have you verified that the library is truly missing, or is it only the
> .so symlink from the dev package?

I've relayed your question, and got the following answer: "only the dev
package with the .so symlink was missing".

[Alexandre Julliard]

Dmitry Timoshkov  writes:
> Alexandre Julliard  wrote:
> > > Hans Leidekker  wrote:
> > > > On Tue, 2019-07-02 at 18:14 +0800, Dmitry Timoshkov wrote:
> > > > > Sounds like a broken install. To fix this I think it would be better to improve
> > > > > > our configure check than to add more (nested) ifdefs.
> > > > >
> > > > > I believe that the added ifdefs are actually necessary, and they logically
> > > > > separate libkrb5 functionality from libgssapi one. Without libkrb5 the only
> > > > > things that won't be supported are custom credentials and the ticket cache,
> > > > > everything else works just fine.
> > > > > > AcquireCredentialsHandle won't be very useful without these krb5 functions.
> > > > >
> > > > > It works just fine with cached system credentials.
> > > >
> > > > gssapi itself depends on krb5, so there's no reason to build with partial support.
> > >
> > > Apparently there are configurations where that's not the case, and supporting them
> > > isn't that hard as the patch shows.
> >
> > Have you verified that the library is truly missing, or is it only the
> > .so symlink from the dev package?
>
> I've relayed your question, and got the following answer: "only the dev
> package with the .so symlink was missing".

OK, thanks. It seems that failing at configure time with a warning is
more appropriate then.

[Dmitry Timoshkov]

Alexandre Julliard  wrote:
> Dmitry Timoshkov  writes:
> > Alexandre Julliard  wrote:
> > > > Hans Leidekker  wrote:
> > > > > On Tue, 2019-07-02 at 18:14 +0800, Dmitry Timoshkov wrote:
> > > > > > Sounds like a broken install. To fix this I think it would be better to improve
> > > > > > > our configure check than to add more (nested) ifdefs.
> > > > > >
> > > > > > I believe that the added ifdefs are actually necessary, and they logically
> > > > > > separate libkrb5 functionality from libgssapi one. Without libkrb5 the only
> > > > > > things that won't be supported are custom credentials and the ticket cache,
> > > > > > everything else works just fine.
> > > > > >
> > > > > > > AcquireCredentialsHandle won't be very useful without these krb5 functions.
> > > > > >
> > > > > > It works just fine with cached system credentials.
> > > > >
> > > > > gssapi itself depends on krb5, so there's no reason to build with partial support.
> > > >
> > > > Apparently there are configurations where that's not the case, and supporting them
> > > > isn't that hard as the patch shows.
> > >
> > > Have you verified that the library is truly missing, or is it only the
> > > .so symlink from the dev package?
> >
> > I've relayed your question, and got the following answer: "only the dev
> > package with the .so symlink was missing".
>
> OK, thanks. It seems that failing at configure time with a warning is
> more appropriate then.

I agree, taking into account addtional information.