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

Support client bootstrapping to receive OSCORE security information #950

Conversation

rikard-sics
Copy link
Contributor

This commit enables the client to receive OSCORE security context information from the bootstrap server during the bootstrapping process. The bootstrap server web UI was updated to accept settings for OSCORE under the LWM2M Server tab. Some changes were done in the client side code to support this functionality. Finally, an OSCORE server identity is now generated and set.

Signed-off-by: Rikard Höglund <rikard.hoglund@ri.se>
This commit adds early support for using OSCORE to communicate between
a Leshan client and server. The server can register and perform its
request
towards the server using OSCORE. Likewise the server can perform
requests to
the client using OSCORE.

The web interface of the server has been extended to allow adding
clients that
use OSCORE for security including their OSCORE security context
information.

On the client side further work is needed to allow setting OSCORE
security
context information on it.

Signed-off-by: Rikard Höglund <rikard.hoglund@ri.se>
…eation

Code to implement the points listed here:
eclipse-leshan#718 (comment)

The OSCORE object is generated and filled in the Leshan client demo.
It will then be used to create a ServerInfo object using the
ServersInfoExtractor. Next the ServerInfo is used in the
CaliforniumEndpointsManager to generate endpoints towards servers
that OSCORE should be used with.

See also issue regarding OSCORE client integration:
eclipse-leshan#726

Signed-off-by: Rikard Höglund <rikard.hoglund@ri.se>
Signed-off-by: Rikard Höglund <rikard.hoglund@ri.se>
An identity string based on OSCORE parameters is now set for
connected clients that use OSCORE. The identity string contains
the OSCORE Sender and Recipient ID the client and server are using.
These parameters can be retrieved from the source EndpointContext.

The identity for OSCORE is also set as a SecurityInfo based on the
OSCORE context configured on the server to use with a specific client.

The identity of a connected client and the configured SecurityInfo are
then matched in the SecurityChecker.

Signed-off-by: Rikard Höglund <rikard.hoglund@ri.se>
This commit adds support for the client communicating to
the bootstrap server using OSCORE. The bootstrap server
web UI has been extended to accept settings for OSCORE.

Some modifications were also done on the client to support
it bootstrapping using OSCORE. It supports OSCORE settings
as command line parameters since before.

Signed-off-by: Rikard Höglund <rikard.hoglund@ri.se>
The possibility to input an ID Context for OSCORE on the
client-demo, bsserver-demo and server demo has now been removed.

Signed-off-by: Rikard Höglund <rikard.hoglund@ri.se>
This commit enables the client to receive OSCORE security context
information from the bootstrap server during the bootstrapping
process. The bootstrap server web UI was updated to accept settings
for OSCORE under the LWM2M Server tab. Some changes were done in
the client side code to support this functionality. Finally, an
OSCORE server identity is now generated and set.

Signed-off-by: Rikard Höglund <rikard.hoglund@ri.se>
@rikard-sics
Copy link
Contributor Author

This is a resubmission of the old PR #919.

Setting the server identity for OSCORE should solve the problem recently reported in #725 (comment).

Signed-off-by: Rikard Höglund <rikard.hoglund@ri.se>
@rikard-sics
Copy link
Contributor Author

I have taken into account the feedback from the last PR with regards to excessive trace printing.

Also I just added code from the suggestion regarding retrieval of the OSCORE object from the old PR:
#919 (comment)
I realized I had not added that yet.

@sbernard31
Copy link
Contributor

Also I just added code from the suggestion regarding retrieval of the OSCORE object from the old PR:
#919 (comment)
I realized I had not added that yet.

I written this quickly without testing just to give the idea, so it could contain bugs 😁

Copy link
Contributor

@sbernard31 sbernard31 left a comment

Choose a reason for hiding this comment

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

Testing this I faced this NPE which is not related to this PR :

java.lang.NullPointerException: null
	at org.eclipse.leshan.server.security.SecurityChecker.checkSecurityInfos(SecurityChecker.java:63)
	at org.eclipse.leshan.server.bootstrap.DefaultBootstrapSessionManager.begin(DefaultBootstrapSessionManager.java:71)
	at org.eclipse.leshan.server.bootstrap.DefaultBootstrapHandler.bootstrap(DefaultBootstrapHandler.java:88)
	at org.eclipse.leshan.server.californium.bootstrap.BootstrapResource.handlePOST(BootstrapResource.java:104)
	at org.eclipse.californium.core.CoapResource.handleRequest(CoapResource.java:227)
	at org.eclipse.leshan.core.californium.LwM2mCoapResource.handleRequest(LwM2mCoapResource.java:51)
	at org.eclipse.californium.core.server.ServerMessageDeliverer.deliverRequest(ServerMessageDeliverer.java:106)
	at org.eclipse.californium.core.network.stack.BaseCoapStack$StackTopAdapter.receiveRequest(BaseCoapStack.java:204)

When connecting to a bootstrap server using oscore with an endpoint name which should not use oscore.

@@ -145,6 +179,10 @@ public static BootstrapWriteRequest toWriteRequest(int instanceId, ACLConfig acl
for (Entry<Integer, ACLConfig> acl : bootstrapConfig.acls.entrySet()) {
requests.add(toWriteRequest(acl.getKey(), acl.getValue(), contentFormat));
}
// handle oscore //TODO: Avoid to write also bs oscore object back to client?
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't get this comment ? if you add an oscore object in your Bootstrap Config that means that you want to write the OSCORE object or I missed something ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My point here was that let's say a client communicates to the bootstrap server using OSCORE, and it gets OSCORE configuration to use towards the device manager. In such case as things are now 2 oscore objects will be written to the client. One for the bootstrap server and one for device manager. But the client already has the OSCORE security configuration to use towards the bootstrap server. So I was not sure how this should be handled (perhaps it's fine).

Copy link
Contributor

Choose a reason for hiding this comment

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

Your concern makes sense but this is maybe not the right place for this TODO, because this is more a bsserver-demo question. I will try to explain.

There are 2 main concepts about bootstrap server.

The SecurityStore which contains information about how a device should connect to server (using psk, rpk , x509, or oscore ).

And ConfigStore which contains configuration which should be written/deleted on device during bootstrap session
Since a recent refactoring(#883), there is 2 way to create this kind of config :

  • the old BootstrapConfig more easier to write but less flexible.
  • the new BootstrapConfiguration which is more complex but also more flexible.

BootstrapUtil is just an utility class which help to go from the old way to the new.

All of those ☝️ above is leshan concept and there is no problem here.

the client already has the OSCORE security configuration to use towards the bootstrap server.

This is up to the user to create a config where it delete then rewrite the oscore object or do not change anything.

But for demo we use a "special"(not so good?) SecurityStore which uses the content of ConfigStore to provide securityInfo. (see BootstrapConfigSecurityStore javadocs)

In that case, in you want to allow client to connect using oscore you need a bootstrap config with oscore object and so you will write an oscore object. This is imposible to accept oscore connection and to not write an oscore object. (This is a bsserver-demo limitation only)

This works for other objects (security 0 and server 1) because we delete it before to rewrite it.
(see bsconfigstore.js)

We should probably also delete OSCORE ? (but maybe only when we write oscore object to avoid to disturb client which does not support OSCORE object ?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see, thanks for the explanation. As for deleting OSCORE, would it be good to add that to this PR also?

Copy link
Contributor

Choose a reason for hiding this comment

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

or in another PR as you prefer :)

Copy link
Contributor

Choose a reason for hiding this comment

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

(if you do it in another PR move the TODO)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I'll try to do it here already.

Copy link
Contributor Author

@rikard-sics rikard-sics Jan 20, 2021

Choose a reason for hiding this comment

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

I have now added this functionality in commit 8034e0f

I found it a bit difficult to determine if the OSCORE object is being written, in a compact way. I didn't want to rely on checking hardcoded indexes in it. But perhaps there is a briefer way.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's OK.

Comment on lines 97 to 110
ObjectLink oscoreObjLink = (ObjectLink) security.getResource(SEC_OSCORE_SECURITY_MODE)
.getValue();
if (oscoreObjLink != null) {
int oscoreObjectInstanceId = oscoreObjLink.getObjectInstanceId();
if (!oscoreObjLink.isNullLink() && oscoreObjLink.getObjectId() != OSCORE) {
LOG.warn(
"The security object's 'OSCORE Security Mode' links to an incorrect object type.");
} else {
oscoreInstance = oscores.getInstance(oscoreObjectInstanceId);
if (oscoreInstance == null) {
// maybe we should use same log level for this one and the other just above
LOG.error("Failed to retrieve OSCORE object linked from BS security object");
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should maybe consider oscore null link as not an error ?
And maybe have more help full log ?
Maybe something like this ?

LwM2mObjectInstance oscoreInstance = null;
ObjectLink oscoreObjLink = (ObjectLink) security.getResource(SEC_OSCORE_SECURITY_MODE).getValue();
if (oscoreObjLink != null && !oscoreObjLink.isNullLink()) {
    if (oscoreObjLink.getObjectId() != OSCORE) {
        LOG.warn("Invalid Security info for bootstrap server : 'OSCORE Security Mode' does not link to OSCORE Object but to {} object.", oscoreObjLink.getObjectId());
    } else {
        oscoreInstance = oscores.getInstance(oscoreObjLink.getObjectInstanceId());
        if (oscoreInstance == null) {
            LOG.warn("Invalid Security info for bootstrap server : OSCORE instance {} does not exist.",oscoreObjLink.getObjectInstanceId() );
        }
    }
}

If it sounds good to you, this should also be adapted to Server by adapting the log (talking about LWM2M server instead of bootstrap server and given some information like server id maybe or url to know which server is not correctly configured)

Copy link
Contributor

Choose a reason for hiding this comment

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

About considering oscore null link as "normal", this is currently the default behavior of our current implementation of Security Object. See :
https://github.com/eclipse/leshan/blob/4488653d712bb917da4a6af1f691eb5ce0cd2f69/leshan-client-core/src/main/java/org/eclipse/leshan/client/object/Security.java#L252

Copy link
Contributor

Choose a reason for hiding this comment

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

We should also check is oscores is null

LwM2mObjectInstance oscoreInstance = null;
ObjectLink oscoreObjLink = (ObjectLink) security.getResource(SEC_OSCORE_SECURITY_MODE).getValue();
if (oscoreObjLink != null && !oscoreObjLink.isNullLink()) {
    if (oscoreObjLink.getObjectId() != OSCORE) {
        LOG.warn("Invalid Security info for bootstrap server : 'OSCORE Security Mode' does not link to OSCORE Object but to {} object.", oscoreObjLink.getObjectId());
    } else {
        if (oscores == null) {
              LOG.warn("Invalid Security info for bootstrap server : OSCORE object enabler is not available." );
        } else {
            oscoreInstance = oscores.getInstance(oscoreObjLink.getObjectInstanceId());
            if (oscoreInstance == null) {
                LOG.warn("Invalid Security info for bootstrap server : OSCORE instance {} does not exist.",oscoreObjLink.getObjectInstanceId() );
            }
       }
    }
}

(:warning: I didn't test that code and write log message quickly so maybe there is still some bug and warning message could be better ?)

Copy link
Contributor Author

@rikard-sics rikard-sics Jan 4, 2021

Choose a reason for hiding this comment

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

Thank you for the suggestions regarding this. I will make a commit fairly soon to try and solve this point.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have now made a commit in a811182 to update this. Next week I will be back full time and can take a look at this and the other open points in more detail.


// First find the context for this endpoint
for (Map.Entry<Integer, BootstrapConfig.OscoreObject> oscoreEntry : bsConfig.oscore.entrySet()) {
OscoreObject value = oscoreEntry.getValue();
for (Map.Entry<Integer, BootstrapConfig.ServerSecurity> bsEntry : bsConfig.security.entrySet()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

here it seems you don't use entry.getKey(), so maybe you should use bsConfig.security.values() instead ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True, I believe it was like that since the code below does things in a similar way.

I have now updated to use .values() instead in commit 465f82c

@sbernard31
Copy link
Contributor

@sbernard31
Copy link
Contributor

(Just to let you know, I will be unavailable next 2 weeks)

@rikard-sics
Copy link
Contributor Author

(Just to let you know, I will be unavailable next 2 weeks)

Yes, I understand. I will be much less available in the next 2-3 weeks also.

@rikard-sics
Copy link
Contributor Author

I written this quickly without testing just to give the idea, so it could contain bugs

Sure :) I did try to test it for combinations of OSCORE communication & OSCORE bootstrapping and it seemed solid.

@rikard-sics
Copy link
Contributor Author

Testing this I faced this NPE which is not related to this PR :

Hmm, thanks a lot for pointing this out. I will try to figure out what is happening there.

Signed-off-by: Rikard Höglund <rikard.hoglund@ri.se>
Signed-off-by: Rikard Höglund <rikard.hoglund@ri.se>
@rikard-sics
Copy link
Contributor Author

No linked to this PR too but I think this attribute need some javadoc :
https://github.com/eclipse/leshan/blob/4488653d712bb917da4a6af1f691eb5ce0cd2f69/leshan-server-core/src/main/java/org/eclipse/leshan/server/bootstrap/BootstrapConfig.java#L228

Javadoc for this has now been added in commit 3784bd8

@rikard-sics
Copy link
Contributor Author

Testing this I faced this NPE which is not related to this PR :

java.lang.NullPointerException: null
	at org.eclipse.leshan.server.security.SecurityChecker.checkSecurityInfos(SecurityChecker.java:63)
	at org.eclipse.leshan.server.bootstrap.DefaultBootstrapSessionManager.begin(DefaultBootstrapSessionManager.java:71)
	at org.eclipse.leshan.server.bootstrap.DefaultBootstrapHandler.bootstrap(DefaultBootstrapHandler.java:88)
	at org.eclipse.leshan.server.californium.bootstrap.BootstrapResource.handlePOST(BootstrapResource.java:104)
	at org.eclipse.californium.core.CoapResource.handleRequest(CoapResource.java:227)
	at org.eclipse.leshan.core.californium.LwM2mCoapResource.handleRequest(LwM2mCoapResource.java:51)
	at org.eclipse.californium.core.server.ServerMessageDeliverer.deliverRequest(ServerMessageDeliverer.java:106)
	at org.eclipse.californium.core.network.stack.BaseCoapStack$StackTopAdapter.receiveRequest(BaseCoapStack.java:204)

When connecting to a bootstrap server using oscore with an endpoint name which should not use oscore.

This NPE seems to be fine now, I am not experiencing this.

Signed-off-by: Rikard Höglund <rikard.hoglund@ri.se>
Signed-off-by: Rikard Höglund <rikard.hoglund@ri.se>
@rikard-sics
Copy link
Contributor Author

I think that covers all the open points. Please tell me if you feel something more should be addressed.

@sbernard31
Copy link
Contributor

This NPE seems to be fine now, I am not experiencing this.

I still get the NPE, if I try to connect with a client with oscore setting but didn't add any bootstrapconfig at server side.

@rikard-sics
Copy link
Contributor Author

rikard-sics commented Jan 21, 2021

I still get the NPE, if I try to connect with a client with oscore setting but didn't add any bootstrapconfig at server side.

Thanks! I will check it out further trying to replicate your setup. I was testing with some bootstrap config added.

@rikard-sics
Copy link
Contributor Author

I still get the NPE, if I try to connect with a client with oscore setting but didn't add any bootstrapconfig at server side.

Hmm, maybe I am still testing with different settings but I cannot replicate this.

This is my output on the bootstrap server:

2021-01-25 16:39:25,173 INFO LeshanBootstrapServer - Bootstrap server started at coap://0.0.0.0/0.0.0.0:5683 coaps://0.0.0.0/0.0.0.0:5684
2021-01-25 16:39:25,461 INFO LeshanBootstrapServerDemo - Web server started at http://0.0.0.0:8080/.
2021-01-25 16:40:03,375 ERROR RequestDecryptor - Security context not found
2021-01-25 16:40:03,378 ERROR ObjectSecurityLayer - Error while receiving OSCore request: Security context not found

I tried having a bootstrap server with no configuration at all, and bootstrapping with OSCORE. I also tried having a bootstrap server with a matching endpoint name but which shouldn't use OSCORE, and bootstrapping with OSCORE.

@rikard-sics
Copy link
Contributor Author

I still get the NPE, if I try to connect with a client with oscore setting but didn't add any bootstrapconfig at server side.

Actually testing a bit more I could replicate it. If I start the bootstrap server, add a client with OSCORE configuration and then delete that client again. The problem is that the OSCORE context will live on and be valid even after the client has been deleted in the bootstrap server web interface. Then for the following request from the client with OSCORE a context will be found and it will decrypt correctly.

@sbernard31
Copy link
Contributor

In my case, I think I was using an existing OSCORE context from another config but with an endpoint name without any bootstrap config.

@rikard-sics
Copy link
Contributor Author

In my case, I think I was using an existing OSCORE context from another config but with an endpoint name without any bootstrap config.

I see, yes it seems I can replicate it. I will add a commit to fix this.

@sbernard31
Copy link
Contributor

The issue is in SecurityChecker.checkSecurityInfos. I guess "isOscore" if block must be more like "isSecure" one ?

Signed-off-by: Rikard Höglund <rikard.hoglund@ri.se>
@rikard-sics
Copy link
Contributor Author

The issue is in SecurityChecker.checkSecurityInfos. I guess "isOscore" if block must be more like "isSecure" one ?

Yes, indeed. This should now have been fixed in commit 6f26f46

@sbernard31
Copy link
Contributor

I integrated this in oscore banch (squashed in 1 commit 0d45552)

I did some minor changes :

  • Rewite a little bit BootstrapConfigSecurityStore.getAllByEndpoint() (rename some variable & remove continue)
  • Remove unwanted space in bsconfigstore.js
  • In ServersInfoExtractor.getInfo() add serverURI in logs.
  • In BootstrapUtil remove //TODO: Avoid to write also bs oscore object back to client?

Maybe we should rebase again OSCORE branch on master, I will look if there is not too much conflict.

@rikard-sics
Copy link
Contributor Author

I integrated this in oscore banch (squashed in 1 commit 0d45552)

I did some minor changes :

  • Rewite a little bit BootstrapConfigSecurityStore.getAllByEndpoint() (rename some variable & remove continue)
  • Remove unwanted space in bsconfigstore.js
  • In ServersInfoExtractor.getInfo() add serverURI in logs.
  • In BootstrapUtil remove //TODO: Avoid to write also bs oscore object back to client?

Great, thanks a lot!

Maybe we should rebase again OSCORE branch on master, I will look if there is not too much conflict.

Yes that may be good to do again. Hopefully there are not too many conflicts.

@sbernard31
Copy link
Contributor

I created a oscore2 branch and tested quickly and it seems OK.
But there was several conflict, I hope I didn't add any issue by resolving it.

Maybe you can test/look at it and if you think it's OK I made it the oscore branch ?

@rikard-sics
Copy link
Contributor Author

I created a oscore2 branch and tested quickly and it seems OK.
But there was several conflict, I hope I didn't add any issue by resolving it.

Maybe you can test/look at it and if you think it's OK I made it the oscore branch ?

Thanks. Yes I can have a look and test things through.

@rikard-sics
Copy link
Contributor Author

I created a oscore2 branch and tested quickly and it seems OK.
But there was several conflict, I hope I didn't add any issue by resolving it.

Maybe you can test/look at it and if you think it's OK I made it the oscore branch ?

I went over and tested a number of things including:

  • Registration with OSCORE
    • Then getting a resource on the client from the server web interface
  • Bootstrapping with OSCORE security
  • Bootstrapping for OSCORE material
  • Bootstrapping with OSCORE security & for OSCORE material
  • Other various combinations with DTLS and CoAP

Everything I tested worked so this looks good to me.

@sbernard31
Copy link
Contributor

Ok so now oscore branch is now rebased on master.
Thx @rikard-sics for the contribution 🙏

@sbernard31 sbernard31 closed this Jan 27, 2021
@rikard-sics
Copy link
Contributor Author

Ok so now oscore branch is now rebased on master.
Thx @rikard-sics for the contribution

Thanks for all the assistance!

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