Re: Proposed patch for key management
On Mon, 4 Jan 2021 at 17:56, Bruce Momjian wrote: > > On Sat, Jan 2, 2021 at 12:47:19PM +, Alastair Turner wrote: > > > > There is also a further validation task - probably beyond the scope of > > the key management patch and into the encryption patch[es] territory - > > checking that the keys supplied are the same keys in use for the data > > currently on disk. It feels to me like this should be done at startup, > > rather than as each file is accessed, which could make startup quite > > slow if there are a lot of keys with narrow scope. > > We do that already on startup by using GCM to validate the KEK when > encrypting each DEK. > Which validates two things - that the KEK is the same one which was used to encrypt the DEKs (instead of returning garbage plaintext when given a garbage key), and that the DEKs have not been tampered with at rest. What it does not check is that the DEKs are the keys used to encrypt the data, that one has not been copied or restored independent of the other.
Re: Proposed patch for key management
Hi Bruce On Mon, 4 Jan 2021 at 18:23, Bruce Momjian wrote: > > On Fri, Jan 1, 2021 at 06:26:36PM +, Alastair Turner wrote: > > After the long intro, my question - If using a standard format, > > managed by a library, for the internal keystore does not result in a > > smaller or simpler patch, is there enough other value for this to be > > worth considering? For security features, standards and building on > > well exercised code bases do really have value, but is it enough... > > I know Stephen Frost looked at PKCS12 but didn't see much value in it > since the features provided by PKCS12 didn't seem to add much for > Postgres, and added complexity... Yeah, OpenSSL 1.1's PKCS12 implementation doesn't offer much for managing arbitrary secrets. Building a reasonable abstraction over the primitives it does provide requires a non-trivial amount of code. > ... Using GCM for key wrapping, heap/index > files, and WAL seemed simple. Having Postgres take on the task of wrapping the keys and deriving the wrapping key requires less code than getting a library to do it, sure. It also expands the functional scope of Postgres, what Postgres is responsible for. This isn't what libraries should ideally do for software development, but it's not uncommon and it's very fertile ground for some of the discussion which has dogged this feature's development. There's a question of whether the new scope should be taken on at all. Fabien expressed a strong opinion that it should not be earlier in the thread. Even if you don't take an absolute view on the additional scope, lines of code do not all create equal risk. Ensuring the confidentiality of an encryption key is far higher stakes code than serialising and deserialising the key which is being secured and stored by a library or external provider. Which is why I like the idea of having OpenSSL (and, at some point, nss) do the KDF, wrapping and storage bit, and have been hacking on some code in that direction. If all that Postgres is doing is the serde, that's also something which can objectively be said to be right or wrong. Key derivation and wrapping are only ever "strong enough for the moment" or "in line with policy/best practice". Which brings us to flexibility. > There is all sorts of flexibility being proposed: > > * scope of keys > * encryption method > * encryption mode > * internal/external > > Some of this is related to wrapping the data keys, and some of it gets > to the encryption of cluster files. I just don't see how anything with > the level of flexibility being asked for could ever be reliable or > simple to administer,... Externalising the key wrap and its KDF (either downward to OpenSSL or to a separate process) makes the related points of flexibility something else's problem. OpenSSL and the like have put a lot of effort into making these things configurable and building the APIs (mainly PCKS11 and KMIP) for handing the functions off to dedicated hardware. Many or most organisations requiring that sort of integration will have those complexities budgeted in already. >... and I don't see the security value of that > flexibility. I feel at a certain point, we just need to choose the best > options and move forward. > > I thought this had all been debated, but people continue to show up and > ask for it to be debated again. At one level, these discussions are > valuable, but at a certain point you end up re-litigate this that you > get nothing else done. I know some people who have given up working on > this feature for this reason. Apologies for covering the PCKS12 ground again, I really did look for any on-list and wiki references to the topic. > I am not asking we stop discussing it, but I do ask we address the > negatives of suggestions, especially when the negatives have already > been clearly stated. The negatives are increasing the amount of code involved in delivering TDE and, implicitly, pushing back the delivery of TDE. I'm not claiming that they aren't valid. I was hoping that PCKS12 would deliver some benefits on the code side, but a bit of investigation showed that wasn't the case. To offset these, I believe that taking a slightly longer route now (and I am keen to do some of that slog) has significant benefits in getting Postgres (or at least the core server processes) out of the business of key derivation and wrapping. Not just the implementation detail, but also decisions about what is "good enough" - the KDF worries me particularly in this regard. With my presales techie hat on, I'm very aware that everyone's good enough is slightly different, and not just on one axis. Using library functions (let's say OpenSSL) will provide a range of flexibility to fit these varied boundaries, at no incremental cost beyond the library integration, and avoid any implementation decisions becoming a lightning rod for objections to Postgres. Regards Alastair
Re: Proposed patch for key management
Hi Fabien On Sat, 2 Jan 2021 at 09:50, Fabien COELHO wrote: > ... > ISTM that pg at the core level should (only) directly provide: > > (1) a per-file encryption scheme, with loadable (hook-replaceable > functions??) to manage pages, maybe: > >encrypt(page_id, *key, *clear_page, *encrypted_page); >decrypt(page_id, *key, *encrypted_page, *clear_page); > > What encrypt/decrypt does is beyond pg core stuff. Ok, a reasonable > implementation should be provided, obviously, possibly in core. There may > be some block-size issues if not full pages are encrypted, so maybe the > interface needs to be a little more subtle. > There are a lot of specifics of the encryption implementation which need to be addressed in future patches. This patch focuses on making keys available to the encryption processes at run-time, so ... > > (2) offer a key management scheme interface, to manage *per-file* keys, > possibly loadable (hook replaceable?). If all files have the same key, > which is stored in a directory and encoded with a KEK, this is just one > (debatable) implementation choice. For that, ISTM that what is needed at > this level is: > >get_key(file_id (relative name? oid? 8 or 16 bytes something?)); > Per-cluster keys for permanent data and WAL allow a useful level of protection, even if it could be improved upon. It's also going to be quicker/simpler to implement, so any API should allow for it. If there's an arbitrary number of DEK's, using a scope label for accessing them sounds right, so "WAL", "local_data", "local_data/tablespaceiod" or "local_data/dboid/tableoid". > ... > (3) ISTM that the key management interface should be external, or at least > it should be possible to make it external easily. I do not think that > there is a significant performance issue because keys are needed once, and > once loaded they are there. A simple way to do that is a separate process > with a basic protocol on stdin/stdout to implement "get_key", which is > basically already half implemented in the patch for retrieving the KEK. > If keys can have arbitrary scope, then the pg backend won't know what to ask for. So the API becomes even simpler with no specific request on stdin and all the relevant keys on stdout. I generally like this approach as well, and it will be the only option for some integrations. On the other hand, there is an advantage to having the key retrieval piece of key management in-process - the keys are not being passed around in plain. There is also a further validation task - probably beyond the scope of the key management patch and into the encryption patch[es] territory - checking that the keys supplied are the same keys in use for the data currently on disk. It feels to me like this should be done at startup, rather than as each file is accessed, which could make startup quite slow if there are a lot of keys with narrow scope. Regards Alastair
Re: Proposed patch for key management
Hi Stephen, Bruce, Fabien On Thu, 31 Dec 2020 at 17:05, Stephen Frost wrote: > > Greetings, > > * Bruce Momjian (br...@momjian.us) wrote: > > On Thu, Dec 31, 2020 at 11:11:11AM +0100, Fabien COELHO wrote: > > > > I am not sure what else I can add to this discussion. Having something > > > > that is completely external might be a nice option, but I don't think it > > > > is the common use-case, and will make the most common use cases more > > > > complex. > > > > > > I'm unsure about what is the "common use case". ISTM that having the > > > postgres process holding the master key looks like a "no" for me in any > > > use > > > case with actual security concern: as the database must be remotely > > > accessible, a reasonable security assumption is that a pg account could be > > > compromised, and the "master key" (if any, that is just one particular > > > cryptographic design) should not be accessible in that case. The first > > > barrier would be pg admin account. > > > > Let's unpack this logic, since I know others, like Alastair Turner (CC > > added), had similar concerns. Frankly, I feel this feature has limited > > security usefulness, so if we don't feel it has sufficient value, let's > > mark it as not wanted and move on. > > Considering the amount of requests for this, I don't feel it's at all > reasonable to suggest that it's 'not wanted'. > FWIW, I think that cluster file encryption has great value, even if we only consider its incremental value on top of disk or volume encryption. Particularly in big IT estates where monitoring tools, backup managers, etc have read access to a lot of mounted volumes. Limiting the impact of these systems (or their credentials) being compromised is definitely useful. > > > I think there are two basic key configurations: > > > > 1. pass data encryption keys in from an external source > > 2. store the data encryption keys wrapped by a key encryption key (KEK) > > passed in from an external source > > I agree those are two basic configurations (though they aren't the only > possible ones). > If we rephrase these two in terms of what the server process does to get a key, we could say 1. Get the keys from another process at startup (and maybe reload) time 2. Get the wrapped keys from a local file... 2.a. directly and unwrap them using the server's own logic and user prompt 2.b. via a library which manages wrapping and user prompting With the general feeling in the mailing list discussions favouring an approach in the #2 family, I have been looking at options around 2b. I was hoping to provide some flexibility for users without adding complexity to the pg code base and use some long standing, standardised, features rather than reinventing or recoding the wheel. It has not been a complete success, or failure. The nearest thing to a standard for wrapped secret store files is PKCS#12. I searched and cannot find it discussed on-list in this context before. It is the format used for Java local keystores in recent versions. The format is supported, up to a point, by OpenSSL, nss and gnuTLS. OpenSSL also has command line support for many operations on the files, including updating the wrapping password. Content in the PKCS12 files is stored in typed "bags". The challenge is that the OpenSSL command line tools currently support only the content types related to TLS operations - certificates, public or private keys, CRLs, ... and not the untyped secret bag which is appropriate for storing AES keys. The c APIs will work with this content but there are not as many helper functions as for other content types. For future information - the nss command line tools understand the secret bags, but don't offer nearly so many options for configuration. For building something now - the OpenSSL pkcs12 functions provide features like tags and friendly names for keys, and configurable key derivation from passwords, but the code to interact with the secretBags is accumulating quickly in my prototype. After the long intro, my question - If using a standard format, managed by a library, for the internal keystore does not result in a smaller or simpler patch, is there enough other value for this to be worth considering? For security features, standards and building on well exercised code bases do really have value, but is it enough... Regards Alastair
Re: Proposed patch for key managment
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 just a SHA hash, this should be at least a PBKDF2 (PKCS#5) operation or preferably scrypt (which is memory intensive as well as computationally intensive). While OpenSSL provides these key derivation options for it's encoding operations, it does not offer a command line option for key derivation using them. What's your view on including third party utilities like nettlepbkdf in the sample or providing this utility as a compiled program? In the same theme - in ossl_cipher_ctx_create and the functions using that context should rather be using the key-wrapping variants of the cipher. As well as lower throughput, with the resulting effects on brute force attempts, the key wrap variants provide more robust ciphertext output " KW, KWP, and TKW are each robust in the sense that each bit of output can be expected to depend in a nontrivial fashion on each bit of input, even when the length of the input data is greater than one block. This property is achieved at the cost of a considerably lower throughput rate, compared to other authenticated-encryption modes, but the tradeoff may be appropriate for some key management applications. For example, a robust method may be desired when the length of the keys to be protected is greater than the block size of the underlying block cipher, or when the value of the protected data is very high. " - NIST SP 800-38F and aim to manage the risk of short or predictable IVs ([1] final para of page 1) On Tue, 22 Dec 2020 at 15:40, Bruce Momjian wrote: > > Here is an updated patch. Are people happy with the Makefile, its > location in the source tree, and the install directory name? I used the > directory name 'auth_commands' because I thought 'auth' was too easily > misinterpreted. I put the scripts in /src/backend/utils/auth_commands. > What's implemented in these patches is an internal keystore, wrapped with a key derived from a passphrase. I'd think that the scripts directory should reflect what they interact with, so 'keystore_commands' or 'local_keystore_command' sounds more specific and therefore better than 'auth_commands'. Regards Alastair [1] https://web.cs.ucdavis.edu/~rogaway/papers/keywrap.pdf
Re: Proposed patch for key managment
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 keyring are used directly to encrypt the database contents. > > Right- understood, but that's not what the patch does today and there > isn't anyone who has proposed such a patch, nor explained why we > couldn't add that capability later. > I'm keen to propose a patch along those lines, even if just as a sample. Minimising the amount of code which would have to be unpicked in that effort would be great. Particularly avoiding any changes to data structures, since that may effectively block adding new capabilities. > > >...Your description above of changes to pass keys out > > of the command sound like they may have addressed this. > > The updates are intended to make it so that the KEK which wraps the > internal keyring will be obtained directly from the cluster key command, > pushing the transformation of a passphrase (should one be provided) into > a proper key to the script, but otherwise allowing the result of things > like 'openssl rand -hex 32' to be used directly as the KEK. > Sounds good. > > > Regarding the on-disk format, separate storage of the key HMACs and > > the wrapped keys sounds like a requirement for implementing a fully > > external keyring or doing key wrapping externally via an OpenSSL or > > nss tls pluggable engine. I'm still looking through the code. > > This seems a bit confusing as the question at hand is the on-disk format > of the internal keyring, not anything to do with an external keyring > (which we wouldn't have any storage of and so I don't understand how it > makes sense to even discuss the idea of storage for the external > keyring..?). > A requirement for validating the keys, no matter which keyring they came from, was mentioned along the way. Since there would be no point in validating keys from the internal ring twice, storing the validation data (HMACs in the current design) separately from the internal keyring's keys seems like it would avoid changing the data structures for the internal keyring when adding an external option. > > Again, we will need the ability to perform the encryption using a simple > passphrase provided by the user, while using multiple randomly generated > data encryption keys, and that's what the latest set of patches are > geared towards. I'm generally in support of adding additional > capabilities in this area in the future, but I don't think we need to > bog down the current effort by demanding that be implemented today. > I'm really not looking to bog down the current effort. 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. Regards Alastair
Re: Proposed patch for key managment
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 > do one thing, and you can't have it changing its behavior based on > parameters, since the parameters are for the script to process, not to > change behavior of the server. > > If we wanted to do this, we would need a new command, and one of the > parameters would be %k or something that would identify the key number > we want to retrieve from the KMS. Stephen pointed out that we could > still validate that key; the key would not be stored wrapped in the > file system, but we could store an HMAC in the file system, and use that > for validation. > I think that this is where we have ended up talking at cross-purposes. My idea (after some refining based on Stephen's feedback) is that there should be only this new command (the cluster key command) and that the program for unwrapping stored keys should be called this way. As could a program to contact an HSM, etc. This moves the generating and wrapping functionality out of the core Postgres processes, making it far easier to add alternatives. I see this approach was discussed on the email thread you linked to, but I can't see where or how a conclusion was reached about it... > > On other interesting approach would be to allow the server to call out > for a KMS when it needs to generate the initial data keys that are > wrapped by the passphrase; this is in contrast to calling the KMS > everytime it needs the data keys. > ... > ...The data keys are generated using this random value > code: > > > https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/port/pg_strong_random.c;hb=HEAD > > so someone would have to explain why generating this remotely in a KMS > is superior, not just based on company policy. > The pg_strong_random() function uses OpesnSSL's RAND_bytes() where available. With appropriate configuration of an OpenSSL engine supplying an alternative RAND, this could be handed off to a TRNG. This is an example of the other option for providing flexibility to support specific key generation, wrapping, ... requirements - handing the operations off to a library like OpenSSL or nss tls which, in turn, can use pluggable providers for these functions. FWIW, the proprietary DBMSs use a pluggable engine approach to meet requirements in this category. > > We should create code for the general use-case, which is currently > passphrase-wrapped system-generated data keys, and then get feedback on > what else we need. However, I should point out that the community is > somewhat immune to the "my company needs this" kind of argument without > logic to back it up, though sometimes I think this entire feature is in > that category. ... Yeah. Security in general, and crypto in particular, are often "because someone told me to" requirements. > > 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 random number generator is a valid argument, but we > need to figure out, once this is implemented, who really wants that, and > how to implement it cleanly. > Clean interfaces would be either "above" the database calling out to commands in user-land or "below" the database in the abstractions which OpenSSL, nss tls and friends provide. Since the conclusion seems already to have been reached that the keyring should be inside the core process and only the passphrase should pop out above, I'll review the patch in this vein and see if I can suggest any routes to carving out more manageable subsets of the patch. "...once this is implemented..." changes become a lot more difficult. Particularly when the changes would affect what are regarded as the database's on-disk files. Which I suspect is a contributing factor to the level of engagement surrounding this feature. Regards Alastair
Re: Proposed patch for key managment
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 item, and received a reply talking > about the design --- that is not helpful. I only have so much patience > for the "I want my secret decoder ring to glow in the dark" type of > feature additions to this already complex feature. Unless we stay on > Desirability, I will not be replying. If you can't answer the > Desirability question, well, talking about items farther right on the > list is not very helpful. > > Now, I will say that your question about how a KMS will use this got me > thinking about how to test this, and that got me to implement the AWS > Secret Manager script, so that we definitely helpful. My point is that > I don't think it is helpful to get into long discussions unless the > Desirability is clearly answered. This is not just related to this > thread --- this kind of jump-over-Desirability has happened a lot in > relation to this feature, so I thought it would be good to clearly state > it now. > 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 so open-ended so as to be actively unhelpful and very quickly starts to sound like "I want my secret decoder ring to glow pink, or blue, or green when anyone asks". Given the complexity of the feature and pixels spilled in discussing it, I understand that it gets frustrating. That said, I believe there is an important case to be made here. In summary, I believe that forcing an approach for key generation and wrapping onto users of Cluster File Encryption limits the Desirability of the feature. Cluster File Encryption for Postgres would be Desirable to many users I meet if, and only if, the generation and handling of keys fits with their corporate policies. Therefore, giving a user the option to pass an encryption key to Postgres for CFE is Desirable. I realise that the option for feeding the keys in directly is an additional feature, and that it has a user experience impact if a passphrase is used. It is, however, a feature which opens up near-infinite flexibility. To stretch the analogy, it is the clip for attaching coloured or opaque coverings to the glowy bit on the secret decoder ring. The generation of keys and the wrapping of keys are contentious issues and are frequently addressed/specified in corporate security policies, standards and technical papers (NIST 800-38F is often mentioned in product docs). There are, for instance, policies in the wild which require that keys for long term use are generated from the output of a True Random Number Generator - the practical implication being that the key used to encrypt data at rest should be created by an HSM. When and why to use symmetric or asymmetric cyphers for key wrapping is another item which different organisations have different policies on - the hardware and cloud service vendors all offer both options, even if they recommend one and make it easier to use. Thanks Alastair
Re: Proposed patch for key managment
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 expand the purpose of cluster_passphrase_command, > > let's call it cluster_key_challenge_command in the examples. > > 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 some reason that would be an issue..? Certainly > the CLI utilities I've seen tend to expect the name of the secret that > you're asking for to be passed on the command line. Agreed. I was working on the assumption that the calling process (initdb or pg_ctl) would have access to the challenge material and be passing it to the utility, so putting it in a command line would allow it to leak. If the utility is accessing the challenge material directly, then just an indicator of which key to work with would be a lot simpler, true. > > > 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 whatever we call it, expects is an > actual key and have the command that is run be a separate command that > takes the passphrase and runs the KDF (key derivation function) on it, > when a passphrase is what the user wishes to use. > > That generally makes more sense to me than having the key derivation > effort built into the backend which I have a hard time seeing any > particular reason for, as long we're already calling out to some > external utility of some kind to get the key. > ... > > With the thought of trying to keep it a reasonably simple interface, I > had a thought along these lines: > > - Separate command that runs the KDF, this is simple enough as it's > really just a hash, and it returns the key on stdout. > - initdb time option that says if we're going to have PG manage the > sub-keys, or not. > - cluster_key_command defined as "a command that is passed the ID of > the key, or keys, required for the cluster to start" > > initdb --pg-subkeys > - Calls cluster_key_command once with "$PG_SYSTEM_ID-main" or similar > and expects the main key to be provided on stdout. Everything else > is then more-or-less as is today: PG generates DEK sub-keys for data > and WAL and then encrypts them with the main key and stores them. > > As long as the enveloped keys file exists on the filesystem, when PG > starts, it'll call the cluster_key_command and will expect the 'main' > key to be provided and it'll then decrypt and verify the DEK sub-keys, > very similar to today. > > In this scenario, cluster_key_command might be set to call a command > which accepts a passphrase and runs a KDF on it, or it might be set to > call out to an external vaulting system or to a Yubikey, etc. > > initdb --no-pg-subkeys > - Calls cluster_key_command for each of the sub-keys that "pg-subkeys" > would normally generate itself, passing "$PG_SYSTEM_ID-keyid" for > each (eg: $PG_SYSTEM_ID-data, $PG_SYSTEM_ID-wal), and getting back > the keys on stdout to use. > > When PG starts, it sees that the enveloped keys file doesn't exist and > does the same as initdb did- calls cluster_key_command multiple times to > get the keys which are needed. We'd want to make sure to have a way > early on that checks that the data DEK provided actually decrypts the > cluster, and bail out otherwise, before actually writing any data with > that key. I'll note though that this approach would actually allow you > to change the WAL key, if you wanted to, though, which could certainly > be nice (naturally there would be various caveats about doing so and > that replicas would have to also be switched to the new key, etc, but > that all seems reasonably solvable). Having a stand-alone utility that > could do that for the --pg-subkeys case would be useful too (and just > generally decrypt it for viewing/backup/replacement/etc). > > Down the line this might even allow us to do an online re-keying, at > least once the challenges around enabling online data checksums are > sorted out, but I don't think that's something to worry about today. > Still, it seems like this potentially provides a pretty clean interface > for that to happen eventually. > Changing the WAL key independently of the local storage key is the big operational advantage I see of managing the two keys separately. It lay
Re: Proposed patch for key managment
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 key? Not sure how > well that's work if want to automate everything though. > > If you have specific ideas, it'd really be helpful to give examples of > what you're thinking. I can think of three specific ideas off the top of my head: the passphrase key wrapper, the secret store and the cloud/HW KMS. Since the examples expand the purpose of cluster_passphrase_command, let's call it cluster_key_challenge_command in the examples. Starting with the passphrase key wrapper, since it's what's in place now. - cluster_key_challenge_command = 'password_key_wrapper %q' - Supplied on stdin to the process is the wrapped DEK (either a combined key for db files and WAL or one for each, on separate calls) - %q is "Please provide WAL key wrapper password" or just "...provide key wrapper password" - Returned on stdout is the unwrapped DEK For a secret store - cluster_key_challenge_command = 'vault_key_fetch' - Supplied on stdin to the process is the secret's identifier (pg_dek_xxUUIDxx) - Returned on stdout is the DEK, which may never have been wrapped, depending on implementation - Access control to the secret store is managed through the challenge command's own config, certs, HBA, ... For an HSM or cloud KMS - cluster_key_challenge_command = 'gcp_kms_key_fetch' - Supplied on stdin to the process is the the wrapped DEK (individual or combined) - Returned on stdout is the DEK (individual or combined) - Access control to the kms is managed through the challenge command's own config, certs, HBA, ... The secret store and HSM/KMS options may be promptless, and therefore amenable to automation, depending on the setup of those clients. > ... > > > > ...That > > > avoids the complication of having to have an API that somehow provides > > > more than one key, while also using the primary DEK key as-is from the > > > key management service and the KEK never being seen on the system where > > > PG is running. > > > > Other than calling out (and therefore potentially prompting) twice, > > what do you see as the complications of having more than one key? > > Mainly just a concern about the API and about what happens if, say, we > decide that we want to have another sub-key, for example. If we're > handling them then there's really no issue- we just add another key in, > but if that's not happening then it's going to mean changes for > administrators. If there's a good justification for it, then perhaps > that's alright, but hand waving at what the issue is doesn't really > help. > Sorry, I wasn't trying to hand wave it away. For automated interactions, like big iron HSMs or cloud KSMs, the difference between 2 operations and 10 operations to start a DB server won't matter. For an admin/operator having to type 10 passwords or get 10 clean thumbprint scans, it would be horrible. My underlying question was, is that toil the only problem to be solved, or is there another reason to get into key combination, key splitting and the related issues which are less documented and less well understood than key wrapping. > ... > > > > I'd describe what the current patch does as using YubiKey to encrypt > > and then decrypt an intermediate secret, which is then used to > > generate/derive a KEK, which is then used to unwrap the stored, > > wrapped DEK. > > This seems like a crux of at least one concern- that the current patch > is deriving the actual KEK from the passphrase and not just taking the > provided value (at least, that's what it looks like from a *very* quick > look into it), and that's the part that I was suggesting that we might > add an option for- to indicate if the cluster passphrase command is > actually returning a passphrase which should be used to derive a key, or > if it's returning a key directly to be used. That does seem to be a > material difference to me and one which we should care about. > Yes. Caring about that is the reason I've been making a nuisance of myself. > > > There's an entirely independent discussion to be had about figuring out > > > a way to actually off-load *entirely* the encryption/decryption of data > > > to the linux crypto system or hardware devices, but unless someone can > > > step up and write code for those today, I'd suggest that we table that > > > effort until after we get this initial capability of having TDE with PG > > > doing all of the encryption/decryption. > > > > I'm hopeful that the work on abstracting OpenSSL, nsstls, etc is going > > to help in this direct. > > Yes, I agree with that general idea but it's a 'next step' kind of > thing, not something we need to try and bake in today. > Agreed. Thanks Alastair
Re: Proposed patch for key managment
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. Specifically, an API > > which allows any potential adopter of Postgres and Cluster File > > Encryption to adopt them without having to accept any particular > > approach to key management, key derivation, wrapping, validation, etc. > > A passphrase key-wrapper with validation will probably be very useful > > to a lot of people, but making it mandatory and requiring twists and > > turns to integrate with already-established security infrastructure > > sounds like a barrier to adoption. > > Yeah, I mentioned this in one of the other threads where we were > discussing KEKs and DEKs and such. Forcing one solution for working > with the KEK/DEK isn't really ideal. > > That said, maybe there's an option to have the user indicate to PG if > they're passing in a passphrase or a DEK, ... Some options very much like the current cluster_passphrase_command, but that takes an input sounds to me like it would meet that requirement. The challenge sent to the command could be just a text prompt, or it could be the wrapped DEK. The selection of the command would indicate what the user was expected to pass. The return would be a DEK. > ...but we otherwise more-or-less > keep things as they are where the DEK that the user provides actually > goes to encrypting the sub-keys that we use for tables vs. WAL..? ... The idea of subkeys sounds great, if it can merge the two potential interactions into one and not complicate rotating the two parts separately. But having Postgres encrypting them, leaves us open to many of the same issues. That still feels like managing the keys, so a corporate who have strong opinions on how that should be done will still ask all sorts of pointy questions, complicating adoption. > ...That > avoids the complication of having to have an API that somehow provides > more than one key, while also using the primary DEK key as-is from the > key management service and the KEK never being seen on the system where > PG is running. > Other than calling out (and therefore potentially prompting) twice, what do you see as the complications of having more than one key? > > > ...It would be > > > similar to the cluster_passphrase_command, but it would be simple in > > > some ways, but complex in others since we need at least two DEKs, and > > > key rotation might be very risky since there isn't validation in the > > > server. > > ... > > I'm not quite following this- I agree that we don't want PG, or anything > really, to see the private key that's on the Yubikey, as that wouldn't > be good- instead, we should be able to ask for the Yubikey to decrypt > the DEK for us and then use that, which I had thought was happening but > it's not entirely clear from the above discussion (and, admittedly, I've > not tried using the patch with my yubikey yet, but I do plan to...). > > Are we sure we're talking about the same thing here..? That's really > the question I'm asking myself. > I'd describe what the current patch does as using YubiKey to encrypt and then decrypt an intermediate secret, which is then used to generate/derive a KEK, which is then used to unwrap the stored, wrapped DEK. > There's an entirely independent discussion to be had about figuring out > a way to actually off-load *entirely* the encryption/decryption of data > to the linux crypto system or hardware devices, but unless someone can > step up and write code for those today, I'd suggest that we table that > effort until after we get this initial capability of having TDE with PG > doing all of the encryption/decryption. I'm hopeful that the work on abstracting OpenSSL, nsstls, etc is going to help in this direct. Regards Alastair
Re: Proposed patch for key managment
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 without having to accept any particular approach to key management, key derivation, wrapping, validation, etc. A passphrase key-wrapper with validation will probably be very useful to a lot of people, but making it mandatory and requiring twists and turns to integrate with already-established security infrastructure sounds like a barrier to adoption. > ...It would be > similar to the cluster_passphrase_command, but it would be simple in > some ways, but complex in others since we need at least two DEKs, and > key rotation might be very risky since there isn't validation in the > server. > I guess that the risk you're talking about here is encryption with a bogus key and bricking the data? In a world where the wrapped keys are opaque to the application, a key would be validated through a roundtrip: request the unwrapping of the key, encrypt a known string, request the unwrapping again, decrypt the known string, compare. If any of those steps fail, don't use the wrapped key provided. Validating that the stored keys have not been fiddled/damaged is the KMS/HSM's responsibility. > >... I don't think this can be accomplished by a contrib module, but > would actually be easy to implement, but perhaps hard to document > because the user API might be tricky. > If integration through a pipeline isn't good enough (it would be a pain for the passphrase, with multiple prompts), what else do you see an API having to provide? > > I think my big question is about your sentence, "A key feature of these > key management approaches is that the application handling the > encrypted data doesn't get the KEK, the HSM/KSM passes the DEK back > after unwrapping it." It is less secure for the HSM to return a KEK > rather than a DEK? I can't see why it would be. The application never > sees the KEK used to wrap the DEK it has stored in the file system, > though that DEK is actually used as a passphrase by Postgres. This is > the same with the Yubikey --- Postgres never sees the private key used > to unlock what it locked with the Yubikey public key. > The protocols and practices are designed for a lot of DEKs and small number of KEK's. So the compromise of a KEK would, potentially, lead to compromise of thousands of DEKs. In this particular case with 2 DEKs wrapped by one KEK, it doesn't sound like much of a difference, I agree. From an acceptance and adoption point of view, I'm just inclined to use the security ecosystem in an established and understood way. Regards Alastair
Re: Proposed patch for key managment
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 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 Thales, Utimaco, (other options are available), or their cloudy lookalikes. I understand the model of wrapping the Data Encryption Key and storing the wrapped key with the encrypted data. The thing I can't find support for in your patch is passing a wrapped DEK to an external system for decryption. A key feature of these key management approaches is that the application handling the encrypted data doesn't get the KEK, the HSM/KSM passes the DEK back after unwrapping it. Google have a neat illustration of their approach, which is very similar to others, at https://cloud.google.com/kms/docs/envelope-encryption#how_to_decrypt_data_using_envelope_encryption So instead of calling out to a passphrase command which returns input for a PBKDF, Postgres (in the form of initdb or postmaster) should be passing the wrapped DEK and getting back the DEK in plain. The value passed from Postgres to the external process doesn't even have to be a wrapped DEK, it could be a key in the key->value sense, for use in a lookup against Vault, CredHub or the Kubernetes secret store. Making the stored keys opaque to the Postgres processes and pushing the task of turning them into cryptographic material to an external hepler process probably wouldn't shrink the patch overall, but it would move a lot of code from core processes into the helper. Maybe that helper should live in contrib, as discussed below, where it would hopefully be joined by a bridge for KMIP and others. On Tue, 15 Dec 2020 at 19:09, 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: > > > > There are a few shell script I should include to show how to create > > > > commands. Where should they be stored? /contrib module? > > > > > > Why are these needed. Is it a matter of documentation? > > > > I have posted some of the scripts. They involved complex shell > > scripting that I doubt the average user can do. The scripts are for > > prompting for a passphrase from the user's terminal, or using the > > Yubikey, with our without typing a pin. I can put them in the docs and > > then users can copy them into a file. Is that the preferred method? > > If we are going to include these in core anywhere, I would think a new > directory under contrib would make the most sense. I'd hope that we > could find a way to automate the testing of them though, so that we have > some idea when they break because otherwise I'd be concerned that > they'll break somewhere down the line and we won't notice for quite a > while. > > Regards Alastair
Re: Parallel copy
On Wed, 26 Feb 2020 at 10:54, Amit Kapila wrote: > > On Tue, Feb 25, 2020 at 9:30 PM Tomas Vondra > wrote: > > ... > > > > Perhaps. I guess it'll depend on the CSV file (number of fields, ...), > > so I still think we need to do some measurements first. > > > > Agreed. > > > I'm willing to > > do that, but (a) I doubt I'll have time for that until after 2020-03, > > and (b) it'd be good to agree on some set of typical CSV files. > > > > Right, I don't know what is the best way to define that. I can think > of the below tests. > > 1. A table with 10 columns (with datatypes as integers, date, text). > It has one index (unique/primary). Load with 1 million rows (basically > the data should be probably 5-10 GB). > 2. A table with 10 columns (with datatypes as integers, date, text). > It has three indexes, one index can be (unique/primary). Load with 1 > million rows (basically the data should be probably 5-10 GB). > 3. A table with 10 columns (with datatypes as integers, date, text). > It has three indexes, one index can be (unique/primary). It has before > and after trigeers. Load with 1 million rows (basically the data > should be probably 5-10 GB). > 4. A table with 10 columns (with datatypes as integers, date, text). > It has five or six indexes, one index can be (unique/primary). Load > with 1 million rows (basically the data should be probably 5-10 GB). > > Among all these tests, we can check how much time did we spend in > reading, parsing the csv files vs. rest of execution? That's a good set of tests of what happens after the parse. Two simpler test runs may provide useful baselines - no constraints/indexes with all columns varchar and no constraints/indexes with columns correctly typed. For testing the impact of various parts of the parse process, my idea would be: - A base dataset with 10 columns including int, date and text. One text field quoted and containing both delimiters and line terminators - A derivative to measure just line parsing - strip the quotes around the text field and quote the whole row as one text field - A derivative to measure the impact of quoted fields - clean up the text field so it doesn't require quoting - A derivative to measure the impact of row length - run ten rows together to make 100 column rows, but only a tenth as many rows If that sounds reasonable, I'll try to knock up a generator. -- Alastair
Re: Parallel copy
On Sat, 15 Feb 2020 at 04:55, Amit Kapila wrote: > > On Fri, Feb 14, 2020 at 7:16 PM Alastair Turner wrote: > > ... > > > > Parsing rows from the raw input (the work done by CopyReadLine()) in a > > single process would accommodate line returns in quoted fields. I don't > > think there's a way of getting parallel workers to manage the > > in-quote/out-of-quote state required. > > > > AFAIU, the whole of this in-quote/out-of-quote state is manged inside > CopyReadLineText which will be done by each of the parallel workers, > something on the lines of what Thomas did in his patch [1]. > Basically, we need to invent a mechanism to allocate chunks to > individual workers and then the whole processing will be done as we > are doing now except for special handling for partial tuples which I > have explained in my previous email. Am, I missing something here? > The problem case that I see is the chunk boundary falling in the middle of a quoted field where - The quote opens in chunk 1 - The quote closes in chunk 2 - There is an EoL character between the start of chunk 2 and the closing quote When the worker processing chunk 2 starts, it believes itself to be in out-of-quote state, so only data between the start of the chunk and the EoL is regarded as belonging to the partial line. From that point on the parsing of the rest of the chunk goes off track. Some of the resulting errors can be avoided by, for instance, requiring a quote to be preceded by a delimiter or EoL. That answer fails when fields end with EoL characters, which happens often enough in the wild. Recovering from an incorrect in-quote/out-of-quote state assumption at the start of parsing a chunk just seems like a hole with no bottom. So it looks to me like it's best done in a single process which can keep track of that state reliably. -- Aastair
Re: Parallel copy
On Fri, 14 Feb 2020 at 11:57, Amit Kapila wrote: > On Fri, Feb 14, 2020 at 3:36 PM Thomas Munro > wrote: > > > > On Fri, Feb 14, 2020 at 9:12 PM Amit Kapila > wrote: ... > > > Another approach that came up during an offlist discussion with Robert > > > is that we have one dedicated worker for reading the chunks from file > > > and it copies the complete tuples of one chunk in the shared memory > > > and once that is done, a handover that chunks to another worker which > > > can process tuples in that area. We can imagine that the reader > > > worker is responsible to form some sort of work queue that can be > > > processed by the other workers. In this idea, we won't be able to get > > > the benefit of initial tokenization (forming tuple boundaries) via > > > parallel workers and might need some additional memory processing as > > > after reader worker has handed the initial shared memory segment, we > > > need to somehow identify tuple boundaries and then process them. > Parsing rows from the raw input (the work done by CopyReadLine()) in a single process would accommodate line returns in quoted fields. I don't think there's a way of getting parallel workers to manage the in-quote/out-of-quote state required. A single worker could also process a stream without having to reread/rewind so it would be able to process input from STDIN or PROGRAM sources, making the improvements applicable to load operations done by third party tools and scripted \copy in psql. > > ... > > > > Another thing we need to figure out is the how many workers to use for > > > the copy command. I think we can use it based on the file size which > > > needs some experiments or may be based on user input. > > > > It seems like we don't even really have a general model for that sort > > of thing in the rest of the system yet, and I guess some kind of > > fairly dumb explicit system would make sense in the early days... > > > > makes sense. > The ratio between chunking or line parsing processes and the parallel worker pool would vary with the width of the table, complexity of the data or file (dates, encoding conversions), complexity of constraints and acceptable impact of the load. Being able to control it through user input would be great. -- Alastair
