Alter apply_critical_fixes() to check that a setting has not already been set before forcing to safe default. #66
Conversation
…Security.auth array which is not understood by MISP as part of the settings model.
|
What about |
|
ok, generalising the method to do this but running into a few misp related issues. also fixing up a few other minor things at the same time that have been annoying me. give me a few days and i should have a sound method to deal with settings generally. |
…settings aren't set before setting defaults. Currently env vars override safe settings. Also, create empty config.php instead of using template. Still need to validate all wanted settings are there. Also, update cake's cacerts. Previous method was using ubuntu's crts, which weren't pem.
|
I like this approach. Do you think it would make sense to split config and data and store the JSON data outside the .sh file? That would improve readability/maintainability. |
|
take a gander at that and let me know what you think. |
|
Nice stuff! Would it make sense to merge Little nit: change of tag in docker-compose should be not pushed. |
Some files are pseudo-json as they include bash variables. Enforce all env var driven settings. Add gettext package for envsubst command, for filling in pseudo-json files with env vars. Move unset setting defaulting and enforcement of env var driven settings behind init_settings() function to dramatically improve code reuse.
I am of the opinion that the function of the two files is sufficiently different that they should remain separated for safety. If you still believe that they should be merged, rather than an "enforced" boolean key, probably better to have "enforced" and "unenforced" object keys that contain their respective settings to apply. still, i'd prefer them left as is to avoid unintentional foot shootings.
rebased and pushed without this accident. |
|
@UFOSmuggler followed up in gitter/matrix. |
There was a problem hiding this comment.
As per discussion:
- add some doc in the readme (especially when, i.e., which scenarios, we should update the enforced / non_enforced JSON to add new settings)
- comment out code that is not used
- test the changeset against more complex configurations (OIDC/LDAP)
- test the changeset whether settings in DB / config are used
- change the file names so files containing the same settings are next to each other when sorted
|
Create CLI_only config directives first (in anticipation of enabling db settings), this involves moving some items around, including the workers directives surprisingly Await the arrival of the system_settings db table before processing whether or not to enable the DB. This is not part of the default schema imported upon initialisation. WIP - turn this into an until loop. Place the config template next to config.php as an example for the administrator. Add missing config directives, bring others in line with existing container config values. Fix some minor issues
|
One thing I forgot to ask: what is the impact on users updating to a docker image featuring this new settings system? Is it something we can qualify/quantify? |
yeah the whole point of this was to handle these settings in a manner non-destructive to the choices an instance's admin makes. the previous method will override some choices. this method respects choices and only sets unset settings to sane defaults. as well as enforce envars, which was the previous behaviour. there should be no other impact upon upgrade of or migration to this docker compose stack. unless i've done something wrong. i have some dev and prod instances using the previous system (not these changes) that i plan to test on before i will be happy with my changes. |
Add documentation regarding new envar, and add to docker-compose.yml and template.php Move "weird default" ZeroMQ setting to initialisation settings. Move some settings to cli_only, as by design or perhaps because the settings model and/or app controller just doesn't account for them being in the db, they have to be in the config.php file. Add code to disable DB settings when applying cli_only settings. Change system_settings table availability check to until loop. Some language changes for clarity. Should now be ready for testing.
|
More testing to go (particularly around the OIDC/other security/auth stuff) but so far: Create new docker compose misp-docker stack: Change envar driven setting and restart or recreate container: Change safe default setting and restart or recreate container: Change CLI_only setting and restart or recreate container: Change bare minimum config file only setting and restart or recreate container: Upgrade existing docker compose stack from prior to these changes: Note: Attempting to set MISP.redis_password to "" in the database results in it not being stored in DB at all. Its default value when unset is "" anyway. Recommend fixing by generating random strings for config passwords and secrets, to be stored in config.php. Everything so far looks fine. comparison of diagnostics pages (from vanilla as possible install): old: new: |
|
Testing of configurations using AAD, OIDC, and LDAP resulted in identical configurations being made by the current misp-docker misp-core image, and one built with these changes. In each case, all settings were generated correctly in the config.php file, and if system_settings table was being used, most of the config went in config.php with the remaining small number of config directives going in the databse. |
|
I think we are ready to go. I would add a line to the README file detailing which JSON file to modify when adding a new setting (something that we do on a regular basis, and hence we want to make as straightforward as possible). Something like: But I can add this myself in case. Do you want me to merge, or further polish it? |
|
click the big shiny merge button please. feel free to add text to documentation. i might polish further at a later point in time. |
Introduce new system to persist mandatory and optional settings (MISP#66)
) * Make safe settings functions handling config json objects. * Also, update cake's cacerts. Previous method was using ubuntu's crts, which weren't pem. * Bring config inline with previous config.php template version. * Move settings into files in /etc/misp-docker. * Fix Security.auth kludge. * Rename functions and settings json files for a bit more clarity. * Add documentation to README.md. * Add a bit of context around adding new envars. * Add ENABLE_DB_SETTINGS envar for turning on MISP.system_setting_db. * Add documentation regarding new envar, and add to docker-compose.yml and template.php. * Move "weird default" ZeroMQ setting to initialisation settings. * Move some settings to cli_only. * Add code to disable DB settings when applying cli_only settings. * Change system_settings table availability check to until loop. * Some language changes for clarity.
Fix for #63
Uses cake console commands to get and set settings.
Works with both DB and config.php settings.
Unfortunately, MISP's serverSettings model does not seem to understand Security.auth, which is used for external auth purposes. For this reason we have to do ugly kludge to check this array, then use the existing ugly kludge to set it as an empty array if it does not already exist. For some reason, the previous script was setting it as "" instead of array(), but it is clearly expected to be an array. Probably doesn't really matter though, and we leave it alone if already "" after this change.