Thread: [PATCH] pglister: auth_receive: Indicate when PGAUTH_KEY is invalid instead of crashing

When Django is improperly configured with a PGAUTH_KEY that does not match the one configured in pgweb, the decryptor
returnsa string that can't be utf-8-decoded. Catching this error makes it possible to return a proper error message
insteadof a 500 error.
 
Of course, the unicode error can be due to other issues (e.g. tempered GET parameters), but wrong PGAUTH_KEY is the
morelikely issue to happen when configuring pglister.
 

Patch attached.
-- 
Célestin Matte

Update: attached patched also catch ValueError exceptions, which can happen as well.

Also attaching similar patch for pgarchives.

As being confused by the lack of error message happened to me again, I think these patches are useful.
-- 
Célestin Matte

On Mon, Feb 20, 2023 at 7:36 PM Célestin Matte <celestin.matte@cmatte.me> wrote:
>
> Update: attached patched also catch ValueError exceptions, which can happen as well.
>
> Also attaching similar patch for pgarchives.
>
> As being confused by the lack of error message happened to me again, I think these patches are useful.

It seems a patch like this was already merged into the upstream of
this, and got deployed into pgarchives & pglister when I synced those
up a couple of weeks ago. Sorry about the lateness of that one. Does
the current version of the code work for what you were looking for?

It might also be worth considering also checking explicitly for the
format of PGAUTH_KEY on startup. Something like the attached maybe?
(against pgweb which is the master of this file) What do you think?

--
 Magnus Hagander
 Me: https://www.hagander.net/
 Work: https://www.redpill-linpro.com/

> It seems a patch like this was already merged into the upstream of
> this, and got deployed into pgarchives & pglister when I synced those
> up a couple of weeks ago. Sorry about the lateness of that one. Does
> the current version of the code work for what you were looking for?

This is the same as what my patch does, indeed
  
> It might also be worth considering also checking explicitly for the
> format of PGAUTH_KEY on startup. Something like the attached maybe?
> (against pgweb which is the master of this file) What do you think?

LGTM

-- 
Célestin Matte

On Thu, Jul 13, 2023 at 3:03 PM Célestin Matte <celestin.matte@cmatte.me> wrote:
>
> > It seems a patch like this was already merged into the upstream of
> > this, and got deployed into pgarchives & pglister when I synced those
> > up a couple of weeks ago. Sorry about the lateness of that one. Does
> > the current version of the code work for what you were looking for?
>
> This is the same as what my patch does, indeed

Thanks for confirming!


> > It might also be worth considering also checking explicitly for the
> > format of PGAUTH_KEY on startup. Something like the attached maybe?
> > (against pgweb which is the master of this file) What do you think?
>
> LGTM

I've applied this one to the upstream repo. Will merge it into the
other repos when it's time for the next round of updates there, I
don't think it's high prio enough to do a "all around run".

//Magnus