Bug 6868 - make bin/cifs.upcall fails
make bin/cifs.upcall fails
Status: RESOLVED FIXED
Product: Samba 3.4
Classification: Unclassified
Component: Client Tools
3.4.3
x64 Linux
: P3 normal
: ---
Assigned To: Karolin Seeger
Samba QA Contact
http://bugs.gentoo.org/show_bug.cgi?i...
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-11-03 04:43 UTC by Marcel Greter
Modified: 2010-02-17 07:48 UTC (History)
1 user (show)

See Also:


Attachments
fix the build with heimdal (4.22 KB, patch)
2009-11-11 18:23 UTC, Guenther Deschner
jlayton: review+
Details
3-4-test version of the patch (4.30 KB, patch)
2009-11-24 04:33 UTC, Guenther Deschner
no flags Details
KRB5_TC_OPENCLOSE not defined in Heimdal (872 bytes, patch)
2009-11-25 14:53 UTC, Guenther Deschner
gd: review? (jlayton)
Details
patch for 3.5 (2.06 KB, patch)
2010-02-17 05:26 UTC, Guenther Deschner
gd: review+
Details
patch for 3.4 (2.10 KB, patch)
2010-02-17 05:26 UTC, Guenther Deschner
gd: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel Greter 2009-11-03 04:43:40 UTC
Build fails with this error:

client/cifs.upcall.c: In function ‘get_tgt_time’:
cifs.upcall.c:97: error: incompatible type for argument 1 of ‘k5_data_equal’
cifs.upcall.c:48: note: expected ‘krb5_data’ but argument is of type ‘Realm’
cifs.upcall.c:97: error: incompatible type for argument 2 of ‘k5_data_equal’
cifs.upcall.c:48: note: expected ‘krb5_data’ but argument is of type ‘Realm’
cifs.upcall.c:98: error: ‘struct Principal’ has no member named ‘data’
cifs.upcall.c:99: error: ‘struct Principal’ has no member named ‘data’
cifs.upcall.c:99: error: incompatible type for argument 2 of ‘k5_data_equal’
cifs.upcall.c:48: note: expected ‘krb5_data’ but argument is of type ‘Realm’
cifs.upcall.c:109: error: ‘KRB5_TC_OPENCLOSE’ undeclared (first use in this function)

It was working with 3.4.2, so I tracked down the problem to this commit:
http://gitweb.samba.org/?p=samba.git;a=commitdiff;h=704b739ad8b5441e4c84215044a77e74e54cf425

Some further investigations showed me:
# cat /usr/include/krb5_asn1.h | grep "Realm;$"
typedef heim_general_string Realm;

So to me it looks like heimdal was using krb5_data at some point and changed it to heim_general_string (checked heimdal-1.3.0rc1 and heimdal-1.2.1).

I also am missing KRB5_TC_OPENCLOSE here. Not sure if heimdal does not install the header or if this is gentoo specific. It does exist in the heimdal sources (appl/dceutils/k5dce.h). I'm currently using app-crypt/heimdal-1.2.1-r4.

You'll find a (hopefully correct) patch on the gentoo bugtracker that uses heim_general_string. I simply copied KRB5_TC_OPENCLOSE from the heimdal source, which is of course, bad.
Comment 1 Jeff Layton 2009-11-11 12:59:48 UTC
That patch may make it compile against heimdal, but it's quite likely to break compilation against MIT krb5. I think we'll need to add some sort of #ifdef trickery to make that work correctly.
Comment 2 Jeff Layton 2009-11-11 13:47:03 UTC
The part I'm not sure of is how to make autoconf check whether we're compiling against heimdal or mit krb5 (or something else entirely). The actual code changes don't look too hard once we get that part figured out.
Comment 3 Jeff Layton 2009-11-11 13:55:02 UTC
According to Jeremy, we'll probably need to do a compile test of some sort:

14:53 < wanon_home> Search for HAVE_KRB5_SESSION_IN_CREDS inside configure.in
14:53 < wanon_home> That's a check for a specific element in a data struct.
14:53 < jlayton> ahh ok, I'll check that out -- thanks
Comment 4 Guenther Deschner 2009-11-11 18:23:52 UTC
Created attachment 4952 [details]
fix the build with heimdal
Comment 5 Jeff Layton 2009-11-11 19:07:44 UTC
Comment on attachment 4952 [details]
fix the build with heimdal

Nice work.

Looks good to me assuming that krb5_princ_realm doesn't return anything that needs to be freed. As best I can tell it doesn't so...ACK!
Comment 6 Guenther Deschner 2009-11-12 03:28:03 UTC
pushed to master and v3-5-test, Karolin can you please pick this up for 3-4-test ?
Comment 7 Karolin Seeger 2009-11-23 01:55:32 UTC
Patch does not apply on v3-4-test:

user@host:/data/git/samba/v3-4-test> git am heimdal.patch                                                                                                         
Applying: s3-kerberos: add smb_krb5_principal_get_realm().                                                                                                            
error: patch failed: source3/include/includes.h:1086                                                                                                                  
error: source3/include/includes.h: patch does not apply                                                                                                               
error: patch failed: source3/libsmb/clikrb5.c:2234                                                                                                                    
error: source3/libsmb/clikrb5.c: patch does not apply                                                                                                                 
Patch failed at 0001 s3-kerberos: add smb_krb5_principal_get_realm().                                                 
Comment 8 Guenther Deschner 2009-11-24 04:33:28 UTC
Created attachment 4991 [details]
3-4-test version of the patch
Comment 9 Guenther Deschner 2009-11-24 04:33:58 UTC
Karolin, this one should work.
Comment 10 Karolin Seeger 2009-11-24 04:38:20 UTC
Thanks, Günther!
Pushed to v3-4-test.
Closing out bug report.
Comment 11 Guenther Deschner 2009-11-25 14:51:49 UTC
still one hunk missing.
Comment 12 Guenther Deschner 2009-11-25 14:53:27 UTC
Created attachment 5012 [details]
KRB5_TC_OPENCLOSE not defined in Heimdal

Jeff, I think it is ok to skip the setflags on the ccache when Heimdal does not know about these flags at all.
Comment 13 Guenther Deschner 2009-12-01 17:47:21 UTC
Fixed for next 3-4 and 3-3 release
Comment 14 Guenther Deschner 2010-02-16 08:41:13 UTC
arg, we cannot use NULL talloc_context in cifs.upcall as that creates malloced memory in v3-4-test (and v3-3-test)
Comment 15 Guenther Deschner 2010-02-16 09:35:23 UTC
(In reply to comment #14)
> arg, we cannot use NULL talloc_context in cifs.upcall as that creates malloced
> memory in v3-4-test (and v3-3-test)
> 

fix is here https://bugzilla.redhat.com/show_bug.cgi?id=565446. still needs a verify/test though.
Comment 16 Guenther Deschner 2010-02-17 05:26:19 UTC
Created attachment 5361 [details]
patch for 3.5
Comment 17 Guenther Deschner 2010-02-17 05:26:56 UTC
Created attachment 5362 [details]
patch for 3.4
Comment 18 Guenther Deschner 2010-02-17 05:28:27 UTC
ok, tested patch pushed to master, patches for v3-4-test and v3-5-test attached (v3-5-test and master dont strictly need the patch but we'd like to be consistent here), v3-3-test does *not* have that problem.

Karolin, please push to v3-5-test and v3-4-test
Comment 19 Karolin Seeger 2010-02-17 07:48:17 UTC
Pushed to both branches.
Will be included in 3.5.0 and 3.4.6.
Closing out bug report.

Thanks!