Re: OpenSSL 3.0.0 compatibility
On 2020-05-29 14:45, Daniel Gustafsson wrote: I think we should set OPENSSL_API_COMPAT=10001, and move that along with whatever our oldest supported release is going forward. That declares our intention, it will silence the deprecation warnings, and IIUC, if the deprecated stuff actually gets removed, you get a clean compiler error that your API level is too low. I think I know what you mean but just to clarify: I master, back-branches or all of the above? I'm not sure. I don't have a good sense of what OpenSSL versions we claim to support in branches older than PG13. We made a conscious decision for 1.0.1 in PG13, but I seem to recall that that discussion also revealed that the version assumptions before that were quite inconsistent. Code in PG12 and before makes references to OpenSSL as old as 0.9.6. But OpenSSL 3.0.0 will reject a compat level older than 0.9.8. My proposal would be to introduce OPENSSL_API_COMPAT=10001 into master after the 13/14 branching, along with any other changes to make it compile cleanly against OpenSSL 3.0.0. Once that has survived some scrutiny from the buildfarm and also from folks building against LibreSSL etc., it should probably be backpatched into PG13. In the immediate future, I wouldn't bother about the older branches (<=PG12) at all. As long as they still compile, users can just disable deprecation warnings, and we may add some patches to that effect at some point, but it's not like OpenSSL 3.0.0 will be adopted into production builds any time soon. Considering how little effort it is to not use the deprecated API's I'm not entirely convinced, but I don't have too strong opinions there. Well, in the case like X509_STORE_load_locations(), the solution is in either case to write a wrapper. It doesn't matter if we write the wrapper or OpenSSL writes the wrapper. Only OpenSSL has already written the wrapper and has created a well-defined way to declare that you want to use the wrapper, so I'd just take that. In any case, using OPENSSL_API_COMPAT is also good just for our own documentation, so we can keep track of what version we claim to support in different branches. If they do, then that key will stop working with any OpenSSL 3 enabled software unless the legacy provider has been loaded. My understanding is that users can load these in openssl.conf, so maybe it's mostly a documentation patch for us? Yes, it looks like that should work, so no additional work required from us. There is also the question of what to do with the test suites in the back branches. If we don't want to change the testdata in the backbranches, we could add a SKIP section for the password key tests iff OpenSSL is 3.0.0+? I suggest to update the test data in PG13+, since we require OpenSSL 1.0.1 there. For the older branches, I would look into changing the test driver setup so that it loads a custom openssl.cnf that loads the legacy providers. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: OpenSSL 3.0.0 compatibility
On 5/28/20 6:16 PM, Daniel Gustafsson wrote: > > OpenSSL also deprecates DES keys in 3.0.0, which cause our password callback > tests to fail with the cryptic error "fetch failed", as the test suite keys > are > encrypted with DES. 0002 fixes this by changing to AES256 (randomly chosen > among the ciphers supported in 1.0.1+ and likely to be around), and could be > applied already today as there is nothing 3.0.0 specific about it. > +1 for applying this forthwith. The key in my recent commit 896fcdb230 is encrypted with AES256. cheers andrew -- Andrew Dunstanhttps://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Read access for pg_monitor to pg_replication_origin_status view
On Fri, May 29, 2020 at 05:39:31PM -0300, Martín Marqués wrote: > I believe we could skip the superuser() check for cases like > pg_replication_origin_session_progress() and > pg_replication_origin_progress(). > > Once option could be to add a third bool argument check_superuser to > replorigin_check_prerequisites() and have it set to false for the > functions which a none superuser could execute. Wouldn't it be just better to remove this hardcoded superuser check and replace it with equivalent ACLs by default? The trick is to make sure that any function calling replorigin_check_prerequisites() has its execution correctly revoked from public. See for example e79350fe. -- Michael signature.asc Description: PGP signature
Re: Inlining of couple of functions in pl_exec.c improves performance
On Sat, May 23, 2020 at 10:33:43PM +0530, Amit Khandekar wrote: > By inlining of the two functions, found noticeable reduction in > execution time as shown (figures are in milliseconds, averaged over > multiple runs; taken from 'explain analyze' execution times) : > ARM VM : >HEAD : 100 ; Patched : 88 => 13.6% improvement > x86 VM : >HEAD : 71 ; Patched : 66 => 7.63% improvement. > > Then I included many assignment statements as shown in attachment > assignmany.sql. This showed further benefit : > ARM VM : >HEAD : 1820 ; Patched : 1549 => 17.5% improvement > x86 VM : >HEAD : 1020 ; Patched : 869 => 17.4% improvement > > Inlining just exec_stmt() showed the improvement mainly on the arm64 > VM (7.4%). For x86, it was 2.7% > But inlining exec_stmt() and exec_cast_value() together showed > benefits on both machines, as can be seen above. This stuff is interesting. Do you have some perf profiles to share? I am wondering what's the effect of the inlining with your test cases. -- Michael signature.asc Description: PGP signature
Re: OpenSSL 3.0.0 compatibility
On Sat, May 30, 2020 at 11:29:11AM +0200, Peter Eisentraut wrote: > I'm not sure. I don't have a good sense of what OpenSSL versions we claim > to support in branches older than PG13. We made a conscious decision for > 1.0.1 in PG13, but I seem to recall that that discussion also revealed that > the version assumptions before that were quite inconsistent. Code in PG12 > and before makes references to OpenSSL as old as 0.9.6. But OpenSSL 3.0.0 > will reject a compat level older than 0.9.8. 593d4e4 claims that we only support OpenSSL >= 0.9.8, meaning that down to PG 10 we have this requirement, and that PG 9.6 and 9.5 should be able to work with OpenSSL 0.9.7 and 0.9.6, but little effort has been put in testing these. > My proposal would be to introduce OPENSSL_API_COMPAT=10001 into master after > the 13/14 branching, along with any other changes to make it compile cleanly > against OpenSSL 3.0.0. Once that has survived some scrutiny from the > buildfarm and also from folks building against LibreSSL etc., it should > probably be backpatched into PG13. In the immediate future, I wouldn't > bother about the older branches (<=PG12) at all. As long as they still > compile, users can just disable deprecation warnings, and we may add some > patches to that effect at some point, but it's not like OpenSSL 3.0.0 will > be adopted into production builds any time soon. Please note that I actually may have to bother about 12 and OpenSSL 3.0.0 as 1.0.2 deprecation is visibly accelerating a move to newer versions at least in my close neighborhood. We are not there yet of course, so doing this work with 14 and 13 sounds fine to me for now. -- Michael signature.asc Description: PGP signature
Re: Incorrect comment in be-secure-openssl.c
On Fri, May 29, 2020 at 02:38:53PM +0900, Michael Paquier wrote: > Indeed, looks good to me. I'll go fix, ust let's wait and see first > if others have any comments. Actually, I was reading again the new sentence, and did not like its first part. Here is a rework that looks much better to me: * Load hardcoded DH parameters. * - * To prevent problems if the DH parameter files don't even exist, we - * can load hardcoded DH parameters supplied with the backend. + * If DH parameters cannot be loaded from a specified file, we can load + * the hardcoded DH parameters supplied with the backend to prevent + * problems. Daniel, is that fine for you? -- Michael signature.asc Description: PGP signature
