Add to for Secrets to be generated before generating secrets #226

DonRichards · 2022-02-10T15:12:43Z

Purpose of this:

In a Slack chat it was brought up that users may be running default secrets and not know it and secrets are generated on every build. In fact there was several small improvements that were needed.

Currently the build process will always generate secrets even when USE_SECRETS is set to true . This adds a simple check then prompt that prevents the code from running the generate script (re)populating the /secrets/live* directory on every make up/down.

For the sake of keeping the description simple I'm going to use the terms make local, make up or make demo interchangeably as a reference to the make build process. It should not matter which of these to use to build ISLE. The tests will not only test the functionality but the ability to bypass/skip a function or not do anything at all. If USE_SECRETS is set to false or all the secrets exist in the secrets/live/ directory, this will not prompt the user for input.

3 scenarios to test for

Test USE_SECRETS set to either true or false for these 3 scenarios. But USE_SECRETS will always result in secrets/live/ being repopulated with new secrets every time a make build process is ran.

Empty secrets/live/ folder
secrets/live/ is missing some of the expected secrets
Found default values in secrets/live/ secrets

Also because this is part of the build process, you will need to run make clean between comparing current behavior vs new and different scenarios.

To test that this doesn't break existing setups

Switch to this PR's branch
With Isle either running or not running but previously built with secrets populated run the make up command.

You will see a message informing you if you're using default values, otherwise no message.

Run make clean to reset. Copy sample.env and modify the .env file to USE_SECRETS=false.

To test when USE_SECRETS is set to false

Run make local
This should automatically generate custom secrets.

To check this worked

Check to see if the directory /secrets/live is populated with secrets, most of them are NOT set to "password"

Run make clean to reset. Copy sample.env and modify the .env file to USE_SECRETS=true to disable new secret generation as the default action.

Testing that answering No to prompt results in no changes with USE_SECRETS is set to true

When the script determines there's an issue, the user is prompted to either generate or copy secrets. This first check verifies nothing happens when n or N is applied to the prompts.

Run make local
Would you like to generate random secrets? Run a script to create secrets? [y/N] N
Would you like to copy the default secrets? Run a script to copy secrets? [y/N] n
Would you like to copy the missing secrets from /home/don/github/isle-dc/secrets/template/? [y/N] n
It should exit completely.

To check this worked

Check to see if the directory /secrets/live is still empty.

Testing auto generate secrets with USE_SECRETS is set to true

Run make demo
Would you like to generate random secrets? Run a script to create secrets? [y/N] y
This should automatically generate custom secrets just like the last test.
Run make up again
There should be no message/warnings.

To check this worked

Check to see if the directory /secrets/live is populated with secrets, most of them are NOT set to "password"

Testing copying template secrets with USE_SECRETS is set to true

Run make clean to reset. Copy sample.env and modify the .env file to USE_SECRETS=true to disable new secret generation as the default action.

Run make local
Would you like to generate random secrets? Run a script to create secrets? [y/N] N
Would you like to copy the default secrets? Run a script to copy secrets? [y/N] y
This will copy the template/ files to the live/ directory. But it's advised to make changes to these files before proceeding. The messages should be clear (if not please suggest something). This also exits not only the bash script but also the MAKEFILE. This was does intentionally.
Run 'make up' again
This will check the config and output a message at both the beginning and the end of the build. This message will inform the user which secrets are using default values with information on where to go for further information.
Run make up again and the message should come up again last to inform the user they are using default/insecure secrets.

Testing with missing secrets while USE_SECRETS is set to true

Delete 1 or many files in /secrets/live
Run 'make up'
Would you like to copy the missing secrets from /home/don/github/isle-dc/secrets/template/? [y/N] y
This will copy (not generate) the missing secrets from the template directory.
Run make up again and message is set back to the warning message when a default value is detected.

Note: This points to where the documentation should be but that doesn't exist yet.

Sorry for the really really long description.

DonRichards · 2022-02-10T15:37:14Z

Added update in the docs to reflect the improvement.

joshdentremont · 2022-02-10T16:25:23Z

@DonRichards I don't think this will work for randomly generated secrets. The other thing USE_SECRETS does is include the file docker-compose.secrets.yml. As I understand it, without this file it won't use secrets at all and must use hard coded values in the containers. This would mean that if USE_SECRETS=false it would never use the randomly generated secrets in secrets/live.

DonRichards · 2022-02-11T03:13:09Z

@joshdentremont Feel free to test this now. I think it covers everything we talked about.

joshdentremont · 2022-02-14T18:31:40Z

@DonRichards sha1sum is not installed on macOS. When I run make local with USE_SECRETS=false I get a bunch of warnings like this:
scripts/check-secrets.sh: line 100: sha1sum: command not found
and it tells me I am using the default secrets

Also, I think we might want to change the name of the USE_SECRETS variable since it no longer does the same thing as it used to and this could confuse people. Maybe we should rename it to USE_CUSTOM_SECRETS or something like that?

DonRichards · 2022-02-15T14:45:51Z

OK, I'll make the change.

DonRichards · 2022-02-15T14:59:29Z

@joshdentremont Give this a try. I switched it to MD5 which is more common and a default for linux mac.

joshdentremont · 2022-02-15T15:35:02Z

Same problem
scripts/check-secrets.sh: line 100: md5sum: command not found

joshdentremont · 2022-02-15T16:03:09Z

Looks like Mac has a program called md5 that does the same thing
https://stackoverflow.com/questions/8996820/how-to-create-md5-hash-in-bash-in-mac-os-x
https://security.stackexchange.com/questions/65499/difference-between-os-x-md5-and-gnu-md5sum

DonRichards · 2022-02-15T16:35:38Z

I added a OS check since in linux the command is md5sum and OSX is just md5. I switched the Linux version back to sha1sum and OSX to MD5. Also added a bypass for anyone that wants a CICD script to generate new secrets by passing yes into the script directly bash scripts/check-secrets.sh yes. It won't impact other functionality. 1 moment and I'll push this. Github is taking a long time.

DonRichards · 2022-02-15T17:11:10Z

OK, @joshdentremont it's ready.

joshdentremont · 2022-02-15T18:04:45Z

It's telling me I'm using default secrets when I'm not. I think this is because of the following
> md5 secrets/template/DRUPAL_DEFAULT_ACCOUNT_PASSWORD
MD5 (secrets/template/DRUPAL_DEFAULT_ACCOUNT_PASSWORD) = 5f4dcc3b5aa765d61d8327deb882cf99
> md5 secrets/template/DRUPAL_DEFAULT_ACCOUNT_PASSWORD | awk '{print $1}'
MD5

I think what you want for MD5 is this (print $4 instead of print $1):
> md5 secrets/template/DRUPAL_DEFAULT_ACCOUNT_PASSWORD | awk '{print $4}'
5f4dcc3b5aa765d61d8327deb882cf99

DonRichards · 2022-02-15T20:02:31Z

Yeah, made an assumption there. OK. I switched MACs to the failsafe so that I'm not duplicated more code or spending too much time on this. So basically it will compare the cat output for either file. Lets give this one a try.

joshdentremont · 2022-02-16T14:01:48Z

Tested with USE_SECRETS=false and it created the randomly generated secrets but does not use them. I wonder if we should stop if from generating them when it's set to false? This probably doesn't matter but it has confused me in the past.

I also misunderstood how you had set this up so you can ignore my earlier comments about renaming USE_SECRETS. I thought you had made it so USE_SECRETS=false was the way to use randomly generated secrets.

Trying with USE_SECRETS=true and got this issue:
'Uses Secrets' is set to 'true'.
Starting scripts/check-secrets.sh
ls: illegal option -- I
usage: ls [-@ABCFGHLOPRSTUWabcdefghiklmnopqrstuwx1%] [file ...]
This seems to be another Mac difference, and is set on this line: https://github.com/Islandora-Devops/isle-dc/pull/226/files#diff-019c27b7e1851c31b7dff95fc6645cb2aabe7d3642de7128aca156872fb6721cR61

I think you can just drop the -I because ls will not list hidden files anyway unless you give it the -a option. I removed it and things seem to run as intended

Another issue I see is that I changed only the drupal account password and it's still telling me I am using the default. I don't know if there's a check for individual ones or if it just outputs all the secrets when any of them are using the defaults.

joshdentremont · 2022-02-16T18:59:09Z

Setting USE_SECRETS=true then saying no to generating and copying secrets causes the build to quit and no secrets to be generated.

Setting USE_SECRETS=true then accepting random secrets seems to work as intended.

Setting USE_SECRETS=true then using the defaults seems to work as intended. It exits the first time to give you opportunity to replace the defaults, and if you don't it gives the appropriate warnings

Setting USE_SECRETS=true then overwriting the defaults also works as intended and uses the custom secrets without warnings.

I would say this is all working except for the ls -I issue and the fact that it seems to be all or nothing with checking whether some secrets are set to the default.

DonRichards · 2022-02-21T19:11:50Z

@joshdentremont I can remove the "i" part of that command. As for changing the admin password, if you're using the make to change the password it should update that file. If you're changing the password within Drupal directly it does not output the password to the file and won't be able to track updated passwords like that.
One of the pros to always having this run upon build is it will always warn the user if they are using defaults and/or if they're missing newly created secrets.

DonRichards · 2022-02-21T19:14:44Z

@joshdentremont Ready to check

DonRichards · 2022-02-24T20:39:48Z

I'll give that a try and push as soon as I can.

DonRichards · 2022-02-25T14:21:56Z

@joshdentremont OK, ready.

joshdentremont · 2022-02-25T20:08:30Z

@DonRichards Looks good now.

DonRichards · 2022-03-30T18:46:14Z

Ready for testing

misilot · 2022-04-08T00:42:12Z

This worked for me, to not stop on existing secrets being there. ❤️

misilot · 2022-04-08T01:00:34Z

README.md

+To enable using secrets after run `make local` or `make up`, set 
+`USE_SECRETS=true` in your .env file. When you run `make docker-compose.yml`, a
+large block of `secrets` will be added at the top of your `docker-compose.yml`
+file.


Might want to also mention that they should be sure to set

to the value of DRUPAL_DEFAULT_SALT to $settings['hash_salt'] in codebase/web/sites/default/settings.php and the value of DRUPAL_DEFAULT_DB_PASSWORD to the value $databases['default']['default']['password'] in the same file (it's actually in the file twice??)

Or maybe we should use the file path option instead of parsing it into the file? $settings['hash_salt'] = file_get_contents('/home/example/salt.txt');

Also will a make up update passwords if they are created after a make local or make up?

I only see $databases['default']['default']['password'] once in my install.

I just pushed a warning to the user to manually update their settings.php file when it notices a difference between the secrets/live/DRUPAL_DEFAULT_SALT and web/sites/default/settings.php file. I felt like it might take too long to try to automatically do this for the user and restart their container because this script is running on the local and we'd need to account for Mac command discrepancies.

It looks like it was my older version had some extra things in there

misilot · 2022-04-08T01:15:39Z

I also think it might be worth at somepoint just creating secrets or using existing ones going forward, and not, not having them. ie "USE_SECRETS" goes away except maybe as a legacy option for existing systems? (expert option?)

seth-shaw-unlv · 2022-04-19T16:07:56Z

The last test for the missing secret prompt didn't work when I tested this morning:

~/isle-dc$ rm secrets/live/DRUPAL_DEFAULT_DB_PASSWORD
~/isle-dc$ make up
WARNING: Some services (activemq, alpaca, blazegraph, cantaloupe, crayfits, drupal, fcrepo, fits, homarus, houdini, hypercube, mariadb, matomo, milliner, recast, solr, traefik) use the 'deploy' key, which will be ignored. Compose does not support 'deploy' configuration - use `docker stack deploy` to deploy to a swarm.
WARNING: Service "drupal" uses an undefined secret file "/home/sshaw/isle-dc/secrets/live/DRUPAL_DEFAULT_DB_PASSWORD", the following file should be created "/home/sshaw/isle-dc/secrets/live/DRUPAL_DEFAULT_DB_PASSWORD"

I should also note that the requests for make demo don't result in a working site anyway, but that is probably a separate issue.

DonRichards · 2022-04-28T13:09:37Z

@misilot @seth-shaw-unlv This is ready to test again.

seth-shaw-unlv · 2022-04-28T15:20:09Z

In the section "Testing auto generate secrets with USE_SECRETS is set to true" it indicates that after selecting "yes" one should run "make up" again. However, the script simply continues running the "make demo". Is that the expected behavior, @DonRichards?

DonRichards · 2022-04-28T16:59:55Z

No. I don't believe so. Let me double-check now

DonRichards · 2022-04-28T17:12:33Z

@seth-shaw-unlv Sorry, yes, that's correct—this way new people can build right away with as few steps as possible. But if a person is only copying files over, I assume it's for a purpose (to add their passwords to the files).

DonRichards · 2022-04-28T17:14:25Z

Oh, and it should ask the user if they want to exit when they opt for copying the defaults over.

Makefile

seth-shaw-unlv

👍

…ra-Devops#226)

DonRichards added 3 commits February 10, 2022 09:32

Add to for Secrets to be generated before generating secrets

fa20607

Copy the defaults over if disabled

1635054

Add no clobber to the copy process

a523c07

DonRichards mentioned this pull request Feb 10, 2022

Unable to manually set secrets #220

Closed

DonRichards added 2 commits February 10, 2022 10:23

Phrase reversed is message.

889a945

Update docs for SECRETS

3d25784

DonRichards added 3 commits February 10, 2022 20:45

Added more info and tests to check secrets

0604e8a

typo

26a05e2

Fix exit when all no to prompts

583e055

Switch hash to md5 instead of sha1 because sha1 isn't n macs by default

287a40e

Add automation for generator script with docs

4b5a649

OS detect to determine which command to use to check hashes

f7ab6d1

move mac specific hash to cat the files

412426a

Remove the exclude in 'ls' check

2e13e9a

Correct the secret name variable

e3a1f9a

DonRichards requested a review from nigelgbanks March 2, 2022 15:58

Merge branch 'development' into fix_secrets_check

1938af9

DonRichards requested a review from a team April 6, 2022 17:56

misilot mentioned this pull request Apr 8, 2022

Should secrets be stored in the "codebase" directory? #249

Closed

misilot suggested changes Apr 8, 2022

View reviewed changes

seth-shaw-unlv self-requested a review April 13, 2022 17:09

DonRichards added 4 commits April 20, 2022 10:11

Add Salt warning

a735794

Improve warning

369990b

Merge branch 'development' into fix_secrets_check

a16d9ed

Correct salt warning

f9dccda

seth-shaw-unlv reviewed Apr 28, 2022

View reviewed changes

Makefile Outdated Show resolved Hide resolved

Remove unneeded comments

dae6b1a

seth-shaw-unlv approved these changes May 18, 2022

View reviewed changes

seth-shaw-unlv merged commit efd5d5f into development May 18, 2022

seth-shaw-unlv deleted the fix_secrets_check branch May 18, 2022 17:55

misilot pushed a commit to misilot/isle-dc that referenced this pull request Jun 1, 2022

Add to for Secrets to be generated before generating secrets (Islando…

9e3717b

…ra-Devops#226)

misilot pushed a commit to misilot/isle-dc that referenced this pull request Jun 6, 2022

Add to for Secrets to be generated before generating secrets (Islando…

d8f6cdf

…ra-Devops#226)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add to for Secrets to be generated before generating secrets #226

Add to for Secrets to be generated before generating secrets #226

DonRichards commented Feb 10, 2022 •

edited

Loading

DonRichards commented Feb 10, 2022

joshdentremont commented Feb 10, 2022

DonRichards commented Feb 11, 2022

joshdentremont commented Feb 14, 2022

DonRichards commented Feb 15, 2022

DonRichards commented Feb 15, 2022

joshdentremont commented Feb 15, 2022

joshdentremont commented Feb 15, 2022

DonRichards commented Feb 15, 2022

DonRichards commented Feb 15, 2022

joshdentremont commented Feb 15, 2022

DonRichards commented Feb 15, 2022

joshdentremont commented Feb 16, 2022 •

edited

Loading

joshdentremont commented Feb 16, 2022

DonRichards commented Feb 21, 2022

DonRichards commented Feb 21, 2022

DonRichards commented Feb 24, 2022

DonRichards commented Feb 25, 2022

joshdentremont commented Feb 25, 2022

DonRichards commented Mar 30, 2022

misilot commented Apr 8, 2022

misilot Apr 8, 2022

misilot Apr 8, 2022

DonRichards Apr 20, 2022

misilot May 4, 2022

misilot commented Apr 8, 2022

seth-shaw-unlv commented Apr 19, 2022

DonRichards commented Apr 28, 2022

seth-shaw-unlv commented Apr 28, 2022

DonRichards commented Apr 28, 2022

DonRichards commented Apr 28, 2022

DonRichards commented Apr 28, 2022

seth-shaw-unlv left a comment

Add to for Secrets to be generated before generating secrets #226

Add to for Secrets to be generated before generating secrets #226

Conversation

DonRichards commented Feb 10, 2022 • edited Loading

Purpose of this:

3 scenarios to test for

To test that this doesn't break existing setups

To test when USE_SECRETS is set to false

To check this worked

Testing that answering No to prompt results in no changes with USE_SECRETS is set to true

To check this worked

Testing auto generate secrets with USE_SECRETS is set to true

To check this worked

Testing copying template secrets with USE_SECRETS is set to true

Testing with missing secrets while USE_SECRETS is set to true

Sorry for the really really long description.

DonRichards commented Feb 10, 2022

joshdentremont commented Feb 10, 2022

DonRichards commented Feb 11, 2022

joshdentremont commented Feb 14, 2022

DonRichards commented Feb 15, 2022

DonRichards commented Feb 15, 2022

joshdentremont commented Feb 15, 2022

joshdentremont commented Feb 15, 2022

DonRichards commented Feb 15, 2022

DonRichards commented Feb 15, 2022

joshdentremont commented Feb 15, 2022

DonRichards commented Feb 15, 2022

joshdentremont commented Feb 16, 2022 • edited Loading

joshdentremont commented Feb 16, 2022

DonRichards commented Feb 21, 2022

DonRichards commented Feb 21, 2022

DonRichards commented Feb 24, 2022

DonRichards commented Feb 25, 2022

joshdentremont commented Feb 25, 2022

DonRichards commented Mar 30, 2022

misilot commented Apr 8, 2022

misilot Apr 8, 2022

Choose a reason for hiding this comment

misilot Apr 8, 2022

Choose a reason for hiding this comment

DonRichards Apr 20, 2022

Choose a reason for hiding this comment

misilot May 4, 2022

Choose a reason for hiding this comment

misilot commented Apr 8, 2022

seth-shaw-unlv commented Apr 19, 2022

DonRichards commented Apr 28, 2022

seth-shaw-unlv commented Apr 28, 2022

DonRichards commented Apr 28, 2022

DonRichards commented Apr 28, 2022

DonRichards commented Apr 28, 2022

seth-shaw-unlv left a comment

Choose a reason for hiding this comment

DonRichards commented Feb 10, 2022 •

edited

Loading

joshdentremont commented Feb 16, 2022 •

edited

Loading