ploigos / ploigos-step-runner

Ploigos Step Runner (PSR) implemented as a Python library.
GNU General Public License v3.0
20 stars 66 forks source link

RFE: Add function to StepImplementer abstract class that returns list of config values that should be considred "private"/"secret"/"protected" #116

Open adamgoossens opened 3 years ago

adamgoossens commented 3 years ago

Issue

if someone passes a value typically considered "secret" to the PSR via non encyrpted means, such as a a flag to the PSR cli or iva a config file that wasn't encrypted, PSR has no way of knowing that value should be protected and will print it to stdout without any obfuscation.

while the defualt stance of the PSR is that any "secret" values should always be encrypted at rest, there is a case to be made for having another level of protect htat for certain config values they are always treated as "protected" and their values always obfuscated whether user passed them in as encrypted or not.

Possible solution

Update StepImplemetner abstract class and its implementations so that as they read in config values that any value considered "always secret" gets registered with the DecryptorUtils as something to obfuscate.

Something to watch out for is currently the StepImplmeenters do not know which values are encrypted by users and which arnt' because that is handled independetly and invisibly to the StepImplementers so have to deal with case of value being pased in encrypted for a "protected config value" and then the StepImplemtner registering that vlaue again with the DecryptorUtils, maybe just having some de-dup logic in the DecryptorUtils or something.

Origional Post

Configuration entries are printed to stdout during a pipeline run. This results in passwords and tokens being visible in cleartext, like so:

Global Configuration Defaults
    {'application-name': 'ref-quarkus-mvn-jenkins-std',
     'container-registries': {'quay-devsecops.apps.ocp4.adamgoossens.com': {'password': 'xxxxxxxxxxxxx',
                                                                            'username': 'tssc'},
                              'registry.redhat.io': {'password': 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx',
                                                     'username': 'xxxxxxxxxxxxxxx|xxxxxxxxxxxxxxxxxx'}},

Either we should avoid printing any content that is marked as sensitive, or ensure everything is encrypted at rest in the configuration tree so no cleartext passwords/tokens will appear in stdout.

itewk commented 3 years ago

@adamgoossens was the value origionally encrypted at rest in the config file that was passed in, or were the values in plane text in the config file?

adamgoossens commented 3 years ago

Morning @itewk - plain text, by the looks of it. This was the generic config that's created by the Operator when deploying a Platform CR.

itewk commented 3 years ago

@adamgoossens one way to address this would be to make the DecryptionUtils#__add_obuscation_targerts funciton public and then individual step implementers could add values to the list. If you wanted to get a bit fancy about it could add a function for the children of StepImplementer to implement that give a list of value names to add the avlues of to the obfuscation list and then update the StepImplemetner#get_value function to look at that list of obfuscate me value names and if one is asked for, add the retrivied value to the obfuscation target list, again assuming that function made public.