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 a credentials store #536

Closed
wants to merge 4 commits into from
Closed

Add a credentials store #536

wants to merge 4 commits into from

Conversation

sbernard31
Copy link
Contributor

Add a credentials store to be able to change credentials depending on foreign peer.

This PR is not really polished, this is version with minimal changing.
This aims to start more technical discussion about it.

@boaks
Copy link
Contributor

boaks commented Feb 5, 2018

FMPOV, the leak issue in #429 consumes all my time I can invest in californium for release 2.0.0.
So I'm not able to work on this, sorry.

@sbernard31
Copy link
Contributor Author

I understand. Does it means that I should not even rely on a review from you ?

@boaks
Copy link
Contributor

boaks commented Feb 6, 2018

Yes, in fact, the plan was to release 2.0.0 next week.
So this will go to 2.1. and will be discussed after 2.0.0.

@rkimsb2
Copy link
Contributor

rkimsb2 commented Apr 27, 2018

+1

@sbernard31
Copy link
Contributor Author

I rebased it on 2.0.x, I will start an experimental branch in leshan to test if this makes the job.

Achim Kraus and others added 2 commits July 13, 2018 16:46
Signed-off-by: Achim Kraus <achim.kraus@bosch-si.com>
also-by: Simon Bernard <sbernard@sierrawireless.com>
@sbernard31
Copy link
Contributor Author

I added support of RPK to Leshan client thx to this PR
See eclipse-leshan/leshan#538 as example of usage.

My first feeling is pretty good.

@boaks, I would really appreciate if we could consider to integrate this for the 2.0.0 release. (Not for the 2.0.0-M11 of course)

Next step support of x509.

@boaks
Copy link
Contributor

boaks commented Jul 17, 2018

FMPOV, the reuse of a client-side-connector for multiple destinations, comes with the risk, that it fails, if the servers are potentially co-locate on the same server-side-endpoint. This PR would not change the possibility to do so, it makes it just easier to do and so more probable.

Maybe server-co-location is not too much used, but, on the other side, I'm not sure, how many different destinations will be used by one client. If it's only a small number, then there are also alternatives.
One alternative would be consider to optimize the Connectors to be used as "Set". This would eliminate that nasty "co-location" pitfall.

In a first step, I have reused the DTLS timer to be also the executor, if only 1 thread is used (I will update PR #699 tomorrow). This will lower the resources per DTLS connector.
A second step would be, to prepare the connectors to share executors (may be just for sending, or, using nio/async io, also for receiving).

@sbernard31
Copy link
Contributor Author

I'm sorry :-/ but I can not understand the server-co-location issue ? are you talking about virtualhost(SNI) ?

@boaks
Copy link
Contributor

boaks commented Jul 17, 2018

I'm sorry :-/ but I can not understand the server-co-location issue ? are you talking about virtualhost(SNI) ?

Yes, "connecting" to two servers but ends up at the same server-endpoint. Basically this may lead to different issues before, and would fail even earlier. But if it somehow succeeds, then you will not be able to decrypt the messages, if your connected to that endpoint twice.

@sbernard31
Copy link
Contributor Author

If this is the only issue, we could start with a first version of CredentialsStore which is not compatible with SNI ?
(I can add a comment and/or raise an exception if a custom CredentialsStore is used and SNI is enabled in the DtlsConnectorConfig builder)

We will be able to add SNI support later, like we did for PSKStore.

@boaks
Copy link
Contributor

boaks commented Jul 17, 2018

If this is the only issue, we could start with a first version of CredentialsStore which is not compatible with SNI ?

That's a misunderstanding! If that "virtual"/"co-located" stuff is intended to be supported, then this PR will bring not too much benefits at all, but may aggravate the pitfalls.

Maybe server-co-location is not too much used, but, on the other side, I'm not sure, how many different destinations will be used by one client. If it's only a small number, then there are also alternatives.

So, FMPOV, the question is: how many destinations are intended for one client? I think this number is low enough, to go for the save variant of using different client-endpoints for different destinations. And so I would consider to optimize the use of a connectors instead.
This may also be more useful for eclipse-leshan/leshan#491

@boaks
Copy link
Contributor

boaks commented Jul 17, 2018

We will be able to add SNI support later, like we did for PSKStore.

Just to mention, the PSKStore only solves the "realm"!
But if a coap-client uses SNI to connect to two (virtual) servers, which are actually on the same coap-server-endpoint, then this will fail ;-(.

@sbernard31
Copy link
Contributor Author

Could you explain which problems this will bring exactly ?
Currently, I only see the benefits which is that this simplify a lot the implementation of RPK and x509 support for Leshan client.

@boaks
Copy link
Contributor

boaks commented Jul 17, 2018

client c-ep1 ---- handshake --> server1 (actually s-ep-common), establish session-1
client c-ep1 ---- handshake --> server2 (also actually s-ep-common), establish session-2

client c-ep1 ---- request --> server 1 (s-ep-common)
client c-ep1 <-- response -- server 1 (s-ep-common)

now the client doesn't know, which session it has to use to decrypt the response. its identified by the "s-ep-common" but this is the same as for server-2 / session-2.

With TLS, this works, because the second handshake will use it's own connection.
Therefore, if DTLS is intended to be used with virtual server, the client need a own endpoint / connector to communicate with servers.

@sbernard31
Copy link
Contributor Author

So this is the case where, we are using SNI and, the client tries to talk to 2 different servers behind the same IP/port.
(Still in draft but connection ID should help)

I don't know if this is a common use case.
Do you have this kind of use case on your side ?

So, I think this still makes sense to provide an API like this even if this does not support this very particular use case. This API could also be used to change credentials dynamically without restarted endpoint.

By the way, I already tried to implement the "1 endpoint by server" solution a long time ago (eclipse-leshan/leshan#450) and you oriented me towards this kind of solution (eclipse-leshan/leshan#450 (comment)) and now I find this very
elegant. :-)

@boaks
Copy link
Contributor

boaks commented Jul 17, 2018

By the way, I already tried to implement the "1 endpoint by server" solution a long time ago (eclipse-leshan/leshan#450) and you oriented me towards this kind of solution (eclipse-leshan/leshan#450 (comment)) and now I find this very
elegant. :-)

Investigating in issue #677 I got aware, that the "virtual" stuff requires different connections, for both
DTLS (SNI) and in CoAP (URI-HOST, see https://tools.ietf.org/html/rfc7252#section-5.10.1). You can't communicate from one client endpoint to two server endpoint, which are virtual, but co-located on the same host. The client would assume communication artefacts (MID or MAC), which are unique per endpoint and so co-locating the physical-endpoint of virtual-endpoints will fail. SNI or URI-HOST are just helpers to overcome the pain caused by virtual hosts. The real issue would be the use of different DNS entries pointing to the same host. So If you can't ensure, that this DNS stuff is not done, your communication may fail. And therefore, since one or two weeks, I started to think, that on the client-side we need different connectors as different endpoints for different destinations.

(Still in draft but connection ID should help)

It will not help for DTLS 1.2, because the server side will not use a cid (page 7, figure 4).
So the client can't use a cid to distinguish both sessions.

I don't know if this is a common use case.
Do you have this kind of use case on your side ?

No, but though the "virtual" stuff is included in DTLS (SNI) and in CoAP (URI-HOST), and the real pain is caused by DNS entries to the same host, I think this requires either an explicit exclusion or the "multi connector" approach.

But you ask questions without answering my question:
How many destinations are intended for a client?

I still guess, this are only a few connections.

And so my conclusion is:

Optimize the use of multiple connectors instead!
In a first step, I have reused the DTLS timer to be also the executor, if only 1 thread is used (I will update PR #699 tomorrow). This will lower the resources per DTLS connector.
A second step would be, to prepare the connectors to share executors (may be just for sending, or, using nio/async io, also for receiving).
And this may also be more useful for eclipse-leshan/leshan#491

@sbernard31
Copy link
Contributor Author

But you ask questions without answering my question:
How many destinations are intended for a client?

Minimum 1 device management server and probably 1 bootstrap server, I think in real world not so much more (even if the lwm2m spec allow up to 65535 server for 1 client)

Optimize the use of multiple connectors instead!

I'm not really interested in optimizing resource for now but mainly by implementing RPK, x509 at lwm2m client side (with bootstrap of course). This PR helps to do that in an elegant way with the only pitfall that it does not work for co-located server which sounds to be a really particular use case.

In a first step, I have reused the DTLS timer to be also the executor, if only 1 thread is used (I will update PR #699 tomorrow). This will lower the resources per DTLS connector.
A second step would be, to prepare the connectors to share executors (may be just for sending, or, using nio/async io, also for receiving).
And this may also be more useful for eclipse-leshan/leshan#491

This PR does not prevent all of this.

@boaks
Copy link
Contributor

boaks commented Jul 17, 2018

This PR does not prevent all of this.

Sure, but as I already wrote above:
If that "virtual"/"co-located" stuff is intended to be supported, then this PR will bring not too much benefits at all, but may aggravate the pitfalls.

@rkimsb2
Copy link
Contributor

rkimsb2 commented Jul 18, 2018

Could I join this issue?

As the disscusion above all, also I didn't find any clues for recognizing server name on exchanging message after handshaking.
But then, Does SNI extension has meaning for DTLS?

First of all, with TLS, this will be work as like @boaks mentioned.
But DTLS doesn't working because of the problem as mentioned above.
It is wierd same extension applies as different form.

Second of all, also I didn't find any clues that the port(endpoint) should be separated when DTLS with SNI extension used.

IMHO, the DTLS spec doesn't explain the SNI extension enoughly.
Could we ask to IETF about this issue?

Or is there any clues i didn't found?

@boaks
Copy link
Contributor

boaks commented Jul 18, 2018

Could I join this issue?

Sure, your welcome :-).

But then, Does SNI extension has meaning for DTLS?

That's also, what I ask myself. AFAIK the intention to support SNI was to make the (PSK) credentials tenant aware. But with #677 and URI-HOST option, which also targets the "virtual server", I'm not longer sure, if this limitation to tenants is really good.

But DTLS doesn't working because of the problem as mentioned above.

The point is: it works, if the client uses different endpoints for different destinations!
And that's true for both DTLS and CoAP, the client must use different endpoints!
And therefore, just since one or two weeks, I changed my own view of using CoAP and DTLS.
And that's now the main topic in this discussion, because @sbernard31 invested a lot of time,
but for me, in the meantime, this goes into a wrong direction with pitfalls, if such "virtual" stuff is then used. And as I wrote above, FMPOV SNI and URI-HOST are just helpers to use "virtual host", so what must be clear, if a client should use only one endpoint for different destinations, is that "virtual hosts" are not used at all!

IMHO, the DTLS spec doesn't explain the SNI extension enoughly.
Could we ask to IETF about this issue?

Sure, but sometimes it takes longer to get answers. :-).
So, do you want to write to the IETF core-mailing-list?

And what's you impression, go for these changes here in this PR and risk, that it fails, if "virtual host" are getting used?
Or work on improving to use sets of connectors more efficiently?
Or try to wait for an answer?

@rkimsb2
Copy link
Contributor

rkimsb2 commented Jul 18, 2018

What I wanted to talk about is that if SNI extension is for both of TLS and DTLS, it also should work as same way on both of it. But actually it does not. On TLS, it can used by single endpoint with multiple virtual server. And on DTLS, it must be separated to different endpoint. This is what I feel weird and want to check to IETF. Also because of it's undefinition of DTLS's RFC docs.

@rkimsb2
Copy link
Contributor

rkimsb2 commented Jul 18, 2018

Am I have a wrong point of view?
is SNI actually work as same way on TLS and DTLS?

@boaks
Copy link
Contributor

boaks commented Jul 18, 2018

On TLS, it can used by single endpoint with multiple virtual server.

No, that's a misunderstanding!
If you connect to multiple servers using TLS, you need multiple TLS connections!
That includes, that the client implicitly create a new endpoint for each connection (endpoint := ip-address+port). The californium TlsClientConnector hides this, but in fact, it alos opens a TLS connection for each destination and therefore creates a new endpoint.

@boaks
Copy link
Contributor

boaks commented Jul 18, 2018

Am I have a wrong point of view?
is SNI actually work as same way on TLS and DTLS?

FMPOV, it works in the same way, but requires the client to use explicit different endpoints, what is implicit done in the same way for TLS.

@rkimsb2
Copy link
Contributor

rkimsb2 commented Jul 18, 2018

Thank you @boaks. Now I got the point.

@boaks
Copy link
Contributor

boaks commented Jul 18, 2018

OK, so what should now be the next steps?

Write to the IETF core list?
Or do we agree, that id "virtual hosts" are used, coap clients must use explicit different endpoint?

Should this PR be reviewed anyway? It doesn't introduce this issue nor does it prevent a solution. But it may aggravate this misuse. So add clear warnings will be one action, which would be required for this PR.

@rkimsb2
Copy link
Contributor

rkimsb2 commented Jul 18, 2018

Write to the IETF core list?

No, I think this is so far from focus for now.

Or do we agree, that id "virtual hosts" are used, coap clients must use explicit different endpoint?

Yes. I think the first concept(multiple server connections for single endpoint) was wrong point.

Should this PR be reviewed anyway?

I think it's No.

And then, what my first focus about this issue was reusing endpoint when the connection has changed to another server.
But now it seems that this work should be done by shutdown old endpoint and create new one, including security policy.
These works are all out of boundary of endpoint's works.

@sbernard31
Copy link
Contributor Author

So what must be clear, if a client should use only one endpoint for different destinations, is that "virtual hosts" are not used at all !

This is not exactly true.
Using only one endpoint for different destinations can work with virtual host.
The limitation is: the client must not talk to several servers which are behind the same "real host".

e.g.

          ------> virtual host 1
         / 
Host 1 
         \
          ------> virtual host 2

          ------> virtual host 3
         / 
Host 2 
         \
          ------> virtual host 4

With this configuration a client which are using only one endpoint can talk to "virtual host 2" and "virtual host 3" without any issue using SNI. But it can not talk with "virtual host 1" and "virtual host 2".
So for example the multi-tenant architecture could work if server does not share clients.

But it may aggravate this misuse

This seems to be the only argument against this PR, and it seems to be very light to me.
Because this is not necessarily a misuse. It could just be a need which is not affected by any pitfalls because the co-located server use-cases is not needed at all.
And again this PR does not prevent to use 1 endpoint by server if needed.

For a lwm2m client (that's my use cases currently), should we support the co-located server ?
Personally, I don't know.
No one here seems to really need this. (@rkimsb2 maybe you can explain us how you are using Leshan and californium ?)
On my side, I talked about that with my workmates at Sierra to get their feeling and this seems really away from what they see and need in the field. (no colocated-server on the horizon)

Generally my feeling is to go with the simple/elegant way which solves the need we have at short or midterm and do not make plans of what would be the needs in a far future. So as this solution is more elegant/simple and is adapted to current use cases, I would rather go with it.

But as both of you seems to think that this is not the right way, and as I'm maybe biased by sunk-cost fallacy, I will consider to go back 6 or 7 month in the past and rework on a 1 endpoint by server solution ...
sad

@boaks please next time, do not hesitate to share your thought as soon as possible, this time 1 weeks before could make me save several days of work.

If any of you change your mind again to not hesitate to share it here. I would be glad to re-switch on 1 endpoint for multiple server. There is really no irony here.

@boaks
Copy link
Contributor

boaks commented Jul 18, 2018

The limitation is: the client must not talk to several servers which are behind the same "real host".

Exactly.

do not hesitate to share your thought as soon as possible

My point here is, that "share" mostly mean "time consuming discussions", and, though the hono-coap-adapter is behind my timelines, I had to focus on that work.

By the way, if your workmates think it's a good solution, I have no general objection,
I just think, it doesn't really pay off. And as I wrote, as long as a warning comment is added to the stuff in californium, you may use it for the leshan client, if you think it's a good idea.

@rkimsb2
Copy link
Contributor

rkimsb2 commented Jul 19, 2018

To @sbernard31

maybe you can explain us how you are using Leshan and californium ?)

Actually, I made some private branch of old version leshan(2.0.0-M3). In case of client, i have to make it as possible to connect multiple server, and modified it myself.

If this issue is closed in any of way, and multiple registration is possible, I think I can move to latest version.

Currently, my private branch shutdown endpoint every time for updating security policy.
This is my current revision of my usage.

@sbernard31
Copy link
Contributor Author

I revive the 1 endpoint by server solution for Leshan, so I think we can close this PR.

@sbernard31 sbernard31 closed this Aug 17, 2018
@sbernard31 sbernard31 deleted the credential_store branch August 17, 2018 09: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.

3 participants