Patchwork Patch to get cifs.upcall to compile with Heimdal

login
register
mail settings
Submitter Torsten Kurbad
Date April 1, 2010, 12:01 p.m.
Message ID <20100401140142.00488990@atalante.iwm-kmrc.de>
Download mbox | patch
Permalink /patch/49208/
State New
Headers show

Comments

Torsten Kurbad - April 1, 2010, 12:01 p.m.
Hi Jeff,

find attached a new version of the patch against current git.

On Wed, 31 Mar 2010 15:16:07 -0400, Jeff Layton <jlayton@samba.org>
wrote:
>>  #include <getopt.h>
>> +#ifdef HAVE_KRB5_KRB5_H
>>  #include <krb5/krb5.h>
>> +#elif defined(HAVE_KRB5_H)
>> +#include <krb5.h>
>> +#endif
>
>^^^^
>I think it would be better to munge the CFLAGS so that
>-I/usr/include/krb5 precedes the other paths if HAVE_KRB5_KRB5_H is
>set. That would leave cifs.upcall.c with less #ifdef goop.

The new patch incorporates a quick hack to accomplish this. I didn't do
much refinement here yet, since there's one problem: We don't really
know the "base directory" of the headers. Consider an installation
where the krb5.h resides in /usr/local/include/krb5/. User invokes
configure by setting CPPFLAGS="-I/usr/local/include", AC_CHECK_HEADERS
_does_ find <krb5/krb5.h>, but setting "-I/usr/include/krb5" would not
lead to the desired result... So, there must be a way to find out,
_where_ exactly AC_CHECK_HEADERS found krb5.h. I don't know how to do
that, though, even after some very intense googling. ;-)

>>  
>> +#ifdef HAVE_KRB5_AUTH_CON_GETSENDSUBKEY
>>  	ret = krb5_auth_con_getsendsubkey(context, auth_context,
>> &tokb); +#else
>> +	ret = krb5_auth_con_getlocalsubkey(context, auth_context,
>> &tokb); +#endif

I fixed that one, too, similar to how we handle
HAVE_KRB5_PRINCIPAL_GET_REALM.

>> +
>> +#ifdef HAVE_KRB5_KEYBLOCK_KEYVALUE /* Heimdal */
>> +	*sess_key = data_blob(tokb->keyvalue.data,
>> tokb->keyvalue.length); +#else /* MIT */
>>  	*sess_key = data_blob(tokb->contents, tokb->length);
>> +#endif
>>  
>
>I think it would be better to declare the same macros that samba uses:
>
>KRB5_KEY_DATA
>KRB5_KEY_LENGTH

I put the necessary macros in replace.h. Looks much clea(n|r)er now.

Best regards,
Torsten
Jeff Layton - April 1, 2010, 12:12 p.m.
On Thu, 1 Apr 2010 14:01:42 +0200
Torsten Kurbad <samba-technical@tk-webart.de> wrote:

> Hi Jeff,
> 
> find attached a new version of the patch against current git.
> 
> On Wed, 31 Mar 2010 15:16:07 -0400, Jeff Layton <jlayton@samba.org>
> wrote:
> >>  #include <getopt.h>
> >> +#ifdef HAVE_KRB5_KRB5_H
> >>  #include <krb5/krb5.h>
> >> +#elif defined(HAVE_KRB5_H)
> >> +#include <krb5.h>
> >> +#endif
> >
> >^^^^
> >I think it would be better to munge the CFLAGS so that
> >-I/usr/include/krb5 precedes the other paths if HAVE_KRB5_KRB5_H is
> >set. That would leave cifs.upcall.c with less #ifdef goop.
> 
> The new patch incorporates a quick hack to accomplish this. I didn't do
> much refinement here yet, since there's one problem: We don't really
> know the "base directory" of the headers. Consider an installation
> where the krb5.h resides in /usr/local/include/krb5/. User invokes
> configure by setting CPPFLAGS="-I/usr/local/include", AC_CHECK_HEADERS
> _does_ find <krb5/krb5.h>, but setting "-I/usr/include/krb5" would not
> lead to the desired result... So, there must be a way to find out,
> _where_ exactly AC_CHECK_HEADERS found krb5.h. I don't know how to do
> that, though, even after some very intense googling. ;-)
> 

That's a very valid point. I'm not sure of the proper way to do this,
so let's just go with your original fix for the includes. We can
consider changing it later if we find a cleaner way to do it.

> >>  
> >> +#ifdef HAVE_KRB5_AUTH_CON_GETSENDSUBKEY
> >>  	ret = krb5_auth_con_getsendsubkey(context, auth_context,
> >> &tokb); +#else
> >> +	ret = krb5_auth_con_getlocalsubkey(context, auth_context,
> >> &tokb); +#endif
> 
> I fixed that one, too, similar to how we handle
> HAVE_KRB5_PRINCIPAL_GET_REALM.
> 
> >> +
> >> +#ifdef HAVE_KRB5_KEYBLOCK_KEYVALUE /* Heimdal */
> >> +	*sess_key = data_blob(tokb->keyvalue.data,
> >> tokb->keyvalue.length); +#else /* MIT */
> >>  	*sess_key = data_blob(tokb->contents, tokb->length);
> >> +#endif
> >>  
> >
> >I think it would be better to declare the same macros that samba uses:
> >
> >KRB5_KEY_DATA
> >KRB5_KEY_LENGTH
> 
> I put the necessary macros in replace.h. Looks much clea(n|r)er now.
> 
> Best regards,
> Torsten
> -- 
> There is one way to find out if a man is honest -- ask him.  If he says
> "Yes" you know he is crooked.
> 		-- Groucho Marx

Much better.

Thanks,

Patch

From 47d0e78b710bb46b9cb20e686f84a063d1161aaa Mon Sep 17 00:00:00 2001
From: Torsten Kurbad <torsten@tk-webart.de>
Date: Thu, 1 Apr 2010 13:50:40 +0200
Subject: [PATCH] cifs-utils heimdal compatibility

---
 Makefile.am   |    6 ++++++
 cifs.upcall.c |   14 +++++++++++---
 configure.ac  |   49 +++++++++++++++++++++++++++++++++++++++----------
 replace.h     |   13 +++++++++++++
 4 files changed, 69 insertions(+), 13 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index fea8bdc..f5d91cf 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -9,6 +9,12 @@  if CONFIG_CIFSUPCALL
 sbin_PROGRAMS = cifs.upcall
 cifs_upcall_SOURCES = cifs.upcall.c data_blob.c asn1.c spnego.c util.c
 cifs_upcall_LDADD = -ltalloc -lkrb5 -lkeyutils
+
+# TODO: This needs to be refined
+if HAVE_KRB5_KRB5_H
+AM_CPPFLAGS = -I/usr/include/krb5
+endif
+
 man_MANS += cifs.upcall.8
 endif
 
diff --git a/cifs.upcall.c b/cifs.upcall.c
index a81eb24..5a63f39 100644
--- a/cifs.upcall.c
+++ b/cifs.upcall.c
@@ -31,7 +31,7 @@  create dns_resolver * * /usr/local/sbin/cifs.upcall %k
 
 #include <string.h>
 #include <getopt.h>
-#include <krb5/krb5.h>
+#include <krb5.h>
 #include <syslog.h>
 #include <dirent.h>
 #include <sys/types.h>
@@ -92,6 +92,15 @@  void krb5_free_unparsed_name(krb5_context context, char *val)
 }
 #endif
 
+#if !defined(HAVE_KRB5_AUTH_CON_GETSENDSUBKEY) /* Heimdal */
+krb5_error_code krb5_auth_con_getsendsubkey(krb5_context context,
+				krb5_auth_context auth_context, 
+				krb5_keyblock **keyblock)
+{
+	return krb5_auth_con_getlocalsubkey(context, auth_context, keyblock);
+}
+#endif
+
 /* does the ccache have a valid TGT? */
 static time_t
 get_tgt_time(const char *ccname) {
@@ -275,7 +284,6 @@  cifs_krb5_get_req(const char *principal, const char *ccname,
 		goto out_free_principal;
 	}
 
-	in_creds.keyblock.enctype = 0;
 	ret = krb5_get_credentials(context, 0, ccache, &in_creds, &out_creds);
 	krb5_free_principal(context, in_creds.server);
 	if (ret) {
@@ -302,7 +310,7 @@  cifs_krb5_get_req(const char *principal, const char *ccname,
 	}
 
 	*mechtoken = data_blob(apreq_pkt.data, apreq_pkt.length);
-	*sess_key = data_blob(tokb->contents, tokb->length);
+	*sess_key = data_blob(KRB5_KEY_DATA(tokb), KRB5_KEY_LENGTH(tokb));
 
 	krb5_free_keyblock(context, tokb);
 out_free_creds:
diff --git a/configure.ac b/configure.ac
index 9f00bea..b031642 100644
--- a/configure.ac
+++ b/configure.ac
@@ -19,20 +19,40 @@  AC_ARG_ENABLE(cifsupcall,
 AC_PROG_CC
 AC_GNU_SOURCE
 
-# Checks for libraries.
-
 # Checks for header files.
 AC_CHECK_HEADERS([arpa/inet.h fcntl.h inttypes.h limits.h mntent.h netdb.h stddef.h stdint.h stdlib.h string.h strings.h sys/mount.h sys/param.h sys/socket.h sys/time.h syslog.h unistd.h], , [AC_MSG_ERROR([necessary header(s) not found])])
 
 if test $enable_cifsupcall != "no"; then
-	AC_CHECK_HEADERS([krb5/krb5.h], ,[
-				if test "$enable_cifsupcall" = "yes"; then
-					AC_MSG_ERROR([krb5/krb5.h not found, consider installing krb5-libs-devel.])
-				else
-					AC_MSG_WARN([krb5/krb5.h not found, consider installing krb5-libs-devel. Disabling cifs.upcall.])
-					enable_cifsupcall="no"
-				fi
-			])
+	AC_CHECK_HEADERS([krb5.h krb5/krb5.h])
+	if test x$ac_cv_header_krb5_krb5_h != xyes ; then
+		if test x$ac_cv_header_krb5_h != xyes ; then
+			if test "$enable_cifsupcall" = "yes"; then
+				AC_MSG_ERROR([krb5.h not found, consider installing krb5-libs-devel.])
+			else
+				AC_MSG_WARN([krb5.h not found, consider installing krb5-libs-devel. Disabling cifs.upcall.])
+				enable_cifsupcall="no"
+			fi
+		fi
+	fi
+fi
+if test $enable_cifsupcall != "no"; then
+	if test x$ac_cv_header_krb5_krb5_h = xyes ; then
+		krb5_include="#include <krb5/krb5.h>"
+	fi
+	if test x$ac_cv_header_krb5_h = xyes ; then
+		krb5_include="#include <krb5.h>"
+	fi
+
+	AC_CACHE_CHECK([for keyvalue in krb5_keyblock],
+		[ac_cv_have_krb5_keyblock_keyvalue],[
+			AC_TRY_COMPILE([$krb5_include],
+			[krb5_keyblock key; key.keyvalue.data = NULL;],
+			ac_cv_have_krb5_keyblock_keyvalue=yes,
+			ac_cv_have_krb5_keyblock_keyvalue=no)])
+	if test x"$ac_cv_have_krb5_keyblock_keyvalue" = x"yes" ; then
+		AC_DEFINE(HAVE_KRB5_KEYBLOCK_KEYVALUE,1,
+			[Whether the krb5_keyblock struct has a keyvalue property])
+	fi
 fi
 if test $enable_cifsupcall != "no"; then
 	AC_CHECK_HEADERS([talloc.h], , [
@@ -54,6 +74,9 @@  if test $enable_cifsupcall != "no"; then
 				fi
 			])
 fi
+if test $enable_cifsupcall != "no"; then
+	AC_CHECK_LIB([krb5], [krb5_init_context])
+fi
 
 # Checks for typedefs, structures, and compiler characteristics.
 AC_HEADER_STDBOOL
@@ -73,11 +96,17 @@  AC_FUNC_STRNLEN
 # check for required functions
 AC_CHECK_FUNCS([alarm atexit endpwent getmntent getpass gettimeofday inet_ntop memset realpath setenv strchr strdup strerror strncasecmp strndup strpbrk strrchr strstr strtol strtoul uname], , [AC_MSG_ERROR([necessary functions(s) not found])])
 
+# determine whether we can use MIT's new 'krb5_auth_con_getsendsubkey' to extract the signing key
+if test $enable_cifsupcall != "no"; then
+	AC_CHECK_FUNCS([krb5_auth_con_getsendsubkey])
+fi
+
 # non-critical functions (we have workarounds for these)
 if test $enable_cifsupcall != "no"; then
 	AC_CHECK_FUNCS([krb5_principal_get_realm krb5_free_unparsed_name])
 fi
 
 AM_CONDITIONAL(CONFIG_CIFSUPCALL, [test "$enable_cifsupcall" != "no"])
+AM_CONDITIONAL(HAVE_KRB5_KRB5_H, [test "$ac_cv_header_krb5_krb5_h" = "yes"])
 
 AC_OUTPUT
diff --git a/replace.h b/replace.h
index 69cf776..ee4d618 100644
--- a/replace.h
+++ b/replace.h
@@ -666,4 +666,17 @@  typedef uint32_t NTSTATUS;
 #define NT_STATUS(x) (x)
 #define NT_STATUS_V(x) (x)
 
+/* These macros unify the keyblock handling of Heimdal and MIT somewhat */
+#ifdef HAVE_KRB5_KEYBLOCK_KEYVALUE /* Heimdal */
+#define KRB5_KEY_TYPE(k)        ((k)->keytype)
+#define KRB5_KEY_LENGTH(k)      ((k)->keyvalue.length)
+#define KRB5_KEY_DATA(k)        ((k)->keyvalue.data)
+#define KRB5_KEY_DATA_CAST      void
+#else /* MIT */
+#define KRB5_KEY_TYPE(k)        ((k)->enctype)
+#define KRB5_KEY_LENGTH(k)      ((k)->length)
+#define KRB5_KEY_DATA(k)        ((k)->contents)
+#define KRB5_KEY_DATA_CAST      krb5_octet
+#endif
+
 #endif /* _LIBREPLACE_REPLACE_H */
-- 
1.7.0.3