TheThingsIndustries / generic-node-se

Generic Node Sensor Edition
https://www.genericnode.com
Other
110 stars 31 forks source link

LoRaWAN apps EUI and keys handling #56

Open elsalahy opened 4 years ago

elsalahy commented 4 years ago

Summary:

In the case of not using a secure element. We need to Improve user experience with EUI and Keys handling for LoRaWAN apps.

Why do we need this?

For the user to be able to edit the EUIs and keys in a straight forward manner.

What is already there? What do you see now?

Confusing commissioning file used by the ST LMN port.

What is missing? What do you want to see?

How do you propose to implement this?

Environment:

Baremetal

Acceptance Criteria:

What can you do yourself and what do you need help with?

All

elsalahy commented 3 years ago

Brain dump of relevant issues:

People are having issues setting up LoRaWAN keys/EUIs on the device, this is for many reasons

  1. Too many options need to be configured (stack issue)
  2. LoRaWAN 1.0.x and 1.1.x confuse people still
  3. There is no documentation on how to do it

The better experience of using the onboard secure element has many barriers:

  1. User needs to know V3
  2. User needs to add a gateway and configure it for V3
  3. User needs to claim the device, and activate it
  4. User needs to flash the app that uses the HW secure element instead of the using the SW keys
wienke commented 3 years ago

Is the high power use of the secure element also a down side of using the secure element?

elsalahy commented 3 years ago

@wienke That might be another issue, I will analyze this and get back to you if it is relevant in LoRaWAN

elsalahy commented 3 years ago

@marnixcro can you please undertake this.

mcserved commented 3 years ago

For a discussion, here is my suggested changes for the handling of keys. #138 already featured the most important issues with the key handling for me.

The suggestion in #184 is an improvement for me since Git wouldn't track the keys anymore, but has some flaws in my opinion since:

  1. The application keys would not be specific to a device anymore. This can be (and is listed as) an advantage, especially if you are testing the applications and only want one. But I think it would be better to leave this separate (as was done with the current solution) as there can be use cases where users want to define their specific keys for a specific application. Copying over the keys to all application would be easy enough in my opinion.
  2. This would decentralise the configurations from the others in the 'conf' folder.

But how can we improve the current setup then? In my opinion there are some things we could try:

  1. Rename app_conf.h to app_conf.def.h and tell users to copy it over during build. app_conf.h would be used in all includes, and can still be added to .gitignore.
    • Advantages: requires the least amount of changes compared to the other solutions; all configurations would not be tracked by git, not just the keys.
    • Disadvantages: code can't compile until changes are made; extra configuration step would be required (copying the file over).
  2. Same as 1, but separate the keys files and only make a keys.def.h file.
    • Advantages: keys more easily distinguished from the others, including when the compile stage fails as it would specifically note that one of the keys is not compiled; Copying key files to all other applications is much easier (applications have different 'conf' files, can't be copied directly).
    • Disadvantages: same as 1.
  3. Same as 2 and have the se-identity.h key handling default to a key value when any of the keys remain undefined (when the .def.h file is not copied over yet). This also requires a guard for including the file in the first place to avoid any compile issues.
    • Advantages: compilation works from the start;
    • Disadvantages: could be confusing that the code compiles with keys with all zero's when user is editing the .def.h file directly; requires extra configurations step (copying the file over).

Another advantage (in all solutions, but the easiest for solution 3) is that the keys don't have to be defined as all 0's. Se could leave the values empty and have the user fill it with the copied over key values without them having to delete the standard value first.

Additionally we could have Cmake build the configuration file as a build step. This would be my preferred solution if we exclusively supported Cmake or Make, but it's not possible for the other (possible future) build tools. But it can still be done regardless.

If we really feel that keys should be untracked by Git, then we could go for solution 3 in my opinion as it makes sure the applications can still be compiled when testing some quick changes. I would also suggest we do something similar for cross.cmake's toolchain_prefix variable as this is also frustrating to keep as a change. All proposed solutions would require proper documentation (in the .def.h files, gnse-docs website, README, etc.) that they are meant to be copied over and edited accordingly.

elsalahy commented 3 years ago

@marnixcro Thank you for the proposals. I'm in favor of option 2.

But I think we should experiment with having one key file for all applications. I think it should be something like software/app/conf/lorawan_keys_conf.h

@marnixcro what are your thoughts on this, do you see a lot of disadvantages?

mcserved commented 3 years ago

@marnixcro Thank you for the proposals. I'm in favor of option 2.

But I think we should experiment with having one key file for all applications. I think it should be something like software/app/conf/lorawan_keys_conf.h

@marnixcro what are your thoughts on this, do you see a lot of disadvantages?

I don't really see any major disadvantages no, I just though you would prefer keeping the old configuration per application style. But I guess it makes more sense to have one master key for all the applications. I'm not sure how most users use the keys and if they use different keys for different applications, although that is probably not the case at all or just a small minority. Another downside is that it can be confusing for applications using the secure element. Both issues aren't really dealbreakers so I would be on-board with this change.

elsalahy commented 3 years ago

@marnixcro I'm working on #133 to reduce the configurations complexity, and per app conf is indeed a manageable style.

We need a common config (for all apps) and a private app config (for each app) Key configurations falls under common conf.

Can you please propose a LoRaWAN key config header that can be used by all applications? Please propose it here before jumping to implementation.

mcserved commented 3 years ago

@elsalahy personally I think that app/conf/lorawan_keys_conf.h is already good enough but I guess we could go for app/conf/lorawan_keys.h to make it a bit shorter (lorawan_keys_conf.def.h is kinda long).

elsalahy commented 3 years ago

Yeah I agree lorawan_keys_conf.h is good enough for now

NicolasMrad commented 2 years ago

any updates here?

azerimaker commented 2 years ago

@NicolasMrad we do have an option to switch the EUI keys from the secure element to a user defined EUI, but the code needs to be cleaned up and merged.