Thread: PAM patch...
The attached diff fixes a bug in the PAM authentication code, which was
causing PAM to report the wrong message upon error. (It would report the
error for the second-to-last error condition after the last function would
fail (makes sense?)
--
Dominic J. Eidson
"Baruk Khazad! Khazad ai-menu!" - Gimli
-------------------------------------------------------------------------------
http://www.the-infinite.org/ http://www.the-infinite.org/~dominic/
Attachment
Not sure if this is 7.2.1 material because it is only an error message
reporting problem. Comments?
This has been saved for the 7.3 release:
http://candle.pha.pa.us/cgi-bin/pgpatches2
---------------------------------------------------------------------------
Dominic J. Eidson wrote:
>
> The attached diff fixes a bug in the PAM authentication code, which was
> causing PAM to report the wrong message upon error. (It would report the
> error for the second-to-last error condition after the last function would
> fail (makes sense?)
>
>
> --
> Dominic J. Eidson
> "Baruk Khazad! Khazad ai-menu!" - Gimli
> -------------------------------------------------------------------------------
> http://www.the-infinite.org/ http://www.the-infinite.org/~dominic/
Content-Description:
[ Attachment, skipping... ]
>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
> (send "unregister YourEmailAddressHere" to majordomo@postgresql.org)
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Not sure if this is 7.2.1 material because it is only an error message
> reporting problem. Comments?
Looks like a bug fix to me. Since it can't possibly break anything
except PAM, I'd rate it as low-risk; hence, reasonable to apply to 7.2.*
too, once we're happy with it.
However, I don't like the patch as given. It seems to perpetuate rather
than replace the fundamentally bogus coding style that led to the
problem in the first place. ISTM that we have a sequence of PAM calls
here that each might fail, and if so we want to report an appropriate
message and return STATUS_ERROR, rather than proceed with the next call.
But as coded, the error handling for call X appears after call X+1!
No wonder it was broken; who could understand this?
I think that the coding pattern shown in lines 746-760 is good:
retval = pam_something(params);
if (retval != PAM_SUCCESS)
{
generate error message;
clean up state as needed;
return STATUS_ERROR;
}
and that the right fix is to make each of the subsequent calls be in
this same pattern, not to try to emulate their nonsensical style.
regards, tom lane
On Fri, 22 Feb 2002, Tom Lane wrote:
> I think that the coding pattern shown in lines 746-760 is good:
>
> retval = pam_something(params);
>
> if (retval != PAM_SUCCESS)
> {
> generate error message;
> clean up state as needed;
> return STATUS_ERROR;
> }
>
> and that the right fix is to make each of the subsequent calls be in
> this same pattern, not to try to emulate their nonsensical style.
That's fair - there was some (weird?) reason it was done the other way in
the example code I based that section off of, but I don't remember
specifically why.
I'll make those changes, and resubmit.
--
Dominic J. Eidson
"Baruk Khazad! Khazad ai-menu!" - Gimli
-------------------------------------------------------------------------------
http://www.the-infinite.org/ http://www.the-infinite.org/~dominic/
On Fri, 22 Feb 2002, Tom Lane wrote:
> and that the right fix is to make each of the subsequent calls be in
> this same pattern, not to try to emulate their nonsensical style.
See attached patch.
--
Dominic J. Eidson
"Baruk Khazad! Khazad ai-menu!" - Gimli
-------------------------------------------------------------------------------
http://www.the-infinite.org/ http://www.the-infinite.org/~dominic/
Attachment
Your patch has been added to the PostgreSQL unapplied patches list at:
http://candle.pha.pa.us/cgi-bin/pgpatches
I will try to apply it within the next 48 hours.
---------------------------------------------------------------------------
Dominic J. Eidson wrote:
> On Fri, 22 Feb 2002, Tom Lane wrote:
>
> > and that the right fix is to make each of the subsequent calls be in
> > this same pattern, not to try to emulate their nonsensical style.
>
> See attached patch.
>
>
> --
> Dominic J. Eidson
> "Baruk Khazad! Khazad ai-menu!" - Gimli
> -------------------------------------------------------------------------------
> http://www.the-infinite.org/ http://www.the-infinite.org/~dominic/
Content-Description:
[ Attachment, skipping... ]
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026
Patch applied to 7.2.X and 7.3. Thanks. --------------------------------------------------------------------------- Dominic J. Eidson wrote: > On Fri, 22 Feb 2002, Tom Lane wrote: > > > and that the right fix is to make each of the subsequent calls be in > > this same pattern, not to try to emulate their nonsensical style. > > See attached patch. > > > -- > Dominic J. Eidson > "Baruk Khazad! Khazad ai-menu!" - Gimli > ------------------------------------------------------------------------------- > http://www.the-infinite.org/ http://www.the-infinite.org/~dominic/ Content-Description: [ Attachment, skipping... ] > > ---------------------------(end of broadcast)--------------------------- > TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026