Build time configuration for CSR

[james-ctc]

Background

I'm using the client certificate as a TLS server certificate for MQTT and need to include the required subject alternative name fields in the certificate signing request (CSR).

i.e. subject alternative name can be added with DNS name and/or IPv4 address

Implementation

By default the subject alternative name extension is not added to the CSR. When CSR_DNS_NAME or CSR_IP_ADDRESS are set using cmake then a subject alternative name extension is created and populated with the supplied values,

e.g.

cmake -DCSR_DNS_NAME=install.pionix.de

Possible future enhancement

APIs and methods could be updated to enable run-time configuration

[AssemblyJohn]

I do not think that a compile-time configuration is a good option for this, the interface should be modified to support this for runtime only. We will clog the make with too much info. I think there should be added one more interface function 'gen_csr' whatver that receives a full struct with all the possible params so that in the future we will not be blocked by any other necessary params. On my side this doesn't have a go. @Pietfried what is your take on this? Shouldn't we stick with the initial design?

[james-ctc]

Suppose I want to use RSA rather than EC for the CSR. At the moment this is hard coded to CryptoKeyType::EC_prime256v1
If it was configurable (as done for CSR_DNS_NAME) then it would be easy to choose. The only option remaining is to edit evse_security.cpp (or apply a patch to it).

I feel supporting a cmake define is a much cleaner solution than requiring file edits or patches.

Perhaps there is an intent for info.key_info.key_type to be configurable, but it isn't yet.
Hence could not this be treated in the same way? i.e. a starting point and not the full solution.

This approach doesn't change any method calls or MQTT messages and hence is safe. The defaults leave existing operation untouched.

[AssemblyJohn]

That is correct, I've made a commit to fix the issue. We only need to modify the relevant interfaces to use it. Again, you are right, I should not have hard-coded these values in code (like the keys), but they should be hardcoded even less in the cmake.

[hikinggrass]

Yes I agree, moving directly to making this runtime configurable is probably the best way forward

[Pietfried]

Agree with making this more flexible and runtime configurable

[james-ctc]

I'm not keen on run-time configuration since the information in the CSR must match the information in system setting files for hostapd, systemd network information, and dnsmasq.

In yocto I can configure variables so that all the configuration is consistent and can't be accidentally changed.
I feel runtime configuration should not be used where a change in the setting can break the system.

EXTRA_OECMAKE += "-DCSR_DNS_NAME=${CHARGER_DNS_NAME} -DCSR_IP_ADDRESS=${HOTSPOT_IPADDRESS}"

is preferable to applying a patch to the code.

[AssemblyJohn]

The patch has already be applied, check the last commit. We now have a full info CSR generation. However if this is a 'must' then I'd say let's make it to impact the cmake builds as little as possible, and not require to know about it too much if somebody does not use it. But it it is used, then the cmake option should over-write the info parameters.

However before implementing, I'd also want to hear what @Pietfried @hikinggrass have to say.

[hikinggrass]

I'm not keen on run-time configuration since the information in the CSR must match the information in system setting files for hostapd, systemd network information, and dnsmasq.

In yocto I can configure variables so that all the configuration is consistent and can't be accidentally changed. I feel runtime configuration should not be used where a change in the setting can break the system.

EXTRA_OECMAKE += "-DCSR_DNS_NAME=${CHARGER_DNS_NAME} -DCSR_IP_ADDRESS=${HOTSPOT_IPADDRESS}"

is preferable to applying a patch to the code.

With this extra info/additional constraints I would also support (at least the possibility) of making this compile-time configurable. There's an argument to be made for runtime configurable settings during development, but on an embedded system you probably want to limit the possibilities of mis-configuring something