Re: [PATCH] Fixed malformed error message on malformed SCRAM message.

From: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
To: Noah Misch <noah(at)leadboat(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Daniele Varrazzo <daniele(dot)varrazzo(at)gmail(dot)com>, pgsql-bugs <pgsql-bugs(at)postgresql(dot)org>
Subject: Re: [PATCH] Fixed malformed error message on malformed SCRAM message.
Date: 2017-06-07 14:48:23
Message-ID: 3cc82687-ee65-d0f0-efc2-ddad60ab206f@iki.fi
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

On 06/02/2017 09:32 AM, Noah Misch wrote:
> On Fri, Jun 02, 2017 at 02:20:00AM -0400, Tom Lane wrote:
>> BTW, since you mention COMMERROR uses in auth.c, isn't the usage at
>> line 687 wrong? It sure looks like the author supposed that that
>> ereport call wouldn't return, but it will. Adjacent similar calls
>> clean up and return NULL.
>
> Probably, though one could argue for proceeding with the short password.
> Deserves a comment if log-only is intentional.

Let's turn it into an ERROR.

> The lack of an exit after COMMERROR "client selected an invalid SASL
> authentication mechanism" looks like a bug.

Yes. That was fixed in commit 505b5d2f86 already.

Taking all the comments in this thread into account, and a few more
things that I spotted while looking at the error messages, I came up
with the attached patch. It includes the changes from Michael's patch
upthread to use errdetail() in the SCRAM errors, and it turns the
protocol violation errors in auth.c from COMMERROR into ERROR. See
commit message for more details. Barring objections, I'll push this
tomorrow.

- Heikki

Attachment Content-Type Size
0001-Improve-authentication-error-messages.patch application/x-download 10.7 KB

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Joe Conway 2017-06-07 15:45:20 Re: BUG #14682: row level security not work with partitioned table
Previous Message David G. Johnston 2017-06-07 14:36:45 Re: BUG #14693: create materialized view forces btrim

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Khandekar 2017-06-07 14:49:00 Re: UPDATE of partition key
Previous Message Robert Haas 2017-06-07 14:43:16 Re: Use of non-restart-safe storage by temp_tablespaces