gen_random_uuid security not explicit in documentation

Started by Nonameabout 9 years ago8 messages
#1Noname
rightfold@gmail.com

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".

--
Sent via pgsql-docs mailing list (pgsql-docs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-docs

#2Michael Paquier
michael.paquier@gmail.com
In reply to: Noname (#1)
Re: gen_random_uuid security not explicit in documentation

(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

--
Sent via pgsql-docs mailing list (pgsql-docs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-docs

#3Heikki Linnakangas
hlinnaka@iki.fi
In reply to: Michael Paquier (#2)
Re: gen_random_uuid security not explicit in documentation

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

--
Sent via pgsql-docs mailing list (pgsql-docs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-docs

#4Michael Paquier
michael.paquier@gmail.com
In reply to: Heikki Linnakangas (#3)
Re: gen_random_uuid security not explicit in documentation

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

--
Sent via pgsql-docs mailing list (pgsql-docs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-docs

#5Noah Misch
noah@leadboat.com
In reply to: Michael Paquier (#4)
Re: gen_random_uuid security not explicit in documentation

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]/messages/by-id/20170404140717.GA2675809@tornado.leadboat.com 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]: /messages/by-id/20170404140717.GA2675809@tornado.leadboat.com

--
Sent via pgsql-docs mailing list (pgsql-docs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-docs

#6Noah Misch
noah@leadboat.com
In reply to: Noah Misch (#5)
Re: gen_random_uuid security not explicit in documentation

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] /messages/by-id/20170404140717.GA2675809@tornado.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:
/messages/by-id/20170404140717.GA2675809@tornado.leadboat.com

--
Sent via pgsql-docs mailing list (pgsql-docs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-docs

#7Heikki Linnakangas
hlinnaka@iki.fi
In reply to: Noah Misch (#6)
Re: gen_random_uuid security not explicit in documentation

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

--
Sent via pgsql-docs mailing list (pgsql-docs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-docs

#8Heikki Linnakangas
hlinnaka@iki.fi
In reply to: Michael Paquier (#4)
Re: gen_random_uuid security not explicit in documentation

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

--
Sent via pgsql-docs mailing list (pgsql-docs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-docs