[keycloak-dev] Truststore SPI

[Marek Posolda]

Not sure if we should have single SPI or two SPIs? One responsible for 
CRUD (storage/load/querying) certificates and another for verifying? As 
it seems that the first will mostly deal with our DB and Infinispan, 
when the second with the Elytron?

Some more inline...

On 20/08/18 10:25, Sebastian Laskawiec wrote:
> On Tue, Aug 14, 2018 at 11:30 PM Stian Thorgersen <sthorger at redhat.com>
> wrote:
>
>> Was thinking some more about how to share certificate functionality between
>> user authentication and client authentication. Potentially also to SAML.
>>
>> We could add a Truststore SPI. The TruststoreProvider interface would have
>> something like the following methods (quick mock stuff):
>>
> We already have a Truststore SPI [1]. If I remember correctly, it's being
> used when Keycloak needs to create an TLS connection to another IDP.
> Perhaps we could come up with another name, like CertificateSpi?
>
> [1]
> https://github.com/keycloak/keycloak/blob/08dbdeb57f678646a30a8ef44a76a8bf50f87e59/server-spi-private/src/main/java/org/keycloak/truststore/TruststoreSpi.java
>
>
>> * boolean verifyCert(CertificateMatcher matcher)
>> * Set<String> listExpiring()
>>
> This one should probably take a date as an argument (or maybe there should
> be an overloaded method that takes it?).
+1

With regards to this, we have a PR that is opened for longer time. The 
PR adds support for periodically checking the certificates and find 
those, which are expired OR about to expire in next N days. And then 
send email to admin about expired certificates: 
https://github.com/keycloak/keycloak/pull/5121

This PR has some minor issues in it's current form, especially:
- It periodically iterates over all realms and clients and retrieves 
their certificates, which has some performance impact. It also converts 
every found certificate from String (PEM) to the binary format to check 
in the certificate itself if it's expired. Converting from PEM to the 
binary has also some non-trivial performance impact
- It checks just some types of certificates (EG. 
"saml.signing.certificate" and realm RSA certificates) but likely ignore 
some other types

The ability to store some metadata in our DB and being able to 
effectively query all expired certificates OR those about to expire in 
next N days, will be very nice to solve both issues.
>
>
>> * void trustCert(Cert cert)
>>
> Probably addCert would be a better name since we have a removeTrustedCert
> method.
>
>
>> * void removeTrustedCert(String id)
>> * Cert getSavedCert(String id)
>> * void removeSavedCert(String id)
>> * void saveCert(Cert cert)
>>
> Plus, methods that list trusted and saved certs.
>
>
>> We'd have a default built-in TruststoreProvider. Potentially delegating
>> some stuff to Elytron, but it would have to store certs in the db. It would
>> deal with checking if cert is still valid, call cert validation like the
>> user cert does, etc..
>>
> That's a very good idea. Some of the interesting stuff is, like
> Certificates Revocation Lists are coming out of the box once we integrate
> Elytron [2].
>
> [2]
> https://docs.jboss.org/author/display/WFLY/SSL+Configuration+using+Elytron+Subsystem#SSLConfigurationusingElytronSubsystem-UsingaCertificateRevocationList
>
>
>> For user and client authentication we'd use verifyCert primarily. We'd have
>> something like UserCertMatcher.create()... and
>> ClientCertMatcher.create()... In the admin console we could add an option
>> to add trusted certs (not sure how that'd work in a cluster).
>
> It shouldn't be hard. We probably need a DB entity for that and let
> Infinispan do the caching. Although, I'm not sure if we have any
> constraints from the security perspective??
IMO if those will be local caches, the security impact should be fine 
(we already stored various keys in infinispan in binary format, but just 
in local caches).

Marek
>
>
>> For SAML
>> clients we'd delegate storing certs to the Truststore SPI instead of
>> storing the cert data alongside the client. The truststore would also add
>> some metadata in the db about certs so we can easily query for expiring
>> certs.
>> _______________________________________________
>> keycloak-dev mailing list
>> keycloak-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>>
> _______________________________________________
> keycloak-dev mailing list
> keycloak-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/keycloak-dev