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 for communication to the bootstrap server using OSCORE #857

Conversation

rikard-sics
Copy link
Contributor

@rikard-sics rikard-sics commented Jun 25, 2020

This pull request 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.

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>
@rikard-sics
Copy link
Contributor Author

To set up a test scenario configure the bootstrap server and client accordingly:

Bootstrap server
Client endpoint: Test
LWM2M Server: (All default values)

LWM2M Bootstrap Server:
Security Mode: OSCORE
Master Secret: AAAA
Sender ID: CC
Recipient ID: BB

Start the client using
-b -u 127.0.0.1 -n Test -msec AAAA -sid BB -rid CC

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.

I repeat it each time but it's important to keep it in mind.
OSCORE is not totally clear to me so maybe some of my remarks does not make so much sense.

About OSCORE+Bootstrap server, my guess is that would have been easier to implement with #437, as previously expressed (#718 (comment)) but maybe having a testable bootstrap server quickly is valuable and you can not wait for #437.

Another general point is that resolving questions around OSCORE Store (#766 (comment)):

  • what should be persisted ?
  • what should not ?
  • what should be exposed at LWM2M level ?
  • what could the API look like.
    sounds really important to me because probably really impacting.
    What is you feellng about that ?

Last point, oscore branch is currently based on 2.0 branch. I created this 2.0 branch to let contributors begin to work on LWM2M 1.1 features. But this 2.0 branch is not up to date at all, a lot of change from leshan 1.0 or 1.1 are missing. So maybe we should consider to rebase oscore branch on master one instead (without the senML encoder/decoder). This will probably generate some conflicts but this is something we should do.

public String oscoreAeadAlgorithm = "";
public String oscoreHmacAlgorithm = "";
public byte[] oscoreMasterSalt = new byte[] {};
public byte[] oscoreIdContext = new byte[] {};
Copy link
Contributor

Choose a reason for hiding this comment

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

Looking at LWM2M v1.1.1, I didn't see a field for :

  • object instance id
  • id context

As this java class should represent the OSCORE object, I guess this is maybe a mistake ?

Regarding the specification, types seems to not match too.

Copy link
Contributor Author

@rikard-sics rikard-sics Jul 2, 2020

Choose a reason for hiding this comment

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

Yes, adding the ID Context and having incorrect types were mistakes. For the object instance id I added it to be able to link the OSCORE object to the security object of the bootstrap server. That is what I then check in the ConfigurationChecker:
https://github.com/eclipse/leshan/blob/1893811749c6d1fb07e72dff6cdc29b7330e55ba/leshan-bsserver-demo/src/main/java/org/eclipse/leshan/server/bootstrap/demo/ConfigurationChecker.java#L61

Without that ID I am not sure how I can associate an OSCORE object to the BS security object, and (later) one OSCORE object to the DM security object. Do you have any suggestion regarding this?

Regarding the ID Context, it is my understanding from talking to some people at Ericsson that they wish to add this as an item in the OSCORE object.
Do you think it is a solution for now to keep the ID Context input boxes on the Leshan demo server and Leshan bootstrap server but disabled? Or should I hide them, or delete them completely?

I have now made commit eb1f6ff with the following updates:

  • Disabled the ID Context input in the bootstrap and server demo apps web UI
  • Corrected types for OSCORE object items that were previously byte arrays
  • Made the algorithms in the OSCORE object have the correct type

Copy link
Contributor

Choose a reason for hiding this comment

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

The OSCORE object instance id is the key in the map. In your case : o.getKey()

Regarding the ID Context, it is my understanding from talking to some people at Ericsson that they wish to add this as an item in the OSCORE object

Meaning changing the LWM2M specification v1.1 ? This is something which could be difficult especially when the version is already released.

Do you think it is a solution for now to keep the ID Context input boxes on the Leshan demo server and Leshan bootstrap server but disabled? Or should I hide them, or delete them completely?

I would go for deleting it for now waiting this was clarified ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The OSCORE object instance id is the key in the map. In your case : o.getKey()

I see, thanks. I will work on updating things accordingly.

Regarding the ID Context, it is my understanding from talking to some people at Ericsson that they wish to add this as an item in the OSCORE object

Meaning changing the LWM2M specification v1.1 ? This is something which could be difficult especially when the version is already released.

That would be for version 1.2 or later.

Do you think it is a solution for now to keep the ID Context input boxes on the Leshan demo server and Leshan bootstrap server but disabled? Or should I hide them, or delete them completely?

I would go for deleting it for now waiting this was clarified ?

Yes, it would be best to delete then. Is it fine if I wait and do this in a separate PR? The reason is that there is input for ID Context in client demo, bsserver demo and server demo. So deleting that functionality would touch all of them.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it fine if I wait and do this in a separate PR?

Yep :)

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 to delete the separate object instance id I had in the OSCORE object. See commit 85cb80c
So now I assign 0 to the oscoreSecurityMode in the security object for the Bootstrap Server and o.getKey() gives me object instance id 0 for that OSCORE object.

Then I guess 1 can be assigned to the oscoreSecurityMode in the security object for the Device Manager and o.getKey() for this second OSCORE object will be 1. As far as I understand this ID is what is assigned in bsconfigstore.js when I do newConfig.oscore[i] = bs.oscore; so newConfig.oscore[i+j] = dm.oscore; would give me id 1.

Yep :)

Okay, sounds good.


OSCoreCtx ctx = new OSCoreCtx(osc.oscoreMasterSecret, false, aeadAlg, osc.oscoreSenderId,
osc.oscoreRecipientId, hkdfAlg, 32, osc.oscoreMasterSalt, osc.oscoreIdContext);
db.addContext(ctx);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm still disturb by this static kind of OSCORE store my guess maybe this should be the priority to get a rid of this because this will really impact the way we will organize the code.
Here our previous conversation about this : #766 (comment)

The way this is currently coded does not work if we restart the server as (db.addContext is not called) and so you get :

2020-06-30 16:16:04,071 ERROR RequestDecryptor - Security context not found
2020-06-30 16:16:04,071 ERROR ObjectSecurityLayer - Error while receiving OSCore request: Security context not found

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes indeed. An OSCORE context should not only be added the moment it is input via the bootstrap server UI but also when the server starts so this issue does not happen.

@@ -161,6 +164,10 @@ public static void main(String[] args) {
public static void createAndStartServer(String webAddress, int webPort, String localAddress, int localPort,
String secureLocalAddress, int secureLocalPort, String modelsFolderPath, String configFilename)
throws Exception {

// Enable OSCORE stack (fine to do even when using DTLS or only CoAP)
OSCoreCoapStackFactory.useAsDefault(OscoreHandler.getContextDB());
Copy link
Contributor

Choose a reason for hiding this comment

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

Not available in 2.0 but probably a proper way will be to change the Leshan DefaultEndpointFactory to use OSCoreCoapStackFactory() only for specific endpoint instead of changing default for all.

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, only endpoints that will actually use OSCORE need this. So when a particular endpoint is created it would need to be known if it will be using OSCORE.

But yeah there is no need to do this for all endpoints. For the bootstrap server for instance it would only need OSCORE on its insecure endpoint that is listening for messages from incoming clients. (At least for now when we don't have DTLS+OSCORE.)

And updating the Californium version used would be a good step to make it possible to do this. I guess that will come with the rebase to master.

usingOscore = true;
break;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

My guess is : if oscore instance is not found configuration is incorrect and exception should be raised.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If bootstrapping is not using OSCORE, there will be no OSCORE object right? I am not sure how I can add more checks here that work generally for both OSCORE and non-OSCORE cases.

Copy link
Contributor

Choose a reason for hiding this comment

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

My point is if sec.oscoreSecurityMode is not null and we can not find a corresponding ocore config, we should raise an error. I mean configuration seems incorrect.

or did 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.

Ok, I understand. I have simplified the OSCORE object retrieval a bit now and also throw an exception if there is no corresponding OSCORE object to what is indicated in the bs security object oscoreSecurityMode. This was done in commit b56a24f

for (var i = 0; i < config.bs.length; i++) {
var bs = config.bs[i];
newConfig.security[i] = bs.security;
newConfig.oscore[i] = bs.oscore;
Copy link
Contributor

Choose a reason for hiding this comment

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

convertConfig should probably be adapted too.

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 will proceed with that.

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 has now been done in commit 5b9392c

@rikard-sics
Copy link
Contributor Author

Thanks for the feedback. I will begin replying to your individual points and updating the code.

Signed-off-by: Rikard Höglund <rikard.hoglund@ri.se>
@rikard-sics rikard-sics force-pushed the oscore_comm_bootstrapping_WIP branch from 879f978 to a0040ca Compare July 1, 2020 14:42
Signed-off-by: Rikard Höglund <rikard.hoglund@ri.se>
Signed-off-by: Rikard Höglund <rikard.hoglund@ri.se>
Signed-off-by: Rikard Höglund <rikard.hoglund@ri.se>
Signed-off-by: Rikard Höglund <rikard.hoglund@ri.se>
Signed-off-by: Rikard Höglund <rikard.hoglund@ri.se>
Signed-off-by: Rikard Höglund <rikard.hoglund@ri.se>
The following changes were performed:
Disabled the ID Context input on bootstrap and server demo apps
Corrected types for OSCORE object items that were previously byte arrays
Made the algorithms in the OSCORE object have the correct type

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

(just to let you know that I will not be able to give you feedback this week, I'm back next week)

@rikard-sics
Copy link
Contributor Author

(just to let you know that I will not be able to give you feedback this week, I'm back next week)

Okay I understand, thanks for letting me know.

@rikard-sics
Copy link
Contributor Author

Last point, oscore branch is currently based on 2.0 branch. I created this 2.0 branch to let contributors begin to work on LWM2M 1.1 features. But this 2.0 branch is not up to date at all, a lot of change from leshan 1.0 or 1.1 are missing. So maybe we should consider to rebase oscore branch on master one instead (without the senML encoder/decoder). This will probably generate some conflicts but this is something we should do.

Yeah it would make sense to rebase the oscore branch on master. I could assist or give that a try as a future PR after this.

I will come back to the other points in your post.

@sbernard31
Copy link
Contributor

Yeah it would make sense to rebase the oscore branch on master. I could assist or give that a try as a future PR after this.

I will try to release the 1.1 soon and I will create the official new 2.0.x/master branch, then we could rebase your work on this one. I will let you know when this will be ready.

@sbernard31
Copy link
Contributor

Now master is the 2.0.0 version which aims LWM2M 1.1.
(By the way, I'm back on Wednesday 15th)

@rikard-sics rikard-sics force-pushed the oscore_comm_bootstrapping_WIP branch from 6560752 to d3215e3 Compare July 9, 2020 19:53
Signed-off-by: Rikard Höglund <rikard.hoglund@ri.se>
@rikard-sics rikard-sics force-pushed the oscore_comm_bootstrapping_WIP branch from d3215e3 to 85cb80c Compare July 9, 2020 19:55
@rikard-sics
Copy link
Contributor Author

Now master is the 2.0.0 version which aims LWM2M 1.1.
(By the way, I'm back on Wednesday 15th)

Okay, thanks for the update. I will proceed with fixes and replying to your remaining points until then.

@rikard-sics
Copy link
Contributor Author

Now master is the 2.0.0 version which aims LWM2M 1.1.

So the plan would be to get this oscore branch mature enough and then merge it in to the master branch?

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

rikard-sics commented Jul 13, 2020

About OSCORE+Bootstrap server, my guess is that would have been easier to implement with #437, as previously expressed (#718 (comment)) but maybe having a testable bootstrap server quickly is valuable and you can not wait for #437.

Yes I would say it's important to have testable support for bootstrapping with OSCORE sooner rather than later. Certainly in the future if 718 is implemented I could adapt the functionality in this code to take advantage of it.

Another general point is that resolving questions around OSCORE Store (#766 (comment)):

  • what could the API look like.
    sounds really important to me because probably really impacting.
    What is you feellng about that ?

Yes good questions. I guess in a sense we would be storing OSCORE-related parameters in 3 places:

  1. The OSCORE context database
  2. The current configuration of the bootstrap or management server (although some is not persisted now)
    • security.data & bootstrap.json
  3. The OSCORE store

Perhaps if the OSCORE store could be the only one having rights to add or delete OSCORE contexts? So the configuration data would be sent to the OSCORE store which then creates OSCORE contexts and adds them to the context database?

In such case the flow above would be 2->3->1. So the context database is kind of isolated. But on the other hand the context database would be needed when creating endpoints.

Also since things are either stored in the OSCORE context database (for things currently in memory) or in the configuration files (or in data structures already being built from the configuration files), storing things in the OSCORE store also would be duplicating that data. But perhaps that would be needed although all OSCORE-related data is already available.

One idea for a flow could be:

  1. The bootstrap or management server starts reading their configuration files for client info
  2. The OSCORE store is filled with the data read in 1 and creates appropriate OSCORE contexts which it adds to the context database. (Perhaps an issue would be needing to pre-create a lot of contexts before clients have connected. But if not done the incoming OSCORE message from a client will fail to process in the OSCORE layer.)
  3. A client connects using OSCORE and as its OSCORE context is already added to the database things work.
  4. A new client is added in the UI of the server. Now the configuration files will be updated and the OSCORE will also receive this new data. If needed it creates new contexts.

(For the client point 1 above would be replaced by reading the config from the command line parameters.)

One main challenge is see is what I wrote earlier about storing duplicated data. Since the OSCORE context database, the configuration files and the OSCORE store would all need to be kept in sync (at least for the persistent data).

  • what should be persisted ?
  • what should not ?
  • what should be exposed at LWM2M level ?

What should be persisted could in my opinion be only what is defined in the OSCORE object* (already being stored into the configuration files). Of course during execution the OSCORE context database will keep track of individual contexts with their sequence numbers and other mutable data.

Thinking about what needs to be exposed at LWM2M level. I can think of:

  • Sender ID
  • Recipient ID
  • The context database used for a client (to create endpoints for it)
  • Functionality to be able to know what client endpoint name is associated to a particular context (although not an OSCORE concept)

The actual security in terms of cryptographic operations, OSCORE will take care of. LWM2M only needs to match that a particular incoming client is using its intended context, while OSCORE already checked that it is using a cryptographically valid context.

*If only what is in the OSCORE object is persisted and the server reboots the clients would need to perform the procedure defined in Appendix B.2 of the OSCORE RFC to rederive their contexts (to avoid using the same key with the same partial IV) since the server will start over from partial IV (sender sequence number 0). This is supported in the OSCORE implementation in Californium. If more is persisted like also the sender sequence number this procedure could be avoided. But AFAIK this procedure in Appendix B.2 is mandatory to implement for LWM2M for the reason of only needing to persist the things in the OSCORE object. (Btw. the client also needs to do this procedure if it reboots/restarts itself).

But yeah, this is an important discussion and will impact the overall design.

@@ -147,7 +146,7 @@
smsBindingKeySecret : [ ],
smsSecurityMode : "NO_SEC",
uri : bsserver.uri,
oscoreSecurityMode : 111 // link to oscore object
oscoreSecurityMode : 0 // link to bs oscore object
Copy link
Contributor

Choose a reason for hiding this comment

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

This value should only be set if oscore is used ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated in commit b56a24f

@sbernard31
Copy link
Contributor

I think we need a kind of plan. 😁

  • Once you fix the last little comment on this PR, I think I can squash/integrate this PR in oscore branch.
  • Then we need to identify the last missing part to achieve a minimal viable feature. (you can list the missing part or we can discuss about that OSCORE support in Leshan #725)
  • Probably defining the OSCORE Store is one of the main missing part ? (We should use a dedicated issue to talk about that.)

About rebasing oscore branch on master we can do that just after we integrate this PR or once we get the minimal viable feature ready. (as you prefer)

Signed-off-by: Rikard Höglund <rikard.hoglund@ri.se>
@rikard-sics rikard-sics force-pushed the oscore_comm_bootstrapping_WIP branch from d35f77a to b56a24f Compare July 16, 2020 11:22
@rikard-sics
Copy link
Contributor Author

rikard-sics commented Jul 16, 2020

I think we need a kind of plan.

Yes, sounds good.

  • Once you fix the last little comment on this PR, I think I can squash/integrate this PR in oscore branch.

I believe all the comments have now been addressed.

  • Then we need to identify the last missing part to achieve a minimal viable feature. (you can list the missing part or we can discuss about that OSCORE support in Leshan #725)

I will make a post in issue #725 regarding this.

  • Probably defining the OSCORE Store is one of the main missing part ? (We should use a dedicated issue to talk about that.)

True, having a separate issue to discuss the design of an OSCORE store would be useful.

About rebasing oscore branch on master we can do that just after we integrate this PR or once we get the minimal viable feature ready. (as you prefer)

Yes, good point. Let me think about it and come back in my post in issue #725.

@sbernard31
Copy link
Contributor

I believe all the comments have now been addressed.

So I integrated in oscore branch (commit 736b8c3).

True, having a separate issue to discuss the design of an OSCORE store would be useful.

Maybe some hints for this discussion. I think maybe we could inspired our-self of DTLS+PSK.

OSCORE DTLS/PSK State Usage
OscoreStore PSKStore Persistent, shared between instance in cluster environment store data need to establish a connection/securityContext
OscoreContext ConnectioStore In memory, not shared, not persistent store the connection/security context established

A connection/security context would be automatically created from (oscore/psk)store when needed.

@sbernard31
Copy link
Contributor

In this #857 (review), I talked about #437.
Just to let you know I begin to experiment this in #883. This could impact some part of this PR, so maybe you could be interested.

@rikard-sics
Copy link
Contributor Author

In this #857 (review), I talked about #437.
Just to let you know I begin to experiment this in #883. This could impact some part of this PR, so maybe you could be interested.

Yes, at some point we should update the code in the oscore branch to fit the style you are doing things in that PR.

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