feat: peer cert as username #775
Conversation
|
@andsel rebased PR ready for your review. The peer cert behavior is behind a BrokerConfiguration flag which is disabled by default. |
| for (Certificate certificate : certificates) { | ||
| pemWriter.writeObject(certificateBoundaryType, certificate.getEncoded()); | ||
| } | ||
| pemWriter.close(); |
There was a problem hiding this comment.
But pemWriter isn't closed automatically by the try statement?
There was a problem hiding this comment.
It is closed automatically yes, however it needs to be closed before calling str.toString() or else the str will not contain the correct contents due to the buffered nature of BufferedWriter which PemWriter extends.
If the authorizer assumes that the operation is coming from the old client and denies the operation, then it is possible for operations which should be allowed to be be silently rejected (as Moquette does not disconnect clients after authZ failures). Please could you describe better? |
|
Thank you for the review @andsel. I've updated the simple parts for formatting and extracting method.
As for the authorizer bit of the description, I took that from the previous PR and it is possible that it no longer applies due to the significant session changes from 0.15 to 0.17. The idea was that an implementor of I will update the description to simplify since this is really just about providing a nicer way to offer TLS mutual auth for devices with client certificates. As a separate point to this PR, yes I do think that unauthorized actions should kick the client (at least as an option); this is what AWS IoT Core does. |
|
Added more testing with real TLS. |
There was a problem hiding this comment.
Thanks a lot @MikeDombo for this. I've a concern about setup method, if you could separate would be great.
Add also a line in the Changelog please 🙏
Thanks for the effort in sustaining the project.
broker/src/test/java/io/moquette/integration/ServerIntegrationSSLClientAuthTest.java
Show resolved
Hide resolved
.../src/test/java/io/moquette/integration/ServerIntegrationSSLClientAuthCertAsUsernameTest.java
Outdated
Show resolved
Hide resolved
.../src/test/java/io/moquette/integration/ServerIntegrationSSLClientAuthCertAsUsernameTest.java
Outdated
Show resolved
Hide resolved
Rebase of #675
This PR adds enables authenticating clients based on their peer certificates without changing the existing authentication interface. This functionality is enabled by the addition of a new configuration option which passes the client certificate PEM in place of
username. The EMQx broker also has the same configuration option.There is also a very small change to behavior with this PR. Previously, clients that did not provide a password were immediately rejected if anonymous sessions were disabled. As far as I can tell, the spec allows username without a password. The code will now allow this and pass the username and empty password to the authenticator.
Why is this needed?
The existing auth interfaces are insufficient when clients need to be identified based on their client certificate. The current interfaces only support username, password, and client ID without any ability for authenticators and authorizers to use the client certificate (if provided) for authN/authZ.
By passing in the client certificate as username, this allows the authorizer to uniquely identify clients and authenticate and authorize based on this cryptographically secure identity.
Close #675