Place LoRaWAN keys in a separate file, separate from source code and application configuration settings

[lnlp]

Summary: Place LoRaWAN keys in a separate file, separate from source code and application configuration settings

Why do we need this?

• Protection against accidentally uploading LoRaWAN keys to public Git(hub) repositories.
• Prevent that source code needs to be modified for changing keys (or configuration files where LoRaWAN keys are mixed with application settings).
• Make it easy to swap LoRaWAN keys for testing purposes or for use with multiple devices.
• Make possible to share the LoRaWAN keys for a device by all relevant LoRaWAN examples included in the repository, without having to add/update/change the keys for each example application separately.

What is already there? What do you see now? LoRaWAN keys are currently defined in file app_conf.h where they are mixed with regular application settings. This has several disadvantages (see why do we need this above).

What is missing? What do you want to see? Separation of LoRaWAN keys from source code and from application configuration files.

How do you propose to implement this? Create Software/keyfiles folder for storing key files that can be used by all LoRaWAN examples. In this folder add file lorawan_keys.h which contains the actual keys.

In se-identity.h #include lorawan-keys.h instead of app_conf.h and remove the OTAA keys from app_conf.h.

Exclude the contents of this folder and any keyfiles from the repository (in .gitignore).

Add file lorawan_keys_example.h to the keyfiles folder and DO include the example in the repository (in .gitignore). This example file can be copied to lorawan_keys.h by the user for adding the LoRaWAN keys for a device. The lorawan_keys.h file will be excluded from the repository so the LoRaWAN keys will not be (accidentally) uploaded to (public) repositories.

File lorawan-keys.h can include both keys required for OTAA and ABP (use the ones appropriate).

To make things even easier for the user, clearly indicate which keys are required for OTAA and which for ABP (by prefixing the key names with OTAA or ABP).

When preferred the keyfiles folder can contain multiple keyfiles, e.g. testdevice1_lorawan_keys.h and testdevice2_lorawan_keys.h. The files contain the actual keys ('are copies of lorawan_keys_example.h') and file lorawan_keys.h only needs to contain an include statement like #include "testdevice1_lorawan_keys.h". This include statement can be easily edited as needed to use one of the other keyfiles without requiring any modifications to application source code or application settings.

Environment: Not relevant.

Acceptance Criteria: No specific acceptance criteria.

What can you do yourself and what do you need help with? I will create a PR for my code where this has already been implemented. I need help from TTI to merge the changes from the PR so everyone can benefit and use this functionality.

[elsalahy]

@lnlp I like the idea of having one central location for the keys and happy to assist you with creating a PR for this. The current solution is a stop-gab solution and we have this issue opened #56 Let's discuss the design decisions before we jump into implementation.

I'm interested in learning from you:

• Did you observe se-identiy.h?
• Have you considered how this will work with secure_element_lorawan app where there is no need for keys at all?
• Why not create testdevice1_app_conf.h and testdevice2_app_conf.hor better yet testdevice1_se-identiy.h and testdevice2_se-identiy.h

[lnlp]

I'm interested in learning from you:

• Did you observe se-identiy.h?

Yes. se-identity.h is where the keys are actually defined. In the current examples you have placed the OTAA (but not ABP) keys in conf.h and have #included conf.h in se-identity.h.

• Have you considered how this will work with secure_element_lorawan app where there is no need for keys at all?

Why should I? The secure_element_lorawan example needs neither OTAA nor ABP keys to be defined in configuration. My feature proposal only targets relevant example applications where keys need to be specified (of course).

Why not create testdevice1_app_conf.h and testdevice2_app_conf.hor better yet testdevice1_se-identiy.h and testdevice2_se-identiy.h

1. I think that what you mean here is why not put app_conf.h or se-identity.h in a shared central location. Essential for using lorawan_keys.h are:

   • that it contains LoRaWAN keys only, not mixed with source code or application settings,
   • that it can be shared with/used by multiple applications,
   • that it can be easily swapped without changing application settings or source code
   • that it is placed in an easily recognizable central location
   • that it can be easily excluded from the repository (in a recognizable location).

2. File se-identity.h contains values for LoRaWAN v1.0.x and v1.1.x keys. This file is fairly complex and is not a simple key-value pair type configuration file however. It also contains source code implementation details/dependencies (the key list structures).

   My preference would be to place all LoRaWAN keys in a simple key-value pair type configuration file (e.g. lorawan_keys.h) and place the key list structure definitions in a separate file (e.g. se-identity.h) which uses the definitions of the first file (#include lorawan_keys.h). This makes the LoRaWAN keys configuration file simpler, easier to maintain and understand and hides implementation details from the user. File se-identity.h in that case can be moved out of the conf folder because it will not contain user modifyable configuration settings anymore.

   Implementation of my proposal can be done in two phases:

   1. Implement current proposal: place LoRaWAN v1.0.x key definitions in file lorawan_keys.h. This means that the advantages can be used now already (there are no disadvantages).
   2. When LoRaWAN v1.1 becomes relevant/available then any additional keys required for LoRaWAN v1.1 (and the key list structures) can be added to lorawan_keys.h and the key list structure definitions (in se-identity.h) can be updated accordingly.

3. app_conf as its name implies contains application configuration settings, not (only) LoRaWAN keys. Application configuration settings and LoRaWAN keys from semantic perspective are totally different things. There can be 10000 devices that all use the exact same application configuration settings but all use different LoRaWAN keys. Application configuration settings and the value of LoRaWAN keys should therefore not be combined in a single configuration file.

I can create a PR for implementation phase 1.

FYI: The concept of using a separate LoRaWAN keys file is already successfully applied in LMIC-node.

[elsalahy]

@lnlp you mention

My feature proposal only targets relevant example applications where keys need to be specified (of course).

How are you targeting specific applications? How will the user know which app uses the lorawan_keys file and which doesn't?

that it can be shared with/used by multiple applications

Some apps like basic and secure_element_lorawan and many future apps might not use soft SE!

that it is placed in an easily recognizable central location

What is your proposal? considering the current structure?

that it can be easily excluded from the repository (in a recognizable location)

I think the current structure provides this flexibility if the user excludes all of the conf folder in his .gitignore

File se-identity.h contains values for LoRaWAN v1.0.x and v1.1.x keys. This file is fairly complex and is not a simple key-value pair type configuration file however. It also contains source code implementation details/dependencies (the key list structures

I have no problem that we fix this and hence why #56 exists, but I think we need to do it on per-application basis.

Your solution is a good improvement for readability if we do it for each application that uses Soft SE

I have a suggestion around this:

1. Moving se-identity.h and commissioning.h to this folder similar to how LMN does it
2. Create your proposed file with LoRaWAN v1.0.x support and I suggest naming it soft_lorawan_app_keys.h
3. You can create a separate folder under conf if this is useful in any way, but I fail to see the added value

At the end, this solution will look like a simplified se-identity.h

Please keep in mind we would like each app to be standalone. We accept some code repetition to avoid increasing complexity. For example we opted to have a secure element app even though all lorawan apps can use soft or secure element based keys, but we felt this will introduce complexity and we wanted to provide a clean standalone example on using HW secure elements with LoRaWAN.

[lnlp]

How are you targeting specific applications? How will the user know which app uses the lorawan_keys file and which doesn't?

How is it currently done and how does the user currently know? There is not much that changes in that aspect. It's very simple actually: in app_conf.h, se-identity.h or where ever you want to put it #include "../../../keyfiles/lorawan_keys.h" and additionally in the documentation of course (and possibly a comment somewhere where useful).

that it can be shared with/used by multiple applications

Some apps like basic and secure_element_lorawan and many future apps might not use soft SE!

So what's the point here?!

that it is placed in an easily recognizable central location

What is your proposal? considering the current structure?

As already suggested: put keyfiles in folder /Software/keyfiles so they can be shared and easily excluded (a 'known safe place' that cannot be accidentally forgotten to exclude in another new example). Single place of definition.

Talking about locations, I would even like to suggest to split up the repository in two separate repositories. One dedicated to software (development) and the other dedicated to hardware (development). Both have a very different pace of development and a software engineer will not be interested that much in the hardware repo and the hardware engineer will not be that interested in the software repo. Different versions of the software should be able to run on a single version of the hardware and a single version of the software should be able to run on different versions of the hardware. I don't actually see the benefit of combining hardware and software development in the same repository for users of the repository. It only gives overhead and requires an extra directory level.

that it can be easily excluded from the repository (in a recognizable location)

I think the current structure provides this flexibility if the user excludes all of the conf folder in his .gitignore

That may be your view but I do not fully agree with that, especially for keyfiles. By the way, it is not the user that should add custom .gitignore files to the repository/examples but the owner of the repository should. Otherwise it becomes very impractical for PR's and synchronizing repositories.

1. Moving se-identity.h and commissioning.h to this folder similar to how LMN does it.

If there is any user configurable / device dependent configuration data in these files then placing that configuration data in the source tree of a library would be bad practice.

2. Create your proposed file with LoRaWAN v1.0.x support and I suggest naming it soft_lorawan_app_keys.h

I have no problem with renaming lorawan-keys.h if for good reason and clear to the user.

soft_lorawan_app_keys.h is not (more) clear to me however. It raises the question 'what is a soft key?'. What about the keys provisioned in secure storage, how would you call these? The user will not see latter in any configuration file. lorawan-keys.h will not contain them either so lorawan-keys.h is clear and not confusing as a name.

3. You can create a separate folder under conf if this is useful in any way, but I fail to see the added value

To store keyfiles you mean. Yes, you could but my suggestion is to place keyfiles centrally where they can be shared by all examples (and if the user has reasons to do it otherwise (s)he can adapt the code accordingly (simply change the path of an include file).

You must make it easy for the user where possible. Being able to share keyfile(s) is importantso the user only needs to provide a single lorawan keys file in a single location (and it should be easy to switch keys for testing purposes and for uploading the same application to multiple devices). If the user wants to customize or put a single example in a single repository it's dead simple to only change the path of the include file in the code.

Tip: lorawan-keys.h can only contain an #include to include a different file that actually has the keys in them. This way switching between LoRaWAN keys (between different supplied files) really is very simple and requires only one single change in a single location.

At the end, this solution will look like a simplified se-identity.h

Yes, assuming that you mean in the end lorawan-keys.h (or whatever its name) will contain all LoRaWAN keys in simple key/value pairs, either 1.0.x or 1.1.x (whichever are required) without the current complexity of se-identity.h.

Please keep in mind we would like each app to be standalone.

Using a shared keyfiles folder does not prohibit that. But if an app is to be 'extracted from the repository by the user' to be used stand alone, there is nothing wrong that in such case the user needs to update the include path for the lorawan-keys.h file.

Making the user having to copy the same keyfile to all (relevant) LoRaWAN example applications and have to repaeat again for every change does not make things easy for the user so I still suggest to use a central keyfiles folder.

For example we opted to have a secure element app even though all lorawan apps can use soft or secure element based keys, but we felt this will introduce complexity and we wanted to provide a clean standalone example on using HW secure elements with LoRaWAN.

Yes, I think that is a good thing. On the other hand it will aslo be useful to additionally have an example app where the user can simply configure which of both options to use (this will clearly demonstrate what is common and what is different between the two).

[elsalahy]

@lnlp I agree with some of your points. At the end this is a user experience improvement.

Please feel free to open a PR if you would like to contribute or I can implement this later in the coming months myself as I don't see it as a priority and the proposed change could break functionality.

Please follow our contribution guideline and please be aware that your PR doesn't have to be merged, especially if we feel it creates more issues than it solves. That said, I will be happy to assist you so that we can have a good implementation.

[elsalahy]

@marnixcro can you take on this issue, it's a low priority but it is a good idea to investigate a proper easy solution for handling the keys.

[NicolasMrad]

any updates here?