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

Add Certificate usage support to leshan demos #991

Merged
merged 11 commits into from
Apr 6, 2021
Merged

Conversation

sbernard31
Copy link
Contributor

@sbernard31 sbernard31 commented Mar 25, 2021

Modified version of #983

System.err.println(
"You need to set a truststore when you are using \"service_certificate_constraint\" usage");
formatter.printHelp(USAGE, options);
return;
Copy link
Contributor Author

@sbernard31 sbernard31 Mar 25, 2021

Choose a reason for hiding this comment

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

If I'm not wrong only SERVICE_CERTIFICATE_CONSTRAINT need a truststore as other mode can use the server certificate as truststore directly.

(For CA constraint I think it should no be used as truststore too : see #936)

Copy link
Contributor

Choose a reason for hiding this comment

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

Need to have a bit of fresher mind in order to think this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dachaac, the PR is merged but please do not hesitate to give me feedback about this here or at #936.

@sbernard31 sbernard31 force-pushed the certificate_usage branch 2 times, most recently from f7f77a4 to f4e15db Compare March 29, 2021 15:26
@sbernard31
Copy link
Contributor Author

I added change about the UI.

I think the missing part for this feature are :

@dachaac I'm waiting for your feedback about all of those before to go further. :)

@sbernard31
Copy link
Contributor Author

I added certchain support. (#993)

I also limited the certificate usage but only for the config of bootstrap server. (more explanation in commit 2de240b )

@sbernard31
Copy link
Contributor Author

(rebased on master)
(first 2 commits are out of topic)

// and so we can set DTLS Connection as client only by default.
if (serverInfo.bootstrap && incompleteConfig.isClientOnly() == null) {
newBuilder.setClientOnly();
}
Copy link
Contributor

@dachaac dachaac Apr 1, 2021

Choose a reason for hiding this comment

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

I ended up having auto detection from certificate what to configure:
dachaac@fea11c8
as one could either have client certificate with or without serverAuth -- clientAuth should be there always.

and the matching one for server:
dachaac@613b6fb

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This changes is about the library itself and not really related to certificate.

Having LWM2M client and server which could act as both DTLS client and DTLS server is mainly needed when DTLS connection is lost (reboot/crash/connection expired) and server initiated request. (for more details)

In case of bootstrap session we don't need it, because session is short and always initiated by client. So I prefer to set the LWM2M client as DTLS clientOnly for bootstrap.

About "auto detection" this could be a good idea for demo, we add a warning explaining that failover will not work in case we provide a clientAuth or serverAuth only certificate.

@@ -104,7 +104,7 @@ keytool -genkeypair -alias server -keyalg EC -dname 'CN=localhost' \
-storetype $DEFAULT_STORE_TYPE \
-ext BasicConstraints=ca:false \
-ext KeyUsage:critical=digitalSignature,keyAgreement \
-ext ExtendedkeyUsage=serverAuth \
-ext ExtendedkeyUsage=serverAuth,clientAuth \
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a note -- making multi purpose certificates can hide problems from real deployments where one should not have multi purpose certificates. -- in server it is easier but especially in client you do not have necessary information to verify that device is acting as a server. That is the reason why I made them explicit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As explained here, I can not see how we can make "none queue mode" work in reliable way without supporting both role at both side.

So in my mind, "multi purpose" is the default behavior.

But I can imagine that I maybe missed some real life constraint about x509 world.

So could you elaborate about :

"in server it is easier but especially in client you do not have necessary information to verify that device is acting as a server" ?

About :

making multi purpose certificates can hide problems from real deployments where one should not have multi purpose certificates.

A solution could be to add some test with clientOnly ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dachaac, we can continue the discussion at #997 if you want.

@sbernard31
Copy link
Contributor Author

@dachaac, I will integrate this PR without commit.

20d6493 - To not use DTLS ClientOnly mode for integration tests.

I think this is better to have default behavior in tests and also have
generate_credentials.sh as example to generate both role certificates.

Hoping you're fine with the rest of the PR.

sbernard31 and others added 10 commits April 6, 2021 16:36
Signed-off-by: Vesa Jääskeläinen <dachaac@gmail.com>
Signed-off-by: Vesa Jääskeläinen <dachaac@gmail.com>
also-by: Simon Bernard <sbernard@sierrawireless.com>
configuration.

For now use LWM2M default mode domain issuer certificate (3).

Signed-off-by: Vesa Jääskeläinen <dachaac@gmail.com>
Adds Web UI for configuring certificate usage setting for registered
clients.

Signed-off-by: Vesa Jääskeläinen <dachaac@gmail.com>
also-by: Simon Bernard <sbernard@sierrawireless.com>
Signed-off-by: Vesa Jääskeläinen <dachaac@gmail.com>
Add certficate chain support for -cert option.

Signed-off-by: Vesa Jääskeläinen <dachaac@gmail.com>
also-by: Simon Bernard <sbernard@sierrawireless.com>
Add certficate chain support for -cert option.

Signed-off-by: Vesa Jääskeläinen <dachaac@gmail.com>
also-by: Simon Bernard <sbernard@sierrawireless.com>
Because for now this is not possible to change server certificate during
bootstrap session, we do not allow CA constraint and trust anchor
assertion certificate usage. Hoping we will do better with the new UI.
@sbernard31 sbernard31 merged commit 2765896 into master Apr 6, 2021
@sbernard31
Copy link
Contributor Author

@dachaac this is integrated in master now but feel free to give feedback anyway.

@sbernard31 sbernard31 deleted the certificate_usage branch June 3, 2021 07:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants