On Fri, Nov 20, 2020 at 3:31 AM Michael Paquier wrote:
>
> On Thu, Nov 19, 2020 at 09:49:05PM +0100, Magnus Hagander wrote:
> > Ugh, that's pretty terrible.
>
> I have spent some time testing this part this morning, and I can see
> that genhtml does not complain with your patch. It looks like in
> On 20 Nov 2020, at 03:31, Michael Paquier wrote
> pg_strong_random.c needs a pgindent run, there are two inconsistent
> diffs. Looks fine except for those nits.
Agreed, this is the least complicated (and most readable) we can make this
file, especially if we add more providers. +1.
cheers .
On Thu, Nov 19, 2020 at 09:49:05PM +0100, Magnus Hagander wrote:
> Ugh, that's pretty terrible.
I have spent some time testing this part this morning, and I can see
that genhtml does not complain with your patch. It looks like in the
case of 2771fce the tool got confused because the same function
On Thu, Nov 19, 2020 at 12:04 PM Michael Paquier wrote:
>
> On Thu, Nov 19, 2020 at 11:00:40AM +0100, Magnus Hagander wrote:
> > I'm thinking the code might get a lot cleaner if we just make a single
> > set of ifdefs, even if that means repeating the function header. In
> > theory we could put th
On Thu, Nov 19, 2020 at 11:00:40AM +0100, Magnus Hagander wrote:
> I'm thinking the code might get a lot cleaner if we just make a single
> set of ifdefs, even if that means repeating the function header. In
> theory we could put them in different *.c files as well, but that
> seems overkill given
On Thu, Nov 19, 2020 at 4:34 AM Michael Paquier wrote:
>
> On Wed, Nov 18, 2020 at 10:43:35AM +0100, Daniel Gustafsson wrote:
> > While it does simplify configure.ac, I'm just not a fan of the strict
> > ordering
> > which is required without the labels even implying it. But that might just
> >
> On 19 Nov 2020, at 04:34, Michael Paquier wrote:
>
> On Wed, Nov 18, 2020 at 10:43:35AM +0100, Daniel Gustafsson wrote:
>> While it does simplify configure.ac, I'm just not a fan of the strict
>> ordering
>> which is required without the labels even implying it. But that might just
>> be
>>
On Wed, Nov 18, 2020 at 10:43:35AM +0100, Daniel Gustafsson wrote:
> While it does simplify configure.ac, I'm just not a fan of the strict ordering
> which is required without the labels even implying it. But that might just be
> my personal preference.
I just looked at that, and the attached see
> On 18 Nov 2020, at 09:54, Michael Paquier wrote:
>
> On Wed, Nov 18, 2020 at 09:25:44AM +0100, Daniel Gustafsson wrote:
>> Technically that is what it does, except for setting the USE_*RANDOM
>> variables
>> for non-OpenSSL builds. We could skip that too, which I think is what you're
>> propo
On Wed, Nov 18, 2020 at 09:25:44AM +0100, Daniel Gustafsson wrote:
> Technically that is what it does, except for setting the USE_*RANDOM variables
> for non-OpenSSL builds. We could skip that too, which I think is what you're
> proposing, but it seems to me that we'll end up with another set of e
> On 18 Nov 2020, at 02:31, Michael Paquier wrote:
>
> On Tue, Nov 17, 2020 at 09:24:30PM +0100, Daniel Gustafsson wrote:
>> I tend to agree, randomness is complicated enough without adding a compile
>> time
>> extensibility which few (if anyone) will ever use. Attached is an attempt at
>> this
On Tue, Nov 17, 2020 at 09:24:30PM +0100, Daniel Gustafsson wrote:
> I tend to agree, randomness is complicated enough without adding a compile
> time
> extensibility which few (if anyone) will ever use. Attached is an attempt at
> this.
Going down to that, it seems to me that we could just remo
> On 16 Nov 2020, at 16:06, Tom Lane wrote:
>
> Magnus Hagander writes:
>> I agree with those -- either we remove the ability to choose random source
>> independently of the SSL library (and then only use the windows crypto
>> provider or /dev/urandom as platform-specific choices when *no* SSL l
Magnus Hagander writes:
> I agree with those -- either we remove the ability to choose random source
> independently of the SSL library (and then only use the windows crypto
> provider or /dev/urandom as platform-specific choices when *no* SSL library
> is used), and in that case we should not hav
On Mon, Nov 16, 2020 at 10:19 AM Daniel Gustafsson wrote:
> > On 16 Nov 2020, at 01:20, Michael Paquier wrote:
> >
> > On Sun, Nov 15, 2020 at 12:16:56PM -0500, Tom Lane wrote:
> >> The obvious problem with this is that if !USE_OPENSSL, we will not have
> >> pulled in openssl's headers.
> >
> >
> On 16 Nov 2020, at 01:20, Michael Paquier wrote:
>
> On Sun, Nov 15, 2020 at 12:16:56PM -0500, Tom Lane wrote:
>> The obvious problem with this is that if !USE_OPENSSL, we will not have
>> pulled in openssl's headers.
>
> FWIW, I argued upthread against including this part because it is
> usel
On Sun, Nov 15, 2020 at 12:16:56PM -0500, Tom Lane wrote:
> The obvious problem with this is that if !USE_OPENSSL, we will not have
> pulled in openssl's headers.
FWIW, I argued upthread against including this part because it is
useless: if not building with OpenSSL, we'll never have the base to b
Magnus Hagander writes:
> I think the defensive-in-code instead of defensive-in-docs is a really
> strong argument, so I have pushed it as such.
I notice warnings that I think are caused by this patch on some buildfarm
members, eg
drongo| 2020-11-15 06:59:05 |
C:\prog\bf\root\HEAD\pgsq
On Fri, Nov 06, 2020 at 01:31:44PM +0100, Magnus Hagander wrote:
> I think the defensive-in-code instead of defensive-in-docs is a really
> strong argument, so I have pushed it as such.
Fine by me. Thanks for the commit.
--
Michael
signature.asc
Description: PGP signature
On Fri, Nov 6, 2020 at 12:08 PM Daniel Gustafsson wrote:
>
> > On 6 Nov 2020, at 00:36, Michael Paquier wrote:
>
> > I still don't see the point of this extra complexity, as
> > USE_OPENSSL_RANDOM implies USE_OPENSSL,
>
> As long as we're sure that we'll remember to fix this when that assumption
> On 6 Nov 2020, at 00:36, Michael Paquier wrote:
> I still don't see the point of this extra complexity, as
> USE_OPENSSL_RANDOM implies USE_OPENSSL,
As long as we're sure that we'll remember to fix this when that assumption no
longer holds (intentional or unintentional) then it's fine to skip
On Thu, Nov 05, 2020 at 01:59:11PM +0100, Daniel Gustafsson wrote:
> Not yet, and potentially never will. Given the consequences of a PRNG which
> hasn't been properly initialized I think it's ok to be defensive in this
> codepath however.
+ /*
+* In case the backend is using the PRNG from
> On 5 Nov 2020, at 13:28, Michael Paquier wrote:
> It seems to me that this one would become incorrect if compiling with
> OpenSSL but select a random source that requires an initialization, as
> it would enforce only OpenSSL initialization all the time.
Right, how about something like the atta
On Thu, Nov 5, 2020 at 1:28 PM Michael Paquier wrote:
>
> On Thu, Nov 05, 2020 at 01:18:15PM +0100, Daniel Gustafsson wrote:
> > What about the (hypothetical) situation where USE_OPENSSL_RANDOM is used
> > without USE_OPENSSL? Wouldn't the below make sure we cover all bases?
>
> You can actually t
On Thu, Nov 05, 2020 at 01:18:15PM +0100, Daniel Gustafsson wrote:
> What about the (hypothetical) situation where USE_OPENSSL_RANDOM is used
> without USE_OPENSSL? Wouldn't the below make sure we cover all bases?
You can actually try that combination, because it is possible today to
compile witho
> On 5 Nov 2020, at 13:12, Michael Paquier wrote:
>
> On Thu, Nov 05, 2020 at 10:49:45AM +0100, Daniel Gustafsson wrote:
>> This must check for USE_OPENSSL as well as per my original patch, since we'd
>> otherwise fail to perform post-fork initialization in case one use OpenSSL
>> with
>> anothe
On Thu, Nov 05, 2020 at 10:49:45AM +0100, Daniel Gustafsson wrote:
> This must check for USE_OPENSSL as well as per my original patch, since we'd
> otherwise fail to perform post-fork initialization in case one use OpenSSL
> with
> anothe PRNG for pg_strong_random. That might be theoretical at th
> On 5 Nov 2020, at 04:44, Michael Paquier wrote:
>
> On Wed, Nov 04, 2020 at 10:05:48AM +0100, Magnus Hagander wrote:
>> Yes, we should absolutely do that. We already do this for
>> pg_strong_random() itself, and we should definitely repeat the pattern
>> in the init function.
>
> This poked at
On Wed, Nov 04, 2020 at 10:05:48AM +0100, Magnus Hagander wrote:
> Yes, we should absolutely do that. We already do this for
> pg_strong_random() itself, and we should definitely repeat the pattern
> in the init function.
This poked at my curiosity, so I looked at it. The result looks
indeed like
On Wed, Nov 4, 2020 at 2:01 AM Michael Paquier wrote:
>
> On Tue, Nov 03, 2020 at 01:46:38PM +0100, Magnus Hagander wrote:
> > On Tue, Nov 3, 2020 at 1:00 PM Daniel Gustafsson wrote:
> >> I kind of like the idea of continuing to abstract this functionality, not
> >> pulling in OpenSSL headers in
On Tue, Nov 03, 2020 at 01:46:38PM +0100, Magnus Hagander wrote:
> On Tue, Nov 3, 2020 at 1:00 PM Daniel Gustafsson wrote:
>> I kind of like the idea of continuing to abstract this functionality, not
>> pulling in OpenSSL headers in fork_process.c is a neat bonus. I'd say it's
>> worth implementi
On Tue, Nov 3, 2020 at 1:00 PM Daniel Gustafsson wrote:
>
> > On 3 Nov 2020, at 11:35, Michael Paquier wrote:
> >
> > On Tue, Nov 03, 2020 at 10:15:48AM +0100, Magnus Hagander wrote:
> >> On Wed, Aug 26, 2020 at 2:19 PM Daniel Gustafsson wrote:
> >>> That's certainly true. The intention though
> On 3 Nov 2020, at 11:35, Michael Paquier wrote:
>
> On Tue, Nov 03, 2020 at 10:15:48AM +0100, Magnus Hagander wrote:
>> On Wed, Aug 26, 2020 at 2:19 PM Daniel Gustafsson wrote:
>>> That's certainly true. The intention though is to make the code easier to
>>> follow (more explicit/discoverable
On Tue, Nov 03, 2020 at 10:15:48AM +0100, Magnus Hagander wrote:
> On Wed, Aug 26, 2020 at 2:19 PM Daniel Gustafsson wrote:
>> That's certainly true. The intention though is to make the code easier to
>> follow (more explicit/discoverable) for anyone trying to implement support
>> for
>
> Is it
On Wed, Aug 26, 2020 at 2:19 PM Daniel Gustafsson wrote:
>
> > On 26 Aug 2020, at 09:56, Michael Paquier wrote:
> > On Tue, Aug 25, 2020 at 03:52:14PM +0200, Daniel Gustafsson wrote:
>
> >> The attached moves all invocations under the correct guards. RAND_poll()
> >> in
> >> fork_process.c need
> On 26 Aug 2020, at 09:56, Michael Paquier wrote:
> On Tue, Aug 25, 2020 at 03:52:14PM +0200, Daniel Gustafsson wrote:
>> The attached moves all invocations under the correct guards. RAND_poll() in
>> fork_process.c needs to happen for both OpenSSL and OpenSSL random, thus the
>> check for both
On Tue, Aug 25, 2020 at 03:52:14PM +0200, Daniel Gustafsson wrote:
> The USE_OPENSSL_RANDOM macro is defined when OpenSSL is used as a randomness
> provider, but the implementation of strong randomness is guarded by
> USE_OPENSSL
> in most places. This is technically the same thing today, but it
The USE_OPENSSL_RANDOM macro is defined when OpenSSL is used as a randomness
provider, but the implementation of strong randomness is guarded by USE_OPENSSL
in most places. This is technically the same thing today, but it seems
hygienic to use the appropriate macro in case we ever want to allow OS
38 matches
Mail list logo
