-
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
Certificate usage support #896
Conversation
Note: I am aware that there are certificate usage test cases missing that should be there. Will be added. But lets get the review otherwise on going not to delay it. |
I will check, if the tests cover my experienced issues developing the |
The tests in Maybe, I have overseen some tests somewhere else. If so, please point me to. |
@boaks as mentioned the test cases are coming soon :) |
cbcc6e6
to
e9d2819
Compare
|
Uups, my bad ;-(. |
@boaks sorry if there was confusion about it. I have now written some more tests but I faced a bit weird DTLS handshake problem where it gets stuck. You might have the expertise to know where the problem is. I didn't yet spot anything that things would be forbidden by my certificate usage rules or so -- kinda feels some unrelated thing just that happens to be triggered. I didn't push them yet in this branch but they are on top of my est-support branch: In I added comment about the problem:
Another problem seems to be what can be debugged with same test case is that if you have breakpoint in CaliforniumEndpointsManager :: verifyCertificate and observe:
|
I am considering dropping server side SNI enablements as that causes empty name to be sent from server. Fixing that to have some name seems to be a bit larger job and I don't think it really brings so much gains. Client side I believe is correct and should be there in case client is connecting to server that has a need for SNI. |
Problem is in Leshan side -- exactly on Lwm2mEndpointContextMatcher.matchPrincipals(). I added debug: protected boolean matchPrincipals(Principal requestedPrincipal, Principal availablePrincipal) {
System.out.println(this.getClass().toString() + ".matchPrincipals() req=" + requestedPrincipal + ", avail=" + availablePrincipal);
System.out.flush(); and I do get frollowing output:
And based on that implementation there -- there is no match and thus nothing is being sent. requestedPrincipal value here is what was in serverPublicKey field encoded (and in this test case it is intermediate CA) -- kinda correct but kinda wrong in this spot. And available was received from server which is of course correct and in that CertPath one can see also intermediate. Now the question here is how should this be handled in here correctly. Should we have custom matcher that also has certificate usage policy attached. In theory one probably should match the DTLS connection session here and make lookup based on that one? But then again if some other server from that CA would be sending with same port/address match would be invalid. One possibility could be that once we have done the certificate usage policy part and then we construct CertPath again and then we attach that for particular connection so then end certificate match could be done. @boaks + @sbernard31 How do you think this should be working? |
I guess leshan uses parts (DN) of the "server's certificate" from the security object as |
Missing root CA in CertPath I could augment by filling missing CA's from trust store by changing it to use AdvancedCertificateVerifier. I am wondering if the right place for that fix would actually be CertPathUtil.validateCertificatePath(). Moved the code to class DefaultLeshanCertificateVerified so that it can have a bit of state too. If I return true on there my tests passes but I am not sure if that is the correct thing to do. As result there is now one failing test case: Updated est-support branch with that test code. I also added some extra debugs in californium to debug it a bit more but I suppose those are not needed now that point is kinda identified. Problem seems to be related to fact that when message is being prepared for sending we do not yet know anything about peer (eg. the server) thus only information there is intermediate CA. I believe this is kinda a wrong implementation that is in place as there is already related connection that actually knows the peer identity but then again I might be missing some point. |
The |
About, the matching principal issue : Leshan client use This commonName is extracted from Security Object (0). (see CaliforniumEndpointsManager.createEndpoint(ServerInfo)) The question is : what currently identified the server ? Maybe the URL which should available in Security Object too and which should be retrieve in available principal too. (SNI is also a potential candidate when it used.) About the PR itself:
And of course there is this critical issue about certificate chain raised here : OpenMobileAlliance/OMA_LwM2M_for_Developers#502 |
About Matching the principal. I may be missing something here so feel free to correct me. Now in client side this is rather simple in a way. It has know direction where to connect. It knows target address and port and can match the communication based on that one. This seems to be happening already like this and I believe Californium is doing this. In server side when client is connecting there we kinda get DTLS session done first and we know the identity of the client right a way. And then we can continue matching the address + port and then lookup which connection it is. I believe this is already done in Californium. If you have NAT in between in theory if the client opens the connection then connection tracking is happening in NAT service and in theory server nor client need to know about it. During handshaking phase client wants to verify that server is what it is expected to be. And server wants to verify that connecting client identity is verified and known. This does the mutual authentication part. After this as long as there is connection tracking we should be fine? DTLS makes it even more less likely that someone could interfere with injected frames as session is encrypted and MACed. During DTLS handshake with X509 there is certificate verify phase where we have the CertificateVerifier and after that we start to know what the other party is. So in the end of handshake we know the real identity of the other part. And later Californium should be handling connection tracking and DTLS stuff and Leshan shouldn't need to care anymore. Now what happens in Leshan Client. It is preparing the endpoint and thus gets DTLS socket created for the connection. It does not know yet full details of the server but it makes best guess based on URI and with details in Security Object. It prepares LWM2M Registration message that gets sent thru the end point. And as there is not yet DTLS handshake done it starts to perform that. Now that there is this self censorship as it is not sure that destination is correct for message it drops the registration frame and it is never sent and thus not received. This is where I do not understand why this is happening on the send operation as it should only be concern for the peer that the received frame is something that is expected. On receive with DTLS we get identity information from the context which is always correct to operate on (and with full chain re-constructed we can do all checks with it). Californium should be handling the connection tracking part so Leshan should not have a need to care about that. In server side we just need to have state tracking done so that different messages like registration can keep the information like end point name in state and necessary association for device inventory or so. If you set the breakpoint on this context matcher in Leshan side you can observe that the context has been matched already before the method. I have been thinking that perhaps the Security object should be disconnected from actual connection logic. When doing initial connection we do not kinda even have security object until bootstrap server indicates one. With EST it is even more disconnected and certificates are out-of-bound of Security object. And then we have this Certificate Usage business which mixes it even more. |
Because the ideas may vary, Californium just offers all the information, including the peer's identity and the dtls session, to delegate that decision to the next layer. One way to use that is to keep the the |
The matcher is not about authenticate the other peer, that done in the handshake. |
FMPOV, DTLS layer is responsible to authenticate the foreign peer. At LWM2M level we only handle this identity, we don't care about lower layer details. At Leshan level we don't care if DTLS connection or DTLS session changed we just want to be sure that identity don't change. You could ask : but how identity could change ? This is the case for Leshan Server and Bootstrap Server. (1 socket for n client) So in case of 1 socket for 1 foreign peer, if DTLSConnector is restrictive enough to ensure that only the expected peer will connect to this endpoint. (and I think we can do that, this is maybe already the case) we could relax endpoint context matching at client side only. This is maybe simpler. 🤔
I think it's not so true as before server initiated bootstrap there is at least factory bootstrap which contains setting with default credentials store in Security Object.
For EST I don't know enough for now. |
e9d2819
to
b84422e
Compare
Changes:
|
@boaks now certificate usage test suite is "complete". Second eye checking them would be good here. |
@boaks & @sbernard31 created a new Lwm2mClientEndpointContextMatcher that still does the verification but in client case it accepts also "server" certificate to be in any part of the chain. |
@sbernard31 Now this should have all commits that are needed. We can split it to one or more PR's but order is about there now. Change set starts with fixing CN handling in tests as that is integral part of the later changes which make it even more enforced that those names are correct. About trust store change. In Java usage of trust store and key stores is the norm. Also californium has the support out-of-the-box so why not utilize it. This is actually quite good as we may want to utilize key store with EST too at later stages. But that we can look at later. As client is missing the trust store support so idea was to modify LeshanServerDemo first and then just copy that implementation around. It is latter also copied to LeshanBootstrapServerDemo but that is only visible in est-support branch and not here as it is not needed in here. "LeshanClientDemo: Add trust store support." could be split in two to have changes for LeshanClientDemo in own commits if that is what you were looking? LeshanClientDemo part can also be bubbled up. "LeshanClientDemo: Add support for client certificate chain" can also be bubbled up after tests if you want that. "LeshanServerDemo: Add support to load trust store from Java trust store" can also be bubbled up after tests if you want that. "CertificateUsage: Add CertificateUsage support" could also be split to extract LeshanClientDemo changes to own commit and bubble it up after tests if you want that. Then the rest of changes till certificate usage integration tests are just incrementally adding new features so that integration tests can pass. |
That would mean: intermediate CAs will not be allowed as trust anchor. I'm not sure, if that is really expected. If not, see truncateCertificatePath to change it. |
@dachaac Could you please push your commits 1 by 1 or by little consistent group. I let you decide what fits better. The idea is to limit the number of points addressed in the same PR AND be able to merge contribution faster in master. The most disturbing part to me is about using pem instead of der encoding in /0/?/3, I will try to get answer about that from OMA. |
Thinking a bit about that certchain but for certificate usage we don't really need this right ? (I mean the pem stuff) |
@dachaac maybe you should be involved in OpenMobileAlliance/OMA_LwM2M_for_Developers#502 |
Note: some rebase mayhem may have broken the configuration UI. At least it seems to be broken in my est-support testing branch. Need to debug this more. |
Found the fix for webui and applied it to my est-support branch. Should not have effect for this PR. |
PR #905: Trust store updates |
b84422e
to
fcfc2dd
Compare
Now previous commits before certificate usage stuff has been merged so I have now rebased and updated this PR a bit. I have sliced a bit more commits so that *Demo commits are on own commits. Now the question is do we split this more. In order to be testable one needs to have all commits in between. |
I suppose a good next step could be a PR with commits which introduce certificate usage at client side + new integration tests about that ? (without server or demo modification) |
Not so easy anymore ;) But managed to slice it a bit more in PR #912. |
Unfortunately I have been hit with some high priority stuff :|. So my window kinda opens next week for rebasing and for such operations. I believe it would probably be more useful to have all client config stuff and probably a good idea to have also the server side. If you are not too rush to have extra content this week I would probably recommend just fine tuning this PR after your M2 release and when ready then integrate. |
…nfiguration. For now use LWM2M default mode domain issuer certificate (3). Signed-off-by: Vesa Jääskeläinen <dachaac@gmail.com>
…age for clients Adds Web UI for configuring certificate usage setting for registered clients. Signed-off-by: Vesa Jääskeläinen <dachaac@gmail.com>
…mand line Signed-off-by: Vesa Jääskeläinen <dachaac@gmail.com>
Needed to support client certificate chains with intermediate certificates. Signed-off-by: Vesa Jääskeläinen <dachaac@gmail.com>
Adds both single certificate and certiifcate chain versions for PEM encoding. Signed-off-by: Vesa Jääskeläinen <dachaac@gmail.com>
These helpers both support hiding internal implementation and making life a bit easier when defining initial Security object. Signed-off-by: Vesa Jääskeläinen <dachaac@gmail.com>
Signed-off-by: Vesa Jääskeläinen <dachaac@gmail.com>
This enables PKI deployments with intermediate CA's. Signed-off-by: Vesa Jääskeläinen <dachaac@gmail.com>
Helper to convert Certificate array to X509Certficate array. Signed-off-by: Vesa Jääskeläinen <dachaac@gmail.com>
In case host name is set for InetSocketAddress use that to configure SNI's host name. Signed-off-by: Vesa Jääskeläinen <dachaac@gmail.com>
This is mandated by LWM2M v1.1.1 transport specification#5.2.8.3. Signed-off-by: Vesa Jääskeläinen <dachaac@gmail.com>
fcfc2dd
to
1a14a54
Compare
Re-worked the branch on top of master. |
@sbernard31 now that this PR is reworked -- is there some commits that you would like to have -- there are webui stuff here too -- but I suppose you wanted those on top of new webui? |
I had a quick look at this.
They could generate more discussions and will slow down the integration of Certificate usage feature so I think this is better to move this in other PRs. About this PR itself, I do not get the usage of CertificateUsage in SecurityInfo 🤔 ? (commit
I guess this is too soon as the new UI is for now only available for leshan-server-demo* and your changes are about leshan-bsserver-demo*. (I need to work on it too 😬) |
(If you want I can cherry pick the code from this PR and create a new one) |
|
I believe the point made in there was that EST provides the client certificate chain out-of-band of LWM2M protocol itself and thus not subject to any problems. And also initial deployment with client certificate chain is not subject to any problem. The problem that Leshan is that those are quite tied up. Eg. in order to support it one either need to use existing in memory storage of same structure (what has been done now) or to have this information in another structure/class and if updates comes thru EST then that gets updated and if they come thru Security object then they get updated too. And then this another structure is used when making connections. We have already verified with the commercial offering that the concept works. |
This we can continue in the new PR :) |
with #991 I'm not sure if we can close this one ? 🤔 |
LWM2M 1.1 brings certificate usage support.
In order for certificate usage support to be more practical these commits also add other features.
Certificate chain support is being added for client to allow supplying certificate chain with intermediate CA's. Problem with LWM2M is that it can support in X.509 mode only one level CA setup (unless server's trust store has all needed intermediates where only Root CA's should be in trust stores). In EST mode certificate delivery is different and can easily support intermediate CA setups. X.509 problem comes from Security Objects certificate field is X.509 certificate encoded as DER. If it would have been PKCS#7 certificate bundle encoded as DER then it would have worked better.
Note: It is industry best practice to have Root CA in offline environment and Intermediate CA in online environment (like what would be used by EST server or by device manufacturing flows).
Note: Actual EST support is to be coming in future and is not part of these commits.
Note: redis support has not really been tested -- it has just been adapted with match changes.
These commits fix server certificate checking to support subjectAlternativeNames which is today's requirement for TLS Server Certificates (vs. using Subject DN's CN field). At the same time support is added for DNS, IPv4 and IPv6 addresses for TLS Server Certificates.
Fixes: #893
Relates to: #859