Horton

Thread

Commits

  1. Forbid gen_random_uuid() with --disable-strong-random

  1. gen_random_uuid security not explicit in documentation

    rightfold@gmail.com — 2017-01-01T23:20:54Z

    The following documentation comment has been logged on the website:

Page: https://www.postgresql.org/docs/9.6/static/pgcrypto.html
Description:

Hello,

The C source code of gen_random_uuid reads:

    /*
    * Generate random bits. pg_backend_random() will do here, we don't
    * promis UUIDs to be cryptographically random, when built with
    * --disable-strong-random.
    */

However, the pgcrypto documentation does not mention
--disable-strong-random
at all. I think the documentation should mention under which conditions
the
function returns secure data.

Radek Slupik

P.S. there is also a typo in the C comment: "promis" should be "promise".

  2. Re: gen_random_uuid security not explicit in documentation

    Michael Paquier <michael.paquier@gmail.com> — 2017-01-03T12:47:37Z

    (Adding Heikki in CC who committed this code)

On Mon, Jan 2, 2017 at 8:20 AM,  <rightfold@gmail.com> wrote:
> The C source code of gen_random_uuid reads:
>
>     /*
>     * Generate random bits. pg_backend_random() will do here, we don&#39;t
>     * promis UUIDs to be cryptographically random, when built with
>     * --disable-strong-random.
>     */
>
> However, the pgcrypto documentation does not mention
> --disable-strong-random
> at all. I think the documentation should mention under which conditions
> the function returns secure data.

That's actually a good idea. But as it does not only apply to
get_random_uuid(), I would think that a notice at the top of the
pgcrypto documentation would make the most sense. Something like:
"If PostgreSQL is built with --disable-strong-random, the data
generated by the functions is not guaranteed to be cryptographically
random."

> P.S. there is also a typo in the C comment: &quot;promis&quot; should be &quot;promise&quot;.

Indeed.
-- 
Michael

  3. Re: gen_random_uuid security not explicit in documentation

    Heikki Linnakangas <hlinnaka@iki.fi> — 2017-06-22T18:02:03Z

    On 01/03/2017 02:47 PM, Michael Paquier wrote:
> (Adding Heikki in CC who committed this code)
>
> On Mon, Jan 2, 2017 at 8:20 AM,  <rightfold@gmail.com> wrote:
>> The C source code of gen_random_uuid reads:
>>
>>     /*
>>     * Generate random bits. pg_backend_random() will do here, we don&#39;t
>>     * promis UUIDs to be cryptographically random, when built with
>>     * --disable-strong-random.
>>     */
>>
>> However, the pgcrypto documentation does not mention
>> --disable-strong-random
>> at all. I think the documentation should mention under which conditions
>> the function returns secure data.
>
> That's actually a good idea. But as it does not only apply to
> get_random_uuid(), I would think that a notice at the top of the
> pgcrypto documentation would make the most sense. Something like:
> "If PostgreSQL is built with --disable-strong-random, the data
> generated by the functions is not guaranteed to be cryptographically
> random."

Hmm, not sure what to do here. --disable-strong-random is similar to 
e.g. --disable-spinlocks; no reasonable production platform would use 
it. So I'm not inclined to sprinkle references to it across the docs, it 
seems better to document what it changes, in the description of 
--disable-strong-random itself.

To be pedantic, the documentation only claims that gen_random_bytes() 
returns cryptographically strong values. For gen_random_uuid(), it just 
says that it's "random". But yeah, it's subtle. By the feat of having 
them side-by-side, and a similar name, you'd think that they behave the 
same. And with --enable-strong-random, they do.

I'm inclined to change gen_random_uuid() to throw an error if the server 
is built with --disable-strong-random, like gen_random_bytes() does. 
That way, they would behave the same.

Thoughts?

- Heikki

  4. Re: gen_random_uuid security not explicit in documentation

    Michael Paquier <michael.paquier@gmail.com> — 2017-06-23T01:23:36Z

    On Fri, Jun 23, 2017 at 3:02 AM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> I'm inclined to change gen_random_uuid() to throw an error if the server is
> built with --disable-strong-random, like gen_random_bytes() does. That way,
> they would behave the same.

No objections to do that. I guess you don't need a patch. As this is
new to 10, I have added an open item.

> Thoughts?

There is this comment in pgcrypto.c with a typo:
     * Generate random bits. pg_backend_random() will do here, we don't promis
s/promis/promise/.
-- 
Michael

  5. Re: gen_random_uuid security not explicit in documentation

    Noah Misch <noah@leadboat.com> — 2017-06-26T04:26:28Z

    On Fri, Jun 23, 2017 at 10:23:36AM +0900, Michael Paquier wrote:
> On Fri, Jun 23, 2017 at 3:02 AM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> > I'm inclined to change gen_random_uuid() to throw an error if the server is
> > built with --disable-strong-random, like gen_random_bytes() does. That way,
> > they would behave the same.
> 
> No objections to do that. I guess you don't need a patch. As this is
> new to 10, I have added an open item.

[Action required within three days.  This is a generic notification.]

The above-described topic is currently a PostgreSQL 10 open item.  Heikki,
since you committed the patch believed to have created it, you own this open
item.  If some other commit is more relevant or if this does not belong as a
v10 open item, please let us know.  Otherwise, please observe the policy on
open item ownership[1] and send a status update within three calendar days of
this message.  Include a date for your subsequent status update.  Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping v10.  Consequently, I will appreciate your efforts
toward speedy resolution.  Thanks.

[1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com

  6. Re: gen_random_uuid security not explicit in documentation

    Noah Misch <noah@leadboat.com> — 2017-06-30T03:45:04Z

    On Sun, Jun 25, 2017 at 09:26:28PM -0700, Noah Misch wrote:
> On Fri, Jun 23, 2017 at 10:23:36AM +0900, Michael Paquier wrote:
> > On Fri, Jun 23, 2017 at 3:02 AM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> > > I'm inclined to change gen_random_uuid() to throw an error if the server is
> > > built with --disable-strong-random, like gen_random_bytes() does. That way,
> > > they would behave the same.
> > 
> > No objections to do that. I guess you don't need a patch. As this is
> > new to 10, I have added an open item.
> 
> [Action required within three days.  This is a generic notification.]
> 
> The above-described topic is currently a PostgreSQL 10 open item.  Heikki,
> since you committed the patch believed to have created it, you own this open
> item.  If some other commit is more relevant or if this does not belong as a
> v10 open item, please let us know.  Otherwise, please observe the policy on
> open item ownership[1] and send a status update within three calendar days of
> this message.  Include a date for your subsequent status update.  Testers may
> discover new open items at any time, and I want to plan to get them all fixed
> well in advance of shipping v10.  Consequently, I will appreciate your efforts
> toward speedy resolution.  Thanks.
> 
> [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com

This PostgreSQL 10 open item is past due for your status update.  Kindly send
a status update within 24 hours, and include a date for your subsequent status
update.  Refer to the policy on open item ownership:
https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com

  7. Re: gen_random_uuid security not explicit in documentation

    Heikki Linnakangas <hlinnaka@iki.fi> — 2017-06-30T05:24:01Z

    
On 30 June 2017 06:45:04 EEST, Noah Misch <noah@leadboat.com> wrote:
>This PostgreSQL 10 open item is past due for your status update. 
>Kindly send
>a status update within 24 hours, and include a date for your subsequent
>status
>update.

I'll fix this some time next week. (I'm on vacation right now)

- Heikki

  8. Re: gen_random_uuid security not explicit in documentation

    Heikki Linnakangas <hlinnaka@iki.fi> — 2017-07-03T09:22:25Z

    On 06/23/2017 04:23 AM, Michael Paquier wrote:
> On Fri, Jun 23, 2017 at 3:02 AM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>> I'm inclined to change gen_random_uuid() to throw an error if the server is
>> built with --disable-strong-random, like gen_random_bytes() does. That way,
>> they would behave the same.
>
> No objections to do that. I guess you don't need a patch. As this is
> new to 10, I have added an open item.

Ok, pushed a fix to do that.

Thanks for bringing this up, Radek!

- Heikki