On 03/12/2016 04:34 PM, Sumit Bose wrote:
On Fri, Mar 11, 2016 at 01:36:52PM +0100, Pavel Březina wrote:
> On 03/11/2016 10:41 AM, Sumit Bose wrote:
>> On Thu, Mar 10, 2016 at 12:54:15PM +0100, Pavel Březina wrote:
>>> On 03/08/2016 05:55 PM, Sumit Bose wrote:
>>>> Hi,
>>>>
>>>> This patch fixes a 2FA issues observed with sudo. See commit message for
>>>> details.
>>>>
>>>> bye,
>>>> Sumit
>>>>
>>>>
>>>> 0001-pam_sss-reorder-pam_message-array.patch
>>>>
>>>>
>>>> From 2c38adad7b527aceb4f9cb41c7d7b4c66d4580c9 Mon Sep 17 00:00:00 2001
>>>> From: Sumit Bose<sbose(a)redhat.com>
>>>> Date: Mon, 7 Mar 2016 17:07:16 +0100
>>>> Subject: [PATCH] pam_sss: reorder pam_message array
>>>>
>>>> There are different expectations about how the pam_message array is
>>>> organized, details can be found in the pam_start man page. E.g. sudo was
>>>> not able to handle the Linux-PAM style but expected the Solaris PAM
>>>> style. With this patch both styles should work as expected.
>>>
>>> I don't see any detail explaining this in the man page... could you
>>> elaborate please?
>>
>> ah, sorry, it is not pam_start but the pam_conv man page:
>>
>> """
>> In passing, it is worth noting that there is a descrepency between the
>> way Linux-PAM handles the const struct pam_message **msg conversation
>> function argument from the way that Solaris' PAM (and derivitives, known
>> to include HP/UX, are there others?) does. Linux-PAM interprets the msg
>> argument as entirely equivalent to the following prototype const struct
>> pam_message *msg[] (which, in spirit, is consistent with the commonly
>> used prototypes for argv argument to the familiar main() function: char
>> **argv; and char *argv[]). Said another way Linux-PAM interprets the msg
>> argument as a pointer to an array of num_msg read only 'struct
>> pam_message' pointers. Solaris' PAM implementation interprets this
>> argument as a pointer to a pointer to an array of num_msg pam_message
>> structures. Fortunately, perhaps, for most module/application developers
>> when num_msg has a value of one these two definitions are entirely
>> equivalent. Unfortunately, casually raising this number to two has led
>> to unanticipated compatibility problems.
>>
>> For what its worth the two known module writer work-arounds for trying
>> to maintain source level compatibility with both PAM implementations
>> are:
>>
>> · never call the conversation function with num_msg greater
>> than one.
>>
>> · set up msg as doubly referenced so both types of conversation
>> function can find the messages. That is, make
>>
>> msg[n] = & (( *msg )[n])
>> """
>>>
>>>> - mesg[0] = (const struct pam_message *) m1;
>>>> - mesg[1] = (const struct pam_message *) m2;
>>>> + mesg[0] = (const struct pam_message *) m;
>>>> + mesg[1] = & (( *mesg )[1]);
>>>
>>> Is it possible to use &m[1] instead of this?
>>
>> I took this version to match the suggestion from the man page, but if
>> you agree I'll add a comment why this somewhat odd notation is used.
>
> Even &m[1] works, but it may be better to stick with manpage suggestion.
> Comment is welcomed though.
Please see attaches new version. I fixed the man page reference and added
an expectation for the assignment. Additionally I removed the malloc() to
underline even further that we have two arrays here.
bye,
Sumit
Ack.
http://sssd-ci.duckdns.org/logs/job/39/26/summary.html