-
Notifications
You must be signed in to change notification settings - Fork 408
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
Support for communication to the bootstrap server using OSCORE #857
Conversation
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>
To set up a test scenario configure the bootstrap server and client accordingly: Bootstrap server LWM2M Bootstrap Server: Start the client using |
There was a problem hiding this 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[] {}; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 ?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 :)
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
...bsserver-demo/src/main/java/org/eclipse/leshan/server/bootstrap/demo/BootstrapStoreImpl.java
Outdated
Show resolved
Hide resolved
@@ -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()); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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; | ||
} | ||
} |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 ?
There was a problem hiding this comment.
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
leshan-client-core/src/main/java/org/eclipse/leshan/client/servers/ServersInfoExtractor.java
Outdated
Show resolved
Hide resolved
leshan-client-core/src/main/java/org/eclipse/leshan/client/servers/ServersInfoExtractor.java
Outdated
Show resolved
Hide resolved
leshan-bsserver-demo/src/main/resources/webapp/tag/bootstrap-modal.tag
Outdated
Show resolved
Hide resolved
for (var i = 0; i < config.bs.length; i++) { | ||
var bs = config.bs[i]; | ||
newConfig.security[i] = bs.security; | ||
newConfig.oscore[i] = bs.oscore; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
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>
879f978
to
a0040ca
Compare
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>
(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. |
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. |
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. |
Now master is the 2.0.0 version which aims LWM2M 1.1. |
6560752
to
d3215e3
Compare
Signed-off-by: Rikard Höglund <rikard.hoglund@ri.se>
d3215e3
to
85cb80c
Compare
Okay, thanks for the update. I will proceed with fixes and replying to your remaining points until then. |
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>
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.
Yes good questions. I guess in a sense we would be storing OSCORE-related parameters in 3 places:
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:
(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 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:
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 |
There was a problem hiding this comment.
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 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in commit b56a24f
I think we need a kind of plan. 😁
About rebasing |
Signed-off-by: Rikard Höglund <rikard.hoglund@ri.se>
d35f77a
to
b56a24f
Compare
Yes, sounds good.
I believe all the comments have now been addressed.
I will make a post in issue #725 regarding this.
True, having a separate issue to discuss the design of an OSCORE store would be useful.
Yes, good point. Let me think about it and come back in my post in issue #725. |
So I integrated in
Maybe some hints for this discussion. I think maybe we could inspired our-self of DTLS+PSK.
A connection/security context would be automatically created from (oscore/psk)store when needed. |
In this #857 (review), I talked about #437. |
Yes, at some point we should update the code in the oscore branch to fit the style you are doing things in that PR. |
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.