Riverside-Software / pct

Build automation for OpenEdge ABL
Apache License 2.0
96 stars 62 forks source link

Add support for passphrase in DBConnection #501

Closed gquerret closed 1 year ago

gquerret commented 1 year ago

It is currently not possible to connect to TDE databases from PCT tasks (in a deployment scenario, TDE doesn't make any sense during a build). The way to connect to a TDE database is to add -Passphrase to the connection string, and this is only required when connection is done in single-user mode. When -Passphrase is used, the ABL session will read from stdin the passphrase.

It should be possible to connect to TDE databases from PCTRun (and inherited tasks), with an additional "passphrase" attribute in the DBConnection class. Stdin should only be modified if the passphrase has to be passed (check singleUser attribute). Additional tasks will also have to handle TDE: IndexRebuild, BinaryDump, BinaryLoad.

The passphrase shouldn't be leaked, so it should never be included in log output. I think the recommendation would be to have the passphrase as an environment variable, and then reference it in the passphrase attribute. Passing the passphrase as an Ant property in the command-line makes it visible in the list of processes, so that wouldn't be a good idea. More on this later...

gquerret commented 1 year ago

PCTRun doesn't connect to databases from the command line, so the -Passphrase and input redirect solution doesn't work in this case. The CONNECT statement only allows -KeyStorePassPhrase followed by the passphrase in double quotes. As the list of connection string is written in the pctinit procedure, including the passphrase in cleartext is not an option. The only way to hide the passphrase will be to use substitute('-db xxx -1 -KeyStorePassPhrase "&1"', os-getenv("passphraseDB1")). Another way would be to read the output of a command line (in parameter).

gquerret commented 1 year ago

DynamicRun also needs to be changed: the connection string is passed in the JSON file, so a separate entry should be prepared for the environment variable or the command line.

gquerret commented 1 year ago

Merged in main:

BinaryDump and BinaryLoad tasks will be done later (if required).

PS: just discovered that Ctrl + E insert backticks around the current selection, or current word in case nothing is selected. Amazing... Especially given where backticks are mapped on a French keyboard.

gquerret commented 1 year ago

Merged in main branch. Removed passphraseEnvName properties.