Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hardcoded settings, for ease of deployment and improved security #1839

Open
lbdroid opened this issue Aug 2, 2019 · 7 comments
Open

Hardcoded settings, for ease of deployment and improved security #1839

lbdroid opened this issue Aug 2, 2019 · 7 comments
Labels
enhancement New feature or request

Comments

@lbdroid
Copy link

lbdroid commented Aug 2, 2019

Given that it is now becoming possible to secure MQTT; #1829

I would like to be able to pre-configure all user settings in the firmware prior to deploying to a device. My goal is to be able to write the firmware onto a device, plug it in, and not have to make any configurations to the device at all.

In achieving this, it would be possible to entirely disable both WEB and TELNET interfaces. This provides the double benefit of reducing the size of the firmware, as well as minimizing the attack surface.

Some configurations are already possible via custom.h, but nowhere near all and not enough to avoid having to make manual configurations (or "restore" a json encoded backup).

@lbdroid lbdroid added the enhancement New feature or request label Aug 2, 2019
@mcspr
Copy link
Collaborator

mcspr commented Aug 3, 2019

...which is already possible by using #define KEY VALUE? Most runtime settings are set up like getSetting("alexaEnabled", ALEXA_ENABLED) (just git grep getSetting for all of those), using define token as default if settings key is not set. Although, they may not be thoroughly documented and some are missing. But see WIFI1_SSID, MQTT_SERVER etc. for some examples of the existing ones.
Or do you mean disabling this mechanism in its entirety, forcing to always use default values instead of dynamic settings?

@PieBru
Copy link

PieBru commented Aug 3, 2019

@ibdroid this is why I use USE_CUSTOM_H, look for it and define your values inside custom.h, they will override system defines.
Piero

@lbdroid
Copy link
Author

lbdroid commented Aug 5, 2019

@PieBru , @mcspr :
I'm fully aware of using custom.h with #defines to set some of the default settings. It goes a long way, but isn't complete, since there are quite a number of settings that can't be made in that manner.

For example; dczRelayIdxX

Other settings are supposedly available, but don't work.
dczEnabled is defined (DOMOTICZ_ENABLED), but doesn't work from custom.h because it is defined as 0 in general.h -- its a redefinition problem.

@mcspr
Copy link
Collaborator

mcspr commented Aug 5, 2019

Then we can add those? What would those be, #define DOMOTICZ_RELAY1_IDX 1?
Same 1-based indexing as buttons have in the name, and the same logic as the setting - relay index first, then custom domoticz index number.
0 by default, following the webui comment "Set IDX to 0 to disable notifications from that component.".

8d009a6 fixes the ifndefs btw

@lbdroid
Copy link
Author

lbdroid commented Aug 5, 2019

That's only the tip of the iceberg though.
But there is also the question of having two ways of setting everything and all of the excess code it results in. Simpler and more easily maintained approach would be to eliminate the majority of the defines-based configurations and replace it with a list of default key-value pairs to be loaded on first boot.

@mcspr
Copy link
Collaborator

mcspr commented Aug 6, 2019

Likely to happen, and would help documentation too. But how would you expect to modify / override it? What format this would be exactly?

Some examples from before:
https://github.com/xoseperez/espurna/tree/v2/code , embedding device config .json in the tree. Needs some build tool to actually import those into the firmware as binary headers. Can be avoided by using string literals, but it kind of inefficient with firmware size that way. JSON parsing is tricky too, because memory requirements are dependant on the .json size
#1279 , using something similar to the ESPHome approach, preparing external config files. Requires platformio build script to parse things (however, it can be just standalone python script, but we need toolchain)
#1680 , which forces migrate.ino to do setSetting(key, DEFINE_DEFAULT) on everything possible at boot. Also uses platformio and needs to know where the toolchain files are. It is probably overly optimistic to dump everything into the runtime settings though, defaults need to stay somewhere else.

@mcspr
Copy link
Collaborator

mcspr commented Jan 17, 2020

Referencing #2048
The main issue with defines is that the values are unknown to anyone outside of module, key <=> define association is only ever used by getSetting directly when constructing things in Configure() or WebSocketData() functions. Sure, defines can become explicitly bound variables, and that might also solve some issues with PROGMEM stuff, but...

When WebUI does parse the data, it is simply comparing string values based on key.If I am not overthinking the process, just as there is a key prefix checker there could be a module-specific method() accepting key as argument to retrieve default value. This complicates stuff b/c then there needs to be a specific "map"-like structure to have the same API as Embedis settings do. And this somehow needs to be stored at compile-time, not runtime and probably each setting needs to become a string too. Aand this somehow needs to also contain flash-strings, which either need to have fixed length of reference another pointer variable.

The main issue with that approach is that now we need a pre-defined structure format to deal with dynamic nature of settings. That is, I am not aware of any non-centralized way to have such a "variant"-type structure per-module and also have multiple types at the same time.
This is used by ESPeasy & Tasmota, bundling settings into one big struct with module-specific fields (e.g., see settings.h)

So this poses another question - if Embedis allows to read arbitrary data blobs as simple maps, why not encode string kv pairs at compilation time into an Embedis-compatible structure (which is simply <value><(uint16_t)value-len><key><(uint16_t)key-len> read from the end of the blob). Then, internally, Embedis can already work with multiple sources of keys (aka dictionaries), so there could be a specific read-only dictionary with default values.
OFC, something / someone needs to build this dictionary. Which may be done automatically by PIO build scripts via small hints inside of module files.
(but, Arduino IDE always becomes a major bummer when trying to add anything fun to the build process...)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants