An X509 peer should support RPK too
In my case I have a DTLS server (using bouncy castle low level API), which need to support PSK, RPK and X509.
A peer which support X509 could support RPK too. Of course, it should be possible to configure that peer in a way it only accept X509.
Currently, it seems there is a bug (or missing feature) which prevent to create a server which accept both.
A comment in the code seems to confirm that: https://github.com/bcgit/bc-java/blob/0e100a58af34d0cf91ea5cfd1f0a6d36681c3653/tls/src/main/java/org/bouncycastle/tls/AbstractTlsServer.java#L605-L606
Maybe it make sense to try to find supported Server Certificate Type based on getCredentials() but in case this is X509 certificate, maybe RPK should be added too by default ?
Maybe we should have a getSupportedServerCertificateTypes() in AbstractTlsServer with default implementation based on getCredentials, so it would be easy to override it change default behavoir.
(Bonus question, as there is not so much documentation, it is often hard to know what is expected when implementing BC interface/abstract class, typically for getCredentials() should I a create a BcDefaultTlsCredentialedSigner on each call ? I guess at least 1 by handshake and so it should be simpler if getCredentials was called only 1 time by handshake ? currently it is called twice, 1 time to get Server Certificate Type and one time to store it ServerHandshakeState.serverCredentials)
Yeah, I think this could be improved. Mainly we seem to be missing a way to tell the full certificate to encode itself as RawPublicKey, and as you say we might want a method that can restrict what the server is willing to do.
In BCJSSE, ProvTlsServer creates and saves the credentials during selectCipherSuite, then just returns that single instance for any getCredentials calls. So once per handshake. Usually we try to store things in the protocol if they are needed more than once, but in this case the second call crept in via a PR. There's plenty of example code that predates that second getCredentials existing.
I tried to experiment a way to solve that.
The bouncy castle modification 👉 https://github.com/sbernard31/bc-java/commit/69e3e2051ac3f3ca18fcff2f8d81158a501d6caf
How it is used in my code 👉 https://github.com/eclipse-leshan/leshan/commit/bd5fd853b3ba498d3c71ac53e9a062c145391c95
In my case that works.
But this is a bit error prone that at first call of getCredentials() context.getSecurityParametersHandshake().getServerCertificateType(); always return default value : CertificateType.X509
With that in mind, I not so sure this is good idea to have default implementation for getSupportedServerCertificateTypes with current state of the code. 🤔
I tried to explain a bit more why :
Currently user need to implements getCredentials to return a BcDefaultTlsCredentialedSigner
We try to guess ServerCertificateType from it.
But that works not so well if server support several kind of signer.
CertificateType is needed to select signer but we try to find CertificateType from signer.
(This also lead to 2 call to getCredentials)
Maybe a better way would be to provide a more high level API to set credentials. Then from that we can guess supportedCertificateTypes. Then from selected server certificate type (after negociation) + that credentials, we can automatically create a default signer.
More high level credentials API would be way to provide :
- private key
- public key or certificate chain
Let me know if this makes sense or if you have better idea ?
@peterdettman any feedback about that ?
@peterdettman @winfriedgerlach Feedback would be appreciate. Or at least let me know what is a best way to help on this. Or maybe that kind of contributions are not really welcomed, some time project are open source but doesn't have a real open development spirit (which is also OK)
@sbernard31 sorry, I am really just the "label janitor", I cannot answer your question. From what I observe at other tickets, contributions are welcome but it sometimes takes longer until they are picked up. Don't give up yet :-)