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

net: lwm2m: Add support for X509 certificates #59019

Merged
merged 2 commits into from Jun 26, 2023

Conversation

SeppoTakalo
Copy link
Collaborator

@SeppoTakalo SeppoTakalo commented Jun 7, 2023

Add support for using X509 certificates.
Default settings use ECDSA certificates with SHA256 hash.

When different settings are required clients should overwrite struct lwm2m_ctx->load_credentials() and
struct lwm2m_ctx->set_socketoptions()

include/zephyr/net/lwm2m.h Outdated Show resolved Hide resolved
samples/net/lwm2m_client/overlay-dtls-cert.conf Outdated Show resolved Hide resolved
subsys/net/lib/lwm2m/lwm2m_engine.c Outdated Show resolved Hide resolved

switch (lwm2m_security_mode(ctx)) {
case LWM2M_SECURITY_PSK:
ret = zsock_setsockopt(ctx->sock_fd, SOL_TLS, TLS_CIPHERSUITE_LIST,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to narrow the list of cipher suites to use, instead of letting mbedTLS to choose?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because when all PSK and certificate compatible cipher suites were enabled, MbedTLS tried to offer all to Client HELLO causing the handshake to always start with certificate mode.
I had to limit that in runtime to force it to PSK mode.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, I thought mbed TLS would be smart enough not to offer certificate-based suites if there's no cerfiticate registered for the context. Ok then.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see that there is difference on how MbedTLS works on Zephyr, and how nRF9160 modem works.
Looks like TLS storage content is not checked before the negotiation starts, or at least that is what I assume.
But on Nordic modem, what ciphers are offered, seem to depend on the content of the security tag.

Therefore only portable choice seem to be to select the offered cipher-suites by hand.

subsys/net/lib/lwm2m/lwm2m_obj_security.c Outdated Show resolved Hide resolved
subsys/net/lib/lwm2m/lwm2m_obj_security.c Outdated Show resolved Hide resolved
@@ -0,0 +1,42 @@
CONFIG_LWM2M_DTLS_SUPPORT=y
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this overlay also enforce bootstrap? We're not feeding any certificates from the application.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessary.. I was testing this manually by feeding the certificates from command line.

lwm2m write 0/0/3 -s "-----BEGIN CERTIFICATE-----\x0aMIIBazCCAR...
lwm2m write 0/0/5 -s "-----BEGIN EC PRIVATE KEY-----\x0aMHcCAQE...
lwm2m write 0/0/4 -s "-----BEGIN CERTIFICATE-----\x0aMIICBzCCAa...
lwm2m write 0/0/2 -u8 2
lwm2m start secure_client

I prefer to write separate documentation as there are now 3 different security modes supported. But I would like to do that on another commit.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, an update in the documentation would be much appreciated. I'm ok with doing this separately.

@SeppoTakalo SeppoTakalo force-pushed the lwm2m_dtls_x509 branch 2 times, most recently from 0e6f469 to aa4cfdb Compare June 14, 2023 11:10
include/zephyr/net/lwm2m.h Outdated Show resolved Hide resolved
Add support for using X509 certificates.
Default settings use ECDSA certificates with SHA256 hash.

When different settings are required clients should overwrite
struct lwm2m_ctx->load_credentials() and
struct lwm2m_ctx->set_socketoptions()

Signed-off-by: Seppo Takalo <seppo.takalo@nordicsemi.no>
rlubos
rlubos previously approved these changes Jun 14, 2023
@SeppoTakalo
Copy link
Collaborator Author

Added a section into documentation regarding various LwM2M security modes.

rlubos
rlubos previously approved these changes Jun 15, 2023
doc/connectivity/networking/api/lwm2m.rst Outdated Show resolved Hide resolved
doc/connectivity/networking/api/lwm2m.rst Outdated Show resolved Hide resolved
doc/connectivity/networking/api/lwm2m.rst Outdated Show resolved Hide resolved
doc/connectivity/networking/api/lwm2m.rst Outdated Show resolved Hide resolved
doc/connectivity/networking/api/lwm2m.rst Outdated Show resolved Hide resolved
doc/connectivity/networking/api/lwm2m.rst Outdated Show resolved Hide resolved
doc/connectivity/networking/api/lwm2m.rst Outdated Show resolved Hide resolved
doc/connectivity/networking/api/lwm2m.rst Outdated Show resolved Hide resolved
include/zephyr/net/lwm2m.h Outdated Show resolved Hide resolved
include/zephyr/net/lwm2m.h Outdated Show resolved Hide resolved
Add documentation regarding various LwM2M security modes.

Signed-off-by: Seppo Takalo <seppo.takalo@nordicsemi.no>
@SeppoTakalo
Copy link
Collaborator Author

Applied Pekka's documentation suggestions.

SeppoTakalo added a commit to SeppoTakalo/sdk-zephyr that referenced this pull request Jun 21, 2023
Add support for using X509 certificates.
Default settings use ECDSA certificates with SHA256 hash.

When different settings are required clients should overwrite
struct lwm2m_ctx->load_credentials() and
struct lwm2m_ctx->set_socketoptions()

Original PR zephyrproject-rtos/zephyr#59019

Signed-off-by: Seppo Takalo <seppo.takalo@nordicsemi.no>
(cherry picked from commit 60ccb9b9e80cc898beeadb50d39d20459fd564c9)
@fabiobaltieri fabiobaltieri merged commit 55451c6 into zephyrproject-rtos:main Jun 26, 2023
18 checks passed
@SeppoTakalo SeppoTakalo deleted the lwm2m_dtls_x509 branch June 26, 2023 13:53
@SeppoTakalo
Copy link
Collaborator Author

Merging this introduced a CI failure with a test that has been introduced later.
Fix submitted in PR #59762

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants