In wbcCredentialCache() about line 1228 of wbc_pam.c: for (i=0; i<params->num_blobs; i++) { if (strcasecmp(params->blobs[i].name, "initial_blob") == 0) { initial_blob = ¶ms->blobs[i]; break; } if (strcasecmp(params->blobs[i].name, "challenge_blob") == 0) { challenge_blob = ¶ms->blobs[i]; break; } } Don't break out of the loop as soon as you've found one.
Created attachment 10093 [details] git-am fix for master. Is this the fix you had in mind ?
Yes, that fixes the issue here. Thanks.
Created attachment 10103 [details] git-am fix for 4.1.next/4.0.next Patchset that went into master. Applies cleanly to 4.1.next and 4.0.next.
Hm, I'm not sure I like that last one ("reject unknown named blobs"). You've changed the backward compatibility story. As things stand, if we add a new optional blob in future, clients can just provide it and know that old implementations of libwbclient will ignore it. Now you've made sure they'll bail out when they see the new blob. I was actually thinking (vaguely) of doing precisely that. I've just hooked up Simo's GSS-NTLMSSP to use libwbclient (http://git.infradead.org/users/dwmw2/gss-ntlmssp.git) and it has some limitations like not allowing us to support the MIC or channel bindings... which may have been solved by allowing the client to pass the client_target_info in to wbcCredentialsCache() instead of letting winbind generate it for itself. I hadn't really fully thought that plan through though; maybe it couldn't have worked anyway...
(In reply to comment #4) > Hm, I'm not sure I like that last one ("reject unknown named blobs"). You've > changed the backward compatibility story. > > As things stand, if we add a new optional blob in future, clients can just > provide it and know that old implementations of libwbclient will ignore it. > Now you've made sure they'll bail out when they see the new blob. > > I was actually thinking (vaguely) of doing precisely that. I've just hooked up > Simo's GSS-NTLMSSP to use libwbclient > (http://git.infradead.org/users/dwmw2/gss-ntlmssp.git) and it has some > limitations like not allowing us to support the MIC or channel bindings... > which may have been solved by allowing the client to pass the > client_target_info in to wbcCredentialsCache() instead of letting winbind > generate it for itself. > > I hadn't really fully thought that plan through though; maybe it couldn't have > worked anyway... Ok, Jeremy I guess it's better to revert the last patch in that case...
(In reply to comment #5) > Ok, Jeremy I guess it's better to revert the last patch in that case... Well, I did say I hadn't really thought it through. In practice even if I do go with the idea of passing in the client_target_info, I'm probably still better off checking the libwbclient version *first* to see if it's supported, rather than just passing it in anyway and then looking at the response to see if winbind was new enough to do the right thing. But that specific example aside, it's still a change in your backward compatibility story that needs to be done intentionally if at all.
I hadn't considered the backwards compatibility story either. I'll revert that last patch and upload a new version without it. Thanks for the careful review David !
Created attachment 10107 [details] patch for 4.1.next, 4.0.next Removed last (reverted) component.
Pushed to autobuild-v4-1-test.
(In reply to comment #9) > Pushed to autobuild-v4-1-test. and autobuild-v4-0-test
Pushed to both branches. Closing out bug report. Thanks!