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
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
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
> 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
>>
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
> 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
>>
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
> 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
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
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
>
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
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 |
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
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
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
> 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
>>
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
> 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
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
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
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
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
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
> 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
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
38 matches
Mail list logo
