search

rstudio / config

config package for R
https://rstudio.github.io/config/
256 stars 27 forks source link

Easy way of decrypting fields #19

Open daroczig opened 6 years ago

daroczig commented 6 years ago

Would you be open/interested in a PR introducing new YAML types for encrypted config values?

See eg https://github.com/daroczig/dbr/blob/master/R/config.R#L32

image

(Image source: http://bit.ly/user2018-dbr)

^^ Although that only supports Amazon KMS for now, but extending it with new types would be similarly easy. This way I could drop the custom functions from my dbr pkg and use config.

jjallaire commented 6 years ago

@jcheng5, @wch, and @trestletech, how does this dovetail with other work/thinking we have done on this front? (I know we have it on our radar to support encrypted config using another mechanism, seems like the !kms extension described here should be orthogonal to that but wanted to check to be sure.

trestletech commented 6 years ago

I'm not aware of anything on our radar around encrypted config values on the R side. Looping @slopp in for broader perspective, though.

jcheng5 commented 6 years ago

I haven't thought much about this recently but this looks like a nice extension. @hadley @gaborcsardi may have more thoughts.

gaborcsardi commented 6 years ago

There are two things here IMO. Encrypted fields are one, and if there are good use cases for them, then they make sense. I can't really judge this.

Second, using the "type system" of YAML, so the parsed config will be typed. E.g. something like

shinydemo:
   host: !type kms |
     AAAbadcafe...

and then the parser could just add a "kms" S3 class to the host object. This seems easy enough to implement, and maybe not too bad to use. One would need to traverse the parsed list after parsing, though.

OTOH, AFAICT with !expr it is already possible to write this, and more:

shinydemo:
   host: !expr kms_decrpyt("
     AAAbadcafe...")
daroczig commented 6 years ago

OTOH, AFAICT with !expr it is already possible to write this, and more

I'd rather avoid putting R function calls in the YAML config if possible so that we can keep that language-independent. Eg one could use the same YAML config both from R and Python -- implementing in both languages how to deal with the kms or other custom types (although the above example with the RMySQL::MySQL() is probably confusing from this point of view, sorry for that).

gaborcsardi commented 6 years ago

I'd rather avoid putting R function calls in the YAML config if possible so that we can keep that language-independent.

Yeah, that is a good point for sg like !type. Maybe namespace-d with package names:

shinydemo:
   host: !type AWR.KMS::kms |
     AAAbadcafe...

But then you'd need to walk the list to decrypt, or config would need to be able to pass handlers to the yaml package.

daroczig commented 6 years ago

Maybe namespace-d with package names

Not sure: trying to keep the YAML language-independent, and for a ciphertext to be decrypted via Amazon KMS, kms type makes more sense to me than AWR.KMS::kms -- especially when I'm using the very same YAML config from Python :) And then config could know how to deal with kms, eg using AWR.KMS::kms_decrypt.

But then you'd need to walk the list to decrypt, or config would need to be able to pass handlers to the yaml package.

I think that's fairly easy and already done in 1-1 lines in my own config implementation at https://github.com/daroczig/dbr/blob/master/R/config.R#L12

jjallaire commented 6 years ago

I agree that we should keep the YAML language-independent. I also agree that since we won't likely end up with more than a handful to a couple-dozen of these handlers (we currently have 0) that we don't need a formal namespacing mechanism per se. That said, since kms isn't a protocol or other generic term, I think you do need a preface of some kind (as one could imagine additional "key management services" from other providers_. You might want to use aws-kms