Re: Proposed patch for key managment

2020-12-31 Thread Stephen Frost
Greetings, * Fabien COELHO (coe...@cri.ensmp.fr) wrote: > >>The implementations should not have to be in any particular language: Shell, > >>Perl, Python, C should be possible. > > > >I disagree that it makes any sense to pass actual encryption out to a > >shell script. > > Yes, sure. I'm

Re: Proposed patch for key managment

2020-12-31 Thread Stephen Frost
Greetings, * Bruce Momjian (br...@momjian.us) wrote: > On Wed, Dec 30, 2020 at 06:49:34PM -0500, Stephen Frost wrote: > > The API to fetch the KEK doesn't care at all about where it's stored or > > how it's derived or anything like that. There's a relatively small > > change which could be made

Re: Proposed patch for key managment

2020-12-31 Thread Fabien COELHO
Hello, The API to fetch the KEK doesn't care at all about where it's stored or how it's derived or anything like that. There's a relatively small change which could be made to have PG request all of the keys that it'll need on startup, if we want to go there as has been suggested elsewhere,

Re: Proposed patch for key managment

2020-12-31 Thread Fabien COELHO
Hello Stephen, The implementations should not have to be in any particular language: Shell, Perl, Python, C should be possible. I disagree that it makes any sense to pass actual encryption out to a shell script. Yes, sure. I'm talking about key management. For actual crypto functions,

Re: Proposed patch for key managment

2020-12-30 Thread Bruce Momjian
On Wed, Dec 30, 2020 at 06:49:34PM -0500, Stephen Frost wrote: > The API to fetch the KEK doesn't care at all about where it's stored or > how it's derived or anything like that. There's a relatively small > change which could be made to have PG request all of the keys that it'll > need on

Re: Proposed patch for key managment

2020-12-30 Thread Stephen Frost
Greetings, * Fabien COELHO (coe...@cri.ensmp.fr) wrote: > I think that an API should be carefully thought about, without assumption > about the underlying cryptography (algorithm, key lengths, modes, how keys > are derived and stored, and so on), and its usefulness be demonstrated by > actually

Re: Proposed patch for key managment

2020-12-28 Thread Fabien COELHO
I want to repeat here what I said in another thread: I think ultimately we will need three commands to control the keys. First, there is the cluster_key_command, which we have now. Second, I think we will need an optional command which returns random bytes --- this would allow users to get

Re: Proposed patch for key managment

2020-12-27 Thread Bruce Momjian
On Sat, Dec 19, 2020 at 11:45:08AM +, Alastair Turner wrote: > On Fri, 18 Dec 2020 at 21:36, Stephen Frost wrote: > > These ideas don't seem too bad but I'd think we'd pass which key we want > > on the command-line using a %i or something like that, rather than using > > stdin, unless there's

Re: Proposed patch for key managment

2020-12-22 Thread Bruce Momjian
On Tue, Dec 22, 2020 at 10:40:17AM -0500, Bruce Momjian wrote: > On Mon, Dec 21, 2020 at 10:07:48PM -0500, Bruce Momjian wrote: > > Attached is the script patch. It is also at: > > > > > > https://github.com/postgres/postgres/compare/master...bmomjian:cfe-sh.diff > > > > I think it still

Re: Proposed patch for key managment

2020-12-22 Thread Bruce Momjian
On Tue, Dec 22, 2020 at 04:13:06PM -0500, Bruce Momjian wrote: > On Tue, Dec 22, 2020 at 08:15:27PM +, Alastair Turner wrote: > > Hi Bruce > > > > In ckey_passphrase.sh.sample > > > > + > > +echo "$PASS" | sha256sum | cut -d' ' -f1 > > + > > > > Under the threat model discussed, a copy of

Re: Proposed patch for key managment

2020-12-22 Thread Bruce Momjian
On Tue, Dec 22, 2020 at 08:15:27PM +, Alastair Turner wrote: > Hi Bruce > > In ckey_passphrase.sh.sample > > + > +echo "$PASS" | sha256sum | cut -d' ' -f1 > + > > Under the threat model discussed, a copy of the keyfile could be > attacked offline. So getting from passphrase to DEKs should

Re: Proposed patch for key managment

2020-12-22 Thread Alastair Turner
Hi Bruce In ckey_passphrase.sh.sample + +echo "$PASS" | sha256sum | cut -d' ' -f1 + Under the threat model discussed, a copy of the keyfile could be attacked offline. So getting from passphrase to DEKs should be as resource intensive as possible to slow down brute-force attempts. Instead of

Re: Proposed patch for key managment

2020-12-22 Thread Bruce Momjian
On Mon, Dec 21, 2020 at 10:07:48PM -0500, Bruce Momjian wrote: > Attached is the script patch. It is also at: > > > https://github.com/postgres/postgres/compare/master...bmomjian:cfe-sh.diff > > I think it still needs docs but those will have to be done after the > code doc patch is

Re: Proposed patch for key managment

2020-12-21 Thread Bruce Momjian
On Mon, Dec 21, 2020 at 04:35:14PM -0500, Bruce Momjian wrote: > On Sun, Dec 20, 2020 at 05:59:09PM -0500, Stephen Frost wrote: > OK, here it the updated patch. The major change, as Stephen pointed > out, is that the cluser_key_command (was cluster_passphrase_command) now > returns a hex-string

Re: Proposed patch for key managment

2020-12-21 Thread Bruce Momjian
On Mon, Dec 21, 2020 at 03:00:32PM -0700, David G. Johnston wrote: > I agree with Stephen; so maybe that part of the Wiki needs to be updated > instead of having it sit there for use as a hammer when someone disagrees with > a proffered patch. > > Or maybe consider that "a patch" doesn't only

Re: Proposed patch for key managment

2020-12-21 Thread David G. Johnston
On Mon, Dec 21, 2020 at 2:44 PM Bruce Momjian wrote: > On Sun, Dec 20, 2020 at 09:38:55PM -0500, Stephen Frost wrote: > > > I'll have a go at adding another keyring and/or abstracting the key > > > wrap and see how well a true peer to the passphrase approach fits in. > > > > Having patches to

Re: Proposed patch for key managment

2020-12-21 Thread Bruce Momjian
On Sun, Dec 20, 2020 at 05:59:09PM -0500, Stephen Frost wrote: > I do generally like the idea of having the single command able to be > used to either provide the KEK (where PG manages the keyring > internally), or called multiple times to retrieve the DEKs (in which > case PG wouldn't be managing

Re: Proposed patch for key managment

2020-12-21 Thread Bruce Momjian
On Sun, Dec 20, 2020 at 09:38:55PM -0500, Stephen Frost wrote: > > I'll have a go at adding another keyring and/or abstracting the key > > wrap and see how well a true peer to the passphrase approach fits in. > > Having patches to review and consider definitely helps, imv. I disagree. Our order

Re: Proposed patch for key managment

2020-12-21 Thread Bruce Momjian
On Sun, Dec 20, 2020 at 05:59:09PM -0500, Stephen Frost wrote: > However, after chatting with Bruce about it for a bit this weekend, I'm > not sure that we need to tackle the second case today. I don't think > there's any doubt that there will be users who will want PG to manage > the keyring and

Re: Proposed patch for key managment

2020-12-20 Thread Stephen Frost
Greetings, * Alastair Turner (min...@decodable.me) wrote: > On Mon, 21 Dec 2020 at 00:33, Stephen Frost wrote: > > * Alastair Turner (min...@decodable.me) wrote: > > > What I'd like specifically is to have the option of an external > > > keyring as a first class key store, where the keys stored

Re: Proposed patch for key managment

2020-12-20 Thread Alastair Turner
Thanks Stephen, On Mon, 21 Dec 2020 at 00:33, Stephen Frost wrote: > > Greetings, > > * Alastair Turner (min...@decodable.me) wrote: ... > > > > What I'd like specifically is to have the option of an external > > keyring as a first class key store, where the keys stored in the > > external

Re: Proposed patch for key managment

2020-12-20 Thread Stephen Frost
Greetings, * Alastair Turner (min...@decodable.me) wrote: > On Sun, 20 Dec 2020 at 22:59, Stephen Frost wrote: > > Yes, it's true that after things are implemented it can be more > > difficult to change them- but if you're concerned about the specific > > on-disk format of the keyring then

Re: Proposed patch for key managment

2020-12-20 Thread Stephen Frost
Greetings Alastair, * Alastair Turner (min...@decodable.me) wrote: > On Sat, 19 Dec 2020 at 16:58, Bruce Momjian wrote: > > To enable the direct injection of keys into the server, we would need a > > new command for this, since trying to make the passphrase command do > > this will lead to

Re: Proposed patch for key managment

2020-12-20 Thread Alastair Turner
Thanks, Bruce On Sat, 19 Dec 2020 at 16:58, Bruce Momjian wrote: > ... > > To enable the direct injection of keys into the server, we would need a > new command for this, since trying to make the passphrase command do > this will lead to unnecessary complexity. The passphrase command should >

Re: Proposed patch for key managment

2020-12-19 Thread Bruce Momjian
On Sat, Dec 19, 2020 at 11:58:37AM -0500, Bruce Momjian wrote: > My final point is that we can find ways to do what you are suggesting as > an addition to what we are adding now. What we need is clear > justification of why these additional features are needed. Requiring > the use of a true

Re: Proposed patch for key managment

2020-12-19 Thread Bruce Momjian
On Sat, Dec 19, 2020 at 11:45:15AM +, Alastair Turner wrote: > Sorry, I have waved Desirability through under the headings of ease of > adoption or not raising barriers to adoption, without detailing what > barriers I see or how to avoid them. I also realise that "don't scare > the users" is

Re: Proposed patch for key managment

2020-12-19 Thread Alastair Turner
Hi Bruce On Sat, 19 Dec 2020 at 02:38, Bruce Momjian wrote: > > I am not going be as kind. Our workflow is: > > Desirability -> Design -> Implement -> Test -> Review -> Commit > https://wiki.postgresql.org/wiki/Todo#Development_Process > > I have already asked about the first

Re: Proposed patch for key managment

2020-12-19 Thread Alastair Turner
Hi Stephen On Fri, 18 Dec 2020 at 21:36, Stephen Frost wrote: > > Greetings Alastair, > > * Alastair Turner (min...@decodable.me) wrote: > > On Wed, 16 Dec 2020 at 22:43, Stephen Frost wrote: ... > > passphrase key wrapper, the secret store and the cloud/HW KMS. > > > > Since the examples

Re: Proposed patch for key managment

2020-12-18 Thread Bruce Momjian
On Fri, Dec 18, 2020 at 04:36:24PM -0500, Stephen Frost wrote: > > Starting with the passphrase key wrapper, since it's what's in place now. > > > > - cluster_key_challenge_command = 'password_key_wrapper %q' > > I do tend to like this idea of having what > cluster_key_challenge_command, or

Re: Proposed patch for key managment

2020-12-18 Thread Stephen Frost
Greetings Alastair, * Alastair Turner (min...@decodable.me) wrote: > On Wed, 16 Dec 2020 at 22:43, Stephen Frost wrote: > > If I'm following, you're suggesting something like: > > > > cluster_passphrase_command = 'aws get %q' > > > > and then '%q' gets replaced with "Please provide the WAL DEK:

Re: Proposed patch for key managment

2020-12-18 Thread Bruce Momjian
On Fri, Dec 18, 2020 at 11:13:44AM -0500, Stephen Frost wrote: > Greetings, > > * Bruce Momjian (br...@momjian.us) wrote: > > On Fri, Dec 18, 2020 at 08:18:53AM -0500, Stephen Frost wrote: > > > - C API in backend - we should try to at least set up the structure to > > > allow multiple

Re: Proposed patch for key managment

2020-12-18 Thread Stephen Frost
Greetings, * Bruce Momjian (br...@momjian.us) wrote: > On Fri, Dec 18, 2020 at 08:18:53AM -0500, Stephen Frost wrote: > > - C API in backend - we should try to at least set up the structure to > > allow multiple encryption implementations, either via different > > libraries or if someone

Re: Proposed patch for key managment

2020-12-18 Thread Bruce Momjian
On Fri, Dec 18, 2020 at 08:18:53AM -0500, Stephen Frost wrote: > What I would be thinking about with this are really three pieces- > > - C API for libpq (not relevant for this, but we have had issues in the > past with exposing OpenSSL-specific things there) Right. > - C API in backend - we

Re: Proposed patch for key managment

2020-12-18 Thread Stephen Frost
Greetings, * Bruce Momjian (br...@momjian.us) wrote: > On Fri, Dec 18, 2020 at 10:01:22AM +0900, Michael Paquier wrote: > > On Thu, Dec 17, 2020 at 12:10:22PM -0500, Bruce Momjian wrote: > > > On Thu, Dec 17, 2020 at 11:39:55AM -0500, Stephen Frost wrote: > > >> I don't think there's any need for

Re: Proposed patch for key managment

2020-12-18 Thread Stephen Frost
Greetings, * Michael Paquier (mich...@paquier.xyz) wrote: > On Thu, Dec 17, 2020 at 12:10:22PM -0500, Bruce Momjian wrote: > > Agreed. I think there is serious risk we would do AES in a different > > way than OpenSSL, especially if I did it. ;-) We can add a native AES > > one day if we want,

Re: Proposed patch for key managment

2020-12-17 Thread Bruce Momjian
On Fri, Dec 18, 2020 at 10:01:22AM +0900, Michael Paquier wrote: > On Thu, Dec 17, 2020 at 12:10:22PM -0500, Bruce Momjian wrote: > > On Thu, Dec 17, 2020 at 11:39:55AM -0500, Stephen Frost wrote: > >> I don't think there's any need for us to implement a fallback > >> implementation of AES. I'm

Re: Proposed patch for key managment

2020-12-17 Thread Bruce Momjian
On Fri, Dec 18, 2020 at 11:19:02AM +0800, Neil Chen wrote: > > > On Fri, Dec 18, 2020 at 3:02 AM Bruce Momjian wrote: > > > Here is a run of all four authentication methods, and updated scripts. > I have renamed Yubiki to PIV since the script should work with anY > PIV-enabled

Re: Proposed patch for key managment

2020-12-17 Thread Neil Chen
On Fri, Dec 18, 2020 at 3:02 AM Bruce Momjian wrote: > > Here is a run of all four authentication methods, and updated scripts. > I have renamed Yubiki to PIV since the script should work with anY > PIV-enabled deviced, like a CAC. > > Thanks for attaching these patches. The unfortunate thing is

Re: Proposed patch for key managment

2020-12-17 Thread Michael Paquier
On Thu, Dec 17, 2020 at 12:10:22PM -0500, Bruce Momjian wrote: > On Thu, Dec 17, 2020 at 11:39:55AM -0500, Stephen Frost wrote: >> I don't think there's any need for us to implement a fallback >> implementation of AES. I'm not entirely sure we need it for hashes >> but since we've already got

Re: Proposed patch for key managment

2020-12-17 Thread Bruce Momjian
On Mon, Dec 14, 2020 at 11:16:18PM -0500, Bruce Momjian wrote: > On Tue, Dec 15, 2020 at 10:36:56AM +0800, Neil Chen wrote: > > Since our implementation is not in contrib, I don't think we should put the > > script there. Maybe we can refer to postgresql.conf.sample?   > > Uh, the script are

Re: Proposed patch for key managment

2020-12-17 Thread Bruce Momjian
On Thu, Dec 17, 2020 at 11:39:55AM -0500, Stephen Frost wrote: > Greetings, > > * Michael Paquier (mich...@paquier.xyz) wrote: > > On Wed, Dec 16, 2020 at 05:04:12PM -0500, Bruce Momjian wrote: > > >> fallback implementation. Finally, pgcrypto is not touched, but we > > > > > > I have a

Re: Proposed patch for key managment

2020-12-17 Thread Stephen Frost
Greetings, * Michael Paquier (mich...@paquier.xyz) wrote: > On Wed, Dec 16, 2020 at 05:04:12PM -0500, Bruce Momjian wrote: > >> fallback implementation. Finally, pgcrypto is not touched, but we > > > > I have a fallback implemention --- it fails? ;-) Did you want me to > > include an AES

Re: Proposed patch for key managment

2020-12-16 Thread Michael Paquier
On Thu, Dec 17, 2020 at 01:15:37AM +0100, Daniel Gustafsson wrote: > In vtls library contexts are abstracted to the core code, with implementations > supplying a struct with a set of function pointers implementing functionality > (this difference is due to libcurl supporting multiple TLS libraries

Re: Proposed patch for key managment

2020-12-16 Thread Daniel Gustafsson
> On 16 Dec 2020, at 17:26, Stephen Frost wrote: > > Greetings, > > * Michael Paquier (mich...@paquier.xyz) wrote: >> On Tue, Dec 15, 2020 at 02:09:09PM -0500, Stephen Frost wrote: >>> Yeah, looking at what's been done there seems like the right approach to >>> me as well, ideally we'd have one

Re: Proposed patch for key managment

2020-12-16 Thread Alastair Turner
On Wed, 16 Dec 2020 at 22:43, Stephen Frost wrote: > > Greetings, ... > > If I'm following, you're suggesting something like: > > cluster_passphrase_command = 'aws get %q' > > and then '%q' gets replaced with "Please provide the WAL DEK: ", or > something like that? Prompting the user for each

Re: Proposed patch for key managment

2020-12-16 Thread Michael Paquier
On Wed, Dec 16, 2020 at 05:04:12PM -0500, Bruce Momjian wrote: > On Wed, Dec 16, 2020 at 10:23:23AM +0900, Michael Paquier wrote: >> Hmm. I don't think that this is right. First, this still mixes >> ciphers and HMAC. > > I looked at the code. The HMAC function body is just one function call >

Re: Proposed patch for key managment

2020-12-16 Thread Stephen Frost
Greetings, * Alastair Turner (min...@decodable.me) wrote: > On Wed, 16 Dec 2020 at 21:32, Stephen Frost wrote: > > * Alastair Turner (min...@decodable.me) wrote: > > > On Wed, 16 Dec 2020 at 00:12, Bruce Momjian wrote: > > > > The second approach is to make a new API for what you want > > >

Re: Proposed patch for key managment

2020-12-16 Thread Alastair Turner
On Wed, 16 Dec 2020 at 21:32, Stephen Frost wrote: > > Greetings, > > * Alastair Turner (min...@decodable.me) wrote: > > On Wed, 16 Dec 2020 at 00:12, Bruce Momjian wrote: > > > The second approach is to make a new API for what you want > > > > I am trying to motivate for an alternate API.

Re: Proposed patch for key managment

2020-12-16 Thread Bruce Momjian
On Wed, Dec 16, 2020 at 01:42:57PM -0500, Bruce Momjian wrote: > On Wed, Dec 16, 2020 at 06:07:26PM +, Alastair Turner wrote: > > Hi Bruce > > > > On Wed, 16 Dec 2020 at 00:12, Bruce Momjian wrote: > > > > > ... > > > > > > The second approach is to make a new API for what you want > >

Re: Proposed patch for key managment

2020-12-16 Thread Bruce Momjian
On Wed, Dec 16, 2020 at 04:32:00PM -0500, Stephen Frost wrote: > Greetings, > > * Alastair Turner (min...@decodable.me) wrote: > > On Wed, 16 Dec 2020 at 00:12, Bruce Momjian wrote: > > > The second approach is to make a new API for what you want > > > > I am trying to motivate for an

Re: Proposed patch for key managment

2020-12-16 Thread Bruce Momjian
On Wed, Dec 16, 2020 at 10:23:23AM +0900, Michael Paquier wrote: > On Tue, Dec 15, 2020 at 04:02:12PM -0500, Bruce Momjian wrote: > > I thought this was going to be a huge job, but once I looked at it, it > > was clear exactly what you were saying. Comparing cryptohash.c and > >

Re: Proposed patch for key managment

2020-12-16 Thread Stephen Frost
Greetings, * Alastair Turner (min...@decodable.me) wrote: > On Wed, 16 Dec 2020 at 00:12, Bruce Momjian wrote: > > The second approach is to make a new API for what you want > > I am trying to motivate for an alternate API. Specifically, an API > which allows any potential adopter of

Re: Proposed patch for key managment

2020-12-16 Thread Bruce Momjian
On Wed, Dec 16, 2020 at 06:07:26PM +, Alastair Turner wrote: > Hi Bruce > > On Wed, 16 Dec 2020 at 00:12, Bruce Momjian wrote: > > > ... > > > > The second approach is to make a new API for what you want > > I am trying to motivate for an alternate API. Specifically, an API > which

Re: Proposed patch for key managment

2020-12-16 Thread Alastair Turner
Hi Bruce On Wed, 16 Dec 2020 at 00:12, Bruce Momjian wrote: > ... > > The second approach is to make a new API for what you want I am trying to motivate for an alternate API. Specifically, an API which allows any potential adopter of Postgres and Cluster File Encryption to adopt them

Re: Proposed patch for key managment

2020-12-16 Thread Stephen Frost
Greetings, * Michael Paquier (mich...@paquier.xyz) wrote: > On Tue, Dec 15, 2020 at 02:09:09PM -0500, Stephen Frost wrote: > > Yeah, looking at what's been done there seems like the right approach to > > me as well, ideally we'd have one set of APIs that'll support all these > > use cases (not

Re: Proposed patch for key managment

2020-12-15 Thread Michael Paquier
On Tue, Dec 15, 2020 at 04:02:12PM -0500, Bruce Momjian wrote: > I thought this was going to be a huge job, but once I looked at it, it > was clear exactly what you were saying. Comparing cryptohash.c and > cryptohash_openssl.c I saw exactly what you wanted, and I think I have > completed it in

Re: Proposed patch for key managment

2020-12-15 Thread Michael Paquier
Hi Stephen, On Tue, Dec 15, 2020 at 02:09:09PM -0500, Stephen Frost wrote: > Yeah, looking at what's been done there seems like the right approach to > me as well, ideally we'd have one set of APIs that'll support all these > use cases (not unlike what curl does..). Ooh... This is interesting.

Re: Proposed patch for key managment

2020-12-15 Thread Bruce Momjian
On Tue, Dec 15, 2020 at 11:13:12PM +, Alastair Turner wrote: > Since it's exciting stuff, I've been trying to lash together a PoC integration > with the crypto infrastructure we see at these customers. Unfortunately, in > short, the API doesn't seem to suit integration with HSM big iron, like

Re: Proposed patch for key managment

2020-12-15 Thread Alastair Turner
Hi Bruce et al Firstly, thanks for shaping the patch, getting it down to a manageable scope of cluster file encryption. I think this is a great feature and it matters to a lot of the customers I talk to at VMware about adopting Postgres. Since it's exciting stuff, I've been trying to lash

Re: Proposed patch for key managment

2020-12-15 Thread Bruce Momjian
On Tue, Dec 15, 2020 at 02:09:09PM -0500, Stephen Frost wrote: > Greetings, > > * Bruce Momjian (br...@momjian.us) wrote: > > On Tue, Dec 15, 2020 at 10:05:49AM +0900, Michael Paquier wrote: > > > On Mon, Dec 14, 2020 at 06:06:15PM -0500, Bruce Momjian wrote: > > > > I am getting close to

Re: Proposed patch for key managment

2020-12-15 Thread Bruce Momjian
On Tue, Dec 15, 2020 at 02:20:33PM +0900, Michael Paquier wrote: > On Mon, Dec 14, 2020 at 10:19:02PM -0500, Bruce Momjian wrote: > > I am going to need someone to help me make these changes. I don't feel > > I know enough about the crypto API to do it, and it will take me 1+ week > > to learn

Re: Proposed patch for key managment

2020-12-15 Thread Stephen Frost
Greetings, * Bruce Momjian (br...@momjian.us) wrote: > On Tue, Dec 15, 2020 at 10:05:49AM +0900, Michael Paquier wrote: > > On Mon, Dec 14, 2020 at 06:06:15PM -0500, Bruce Momjian wrote: > > > I am getting close to applying these patches, probably this week. The > > > patches are cumulative: > >

Re: Proposed patch for key managment

2020-12-15 Thread Bruce Momjian
On Tue, Dec 15, 2020 at 10:36:56AM +0800, Neil Chen wrote: > 2. I tried to add support for AES_CTR mode, and the code for encrypting buffer > blocks. In the process I found that in pg_cipher_ctx_create, the key length is > declared as "byte". However, in the CryptoKey structure, the length is

Re: Proposed patch for key managment

2020-12-15 Thread Bruce Momjian
On Mon, Dec 14, 2020 at 11:16:18PM -0500, Bruce Momjian wrote: > > 1. Previously, we added a variable bootstrap_keys_wrap that is used for > > encryption during initdb. However, since we save the "wrapped" key, we need > > to > > use a global KEK that can be accessed in boot mode to unwrap it

Re: Proposed patch for key managment

2020-12-15 Thread Bruce Momjian
On Tue, Dec 15, 2020 at 02:20:33PM +0900, Michael Paquier wrote: > > Uh, I got this code from pg_resetwal.c, which does something similar to > > pg_altercpass. > > Yeah, that's actually the part where it is a bad idea to just copy > this pattern. pg_resetwal should not do that in the long term

Re: Proposed patch for key managment

2020-12-14 Thread Michael Paquier
On Mon, Dec 14, 2020 at 10:19:02PM -0500, Bruce Momjian wrote: > On Tue, Dec 15, 2020 at 10:05:49AM +0900, Michael Paquier wrote: >> - The HMAC split is terrible, as mentioned upthread. I think that you >> would need to extract this stuff as a separate patch, and not add more >> mess to the

Re: Proposed patch for key managment

2020-12-14 Thread Bruce Momjian
On Tue, Dec 15, 2020 at 10:36:56AM +0800, Neil Chen wrote: > Hi, Bruce   > > I read your question and here are some of my thoughts. > > >         Why is KmgrShmemData a struct, when it only has a single member?  > Are >         all shared memory areas structs? > >   > Yes, all

Re: Proposed patch for key managment

2020-12-14 Thread Bruce Momjian
On Tue, Dec 15, 2020 at 10:05:49AM +0900, Michael Paquier wrote: > On Mon, Dec 14, 2020 at 06:06:15PM -0500, Bruce Momjian wrote: > > I am getting close to applying these patches, probably this week. The > > patches are cumulative: > > > >

Re: Proposed patch for key managment

2020-12-14 Thread Neil Chen
Hi, Bruce I read your question and here are some of my thoughts. Why is KmgrShmemData a struct, when it only has a single member? > Are > all shared memory areas structs? > Yes, all areas created by ShmemInitStruct() are structs. I think the significance of using struct is that

Re: Proposed patch for key managment

2020-12-14 Thread Michael Paquier
On Mon, Dec 14, 2020 at 06:06:15PM -0500, Bruce Momjian wrote: > I am getting close to applying these patches, probably this week. The > patches are cumulative: > > https://github.com/postgres/postgres/compare/master...bmomjian:key.diff > >

Re: Proposed patch for key managment

2020-12-14 Thread Bruce Momjian
On Wed, Dec 2, 2020 at 04:38:14PM -0500, Bruce Momjian wrote: > Attached is a patch for key management, which will eventually be part of > cluster file encryption (CFE), called TDE (Transparent Data Encryption) > by Oracle. It is an update of Masahiko Sawada's patch from July 31: > > >

Re: Proposed patch for key managment

2020-12-11 Thread Bruce Momjian
On Fri, Dec 11, 2020 at 01:01:14PM -0500, Bruce Momjian wrote: > On Wed, Dec 9, 2020 at 08:40:50PM -0500, Bruce Momjian wrote: > > My next task is to write a script for Yubikey authentication. > > I know Craig Ringer wanted Yubikey support, which allows two-factor > authentication, so I have

Re: Proposed patch for key managment

2020-12-11 Thread Bruce Momjian
On Wed, Dec 9, 2020 at 08:40:50PM -0500, Bruce Momjian wrote: > My next task is to write a script for Yubikey authentication. I know Craig Ringer wanted Yubikey support, which allows two-factor authentication, so I have added it to the most recent patch by adding a cluster_passphrase_command

Re: Proposed patch for key managment

2020-12-10 Thread Bruce Momjian
On Thu, Dec 10, 2020 at 07:26:48PM +0800, Neil Chen wrote: > > > Hi, everyone > > I have read the patch and did some simple tests. I'm not entirely sure > about some code segments; e.g.: > > In the BootStrapKmgr() we generate a data encryption key by: > key =

Re: Proposed patch for key managment

2020-12-10 Thread Bruce Momjian
On Wed, Dec 9, 2020 at 05:18:37PM -0500, Stephen Frost wrote: > Greetings, > > * Daniel Gustafsson (dan...@yesql.se) wrote: > > > On 2 Dec 2020, at 22:38, Bruce Momjian wrote: > > > Attached is a patch for key management, which will eventually be part of > > > cluster file encryption (CFE),

Re: Proposed patch for key managment

2020-12-10 Thread Bruce Momjian
On Wed, Dec 9, 2020 at 10:34:59PM +0100, Daniel Gustafsson wrote: > > On 2 Dec 2020, at 22:38, Bruce Momjian wrote: > > > > Attached is a patch for key management, which will eventually be part of > > cluster file encryption (CFE), called TDE (Transparent Data Encryption) > > by Oracle. > >

Re: Proposed patch for key managment

2020-12-10 Thread Neil Chen
Hi, everyone > > I have read the patch and did some simple tests. I'm not entirely sure > about some code segments; e.g.: > > In the BootStrapKmgr() we generate a data encryption key by: > key = generate_crypto_key(file_encryption_keylen); > > However, I found that the file_encryption_keylen is

Re: Proposed patch for key managment

2020-12-09 Thread Bruce Momjian
On Fri, Dec 4, 2020 at 10:32:45PM -0500, Bruce Momjian wrote: > I can break out the -R/file descriptor passing part as a separate patch, > and have the ssl_passphrase_command use that, but that's the only part I > know can be useful on its own. > > Since the patch is large, I found a way to push

Re: Proposed patch for key managment

2020-12-09 Thread Stephen Frost
Greetings, * Daniel Gustafsson (dan...@yesql.se) wrote: > > On 2 Dec 2020, at 22:38, Bruce Momjian wrote: > > Attached is a patch for key management, which will eventually be part of > > cluster file encryption (CFE), called TDE (Transparent Data Encryption) > > by Oracle. > > The attackvector

Re: Proposed patch for key managment

2020-12-09 Thread Daniel Gustafsson
> On 2 Dec 2020, at 22:38, Bruce Momjian wrote: > > Attached is a patch for key management, which will eventually be part of > cluster file encryption (CFE), called TDE (Transparent Data Encryption) > by Oracle. The attackvector protected against seems to be operating systems users (*without*

Re: Proposed patch for key managment

2020-12-06 Thread Bruce Momjian
On Mon, Dec 7, 2020 at 11:03:30AM +0900, Michael Paquier wrote: > On Sat, Dec 05, 2020 at 10:42:05AM -0500, Bruce Momjian wrote: > > On Sat, Dec 5, 2020 at 09:54:33PM +0900, Michael Paquier wrote: > >> On Fri, Dec 04, 2020 at 10:52:29PM -0500, Bruce Momjian wrote: > >>> if (state->evpctx ==

Re: Proposed patch for key managment

2020-12-06 Thread Bruce Momjian
On Mon, Dec 7, 2020 at 09:30:03AM +0900, Masahiko Sawada wrote: > Thank you for updating the patch! > > I think we need explicit_bzero() also in freeing the keywrap context. pg_cryptohash_free() already has this: explicit_bzero(state, sizeof(pg_cryptohash_state)); explicit_bzero(ctx,

Re: Proposed patch for key managment

2020-12-06 Thread Michael Paquier
On Sat, Dec 05, 2020 at 10:42:05AM -0500, Bruce Momjian wrote: > On Sat, Dec 5, 2020 at 09:54:33PM +0900, Michael Paquier wrote: >> On Fri, Dec 04, 2020 at 10:52:29PM -0500, Bruce Momjian wrote: >>> if (state->evpctx == NULL) >>> { >>> - explicit_bzero(state,

Re: Proposed patch for key managment

2020-12-06 Thread Masahiko Sawada
On Sun, 6 Dec 2020 at 00:42, Bruce Momjian wrote: > > On Sat, Dec 5, 2020 at 09:54:33PM +0900, Michael Paquier wrote: > > On Fri, Dec 04, 2020 at 10:52:29PM -0500, Bruce Momjian wrote: > > > OK, I worked with Sawada-san and added the attached patch. The updated > > > full patch is at the same

Re: Proposed patch for key managment

2020-12-05 Thread Bruce Momjian
On Sat, Dec 5, 2020 at 09:54:33PM +0900, Michael Paquier wrote: > On Fri, Dec 04, 2020 at 10:52:29PM -0500, Bruce Momjian wrote: > > OK, I worked with Sawada-san and added the attached patch. The updated > > full patch is at the same URL: :-) > > > >

Re: Proposed patch for key managment

2020-12-05 Thread Michael Paquier
On Fri, Dec 04, 2020 at 10:52:29PM -0500, Bruce Momjian wrote: > OK, I worked with Sawada-san and added the attached patch. The updated > full patch is at the same URL: :-) > > https://github.com/postgres/postgres/compare/master...bmomjian:key.diff Oh, I see that you use SHA256 during

Re: Proposed patch for key managment

2020-12-05 Thread Masahiko Sawada
On Sat, 5 Dec 2020 at 11:08, Bruce Momjian wrote: > > On Wed, Dec 2, 2020 at 04:38:14PM -0500, Bruce Momjian wrote: > > If most people approve of this general approach, and the design > > decisions made, I would like to apply this in the next few weeks, but > > this brings complications. The

Re: Proposed patch for key managment

2020-12-04 Thread Bruce Momjian
On Sat, Dec 5, 2020 at 12:15:13PM +0900, Masahiko Sawada wrote: > diff --git a/src/common/cryptohash_openssl.c b/src/common/cryptohash_openssl.c > index e5233daab6..a45c86fa67 100644 > --- a/src/common/cryptohash_openssl.c > +++ b/src/common/cryptohash_openssl.c > @@ -81,6 +81,8 @@

Re: Proposed patch for key managment

2020-12-04 Thread Bruce Momjian
On Sat, Dec 5, 2020 at 11:39:18AM +0900, Michael Paquier wrote: > On Fri, Dec 04, 2020 at 09:08:03PM -0500, Bruce Momjian wrote: > > Here is an updated patch to handle the new hash API introduced by > > commit 87ae9691d2. > > + if (!ossl_initialized) > + { > +#ifdef

Re: Proposed patch for key managment

2020-12-04 Thread Michael Paquier
On Fri, Dec 04, 2020 at 09:08:03PM -0500, Bruce Momjian wrote: > Here is an updated patch to handle the new hash API introduced by > commit 87ae9691d2. + if (!ossl_initialized) + { +#ifdef HAVE_OPENSSL_INIT_SSL + OPENSSL_init_ssl(OPENSSL_INIT_LOAD_CONFIG, NULL); +#else +

Re: Proposed patch for key managment

2020-12-04 Thread Bruce Momjian
On Wed, Dec 2, 2020 at 04:38:14PM -0500, Bruce Momjian wrote: > If most people approve of this general approach, and the design > decisions made, I would like to apply this in the next few weeks, but > this brings complications. The syntax added by this commit might not > provide a useful

Proposed patch for key managment

2020-12-02 Thread Bruce Momjian
Attached is a patch for key management, which will eventually be part of cluster file encryption (CFE), called TDE (Transparent Data Encryption) by Oracle. It is an update of Masahiko Sawada's patch from July 31: