[keycloak-dev] File-based Vault implementation
Stefan Guilhen
sguilhen at redhat.com
Fri Aug 9 14:46:11 EDT 2019
On Fri, Aug 9, 2019 at 10:38 AM Marek Posolda <mposolda at redhat.com> wrote:
> On 09. 08. 19 14:46, Hynek Mlnarik wrote:
> > I agree that certain level of cacheability should be there, however
> > its being enabled needs to be decided by an administrator.
> >
> > If the admin decides to use vault for secrets, we should employ all
> > measures for obtaining it only when needed. If they decide a secret is
> > safe to cache it in Keycloak, then we should support it. Both modes
> > can be implemented for the vault provider (note caching is
> > implementation dependent, see below), but here I'd start with the
> > strictest and safest option of accessing the vault whenever needed. As
> > Sebastian pointed out, the sample implementation might be very fast,
> > even though certainly slower than putting a secret directly into one
> > of the models. Once this is done, we can work on caching.
>
> Yes, the ideal is if admin can decide if he prefer stronger performance
> or security, so ideal is, if caching will be provided in the
> VaultProvider. I agree with that. Maybe we can have CacheVaultProvider,
> which will just delegate to other providers similarly like we do for
> models, but invalidation might be tricky, so not sure...
>
+1 to having a cacheable provider in place, so the factory can instantiate
the proper provider based on some configuration (e.g. cache-enabled,
true-false).
Given that memory can't be fully protected and any kind of secret we use
that can be overridden with garbage really just shortens the window a secret
is exposed in memory, I would simply make the cacheable version default and
allow "paranoid" admins to turn it off if they want it
@Hynek for us that would mean the Cacheable version of the provider either
needs to return a secret whose close() method doesn't do anything or
it has to clone the cached secret before returning it as users of the API
will prob use the secret in try-with-resources blocks and overriding a
cached
secret is definitely the wrong thing to do.
Irrespective of the choice for a default provider I agree with Sebastian's
point that we should run a profiler to get an idea of how much non-cacheable
secrets impact the performance.
> BTV. question is if for the "paranoid" administrators, who would prefer
> the safe path and more memory protection is the MappedByteBuffer good
> option? Hopefully it is fine, but just pointing... :)
>
>
The way I understand it, MappedByteBuffer has the positions of the buffer
that holds the file contents in memory, not the buffer itself. So
if someone can get a heapdump and tries to analyse it the file contents
won't be there. I see it as a protection from a heapdump.
> Marek
>
> >
> > Caching the secrets is connected with invalidation and that is
> > dependent on actual vault implementation. The secret value can change
> > over time, and we'd need to adjust / invalidate the cached value. For
> > example if Kubernetes secrets change in the file, they are immediately
> > propagated to a running pod, and we should pick up the new value.
> > Since this is based on files, we would need to invalidate / reload the
> > entry if file modification time changes.
> >
> > On Fri, Aug 9, 2019 at 10:56 AM Sebastian Laskawiec
> > <slaskawi at redhat.com <mailto:slaskawi at redhat.com>> wrote:
> >
> > At least for File-based Vault implementation, I would like to
> > experiment a
> > bit with MappedByteBuffers [1] (the PR still contains the old
> > code, I'm
> > about to update very soon). If that goes well, we should get a sort
> of
> > trade-off between performance (reading the same secret over and
> > over should
> > be blazing fast) and security (the caller of the vault will obtain
> > a secret
> > and the override it with random data when it's done using it).
> >
> > But that's actually a good point - we should run a performance
> > test (or
> > profile the code using Flight Recorder) once the implementation is
> > ready.
> >
> > [1] https://www.baeldung.com/java-mapped-byte-buffer
> >
> > On Thu, Aug 8, 2019 at 7:24 PM Marek Posolda <mposolda at redhat.com
> > <mailto:mposolda at redhat.com>> wrote:
> >
> > > I am sorry for joining late.
> > >
> > > I guess you already take performance into account, but still I
> would
> > > like to point it again here. Because usually there is some
> trade-off
> > > between performance and security :)
> > >
> > > IMO the important question is at which point exactly the vault
> > will be
> > > called? Will it be directly when particular value (eg. client
> > secret) is
> > > retrieved from DB, so the secret would be still cached in memory
> > as it
> > > is today? Or do you want to prevent caching secrets at all? I would
> > > personally prefer the first option by default due the better
> > performance
> > > and eventually allow the second option in case that people prefer
> > > stronger security against performance.
> > >
> > > For example clientSecret is always needed when refreshing token,
> > > exchanging code-to-token etc. So if you always need to read the
> file
> > > during each refreshToken request, it is not ideal. I see the
> > main point
> > > of the vault is to prevent plain-text passwords in DB. The
> > prevention of
> > > have secrets in memory is not so big priority if it means the
> > > significant performance degradation IMO.
> > >
> > > Marek
> > >
> > >
> > > On 08. 08. 19 14:35, Pedro Igor Silva wrote:
> > > > On Thu, Aug 8, 2019 at 4:34 AM Sebastian Laskawiec
> > <slaskawi at redhat.com <mailto:slaskawi at redhat.com>>
> > > > wrote:
> > > >
> > > >> I briefly looked at the SPI and it seems a bit over the top
> > comparing to
> > > >> what we need. Plus we would create a strong connection
> > between Keycloak
> > > and
> > > >> Elytron Security SPIs and I'm not sure if this is desirable.
> > > >>
> > > >> Maybe a translation layer (a simple Vault SPI implementation
> that
> > > >> delegates to Elytron SPIs) would be better?
> > > >>
> > > > Yeah, it is. Like I said, for this particular case your SPI is
> > more
> > > simple
> > > > and you won't get much from Elytron.
> > > >
> > > >
> > > >>> For read-write, you have the key store implementation from
> > Elytron that
> > > >>> can save you some time. So your credentials are stored more
> > securely
> > > and
> > > >>> you can easily look up them.
> > > >>>
> > > >> I agree with you here. The write path of the Vault SPI is a
> > bit tricky.
> > > >> But I'm not sure if that will happen (we will probably see in
> the
> > > future).
> > > >>
> > > >> My personal vote here is to leave the door open and implement a
> > > delegation
> > > >> layer to Elytron SPIs. We can leave that as an Experimental
> > Feature if
> > > we
> > > >> want to avoid extensive testing on the product side.
> > > >>
> > > > I see. If you are not planning to deliver the write path
> > anytime soon,
> > > > let's talk more about it later.
> > > >
> > > > Thanks.
> > > >
> > > >
> > > >>
> > > >>> I just wanted to let you know about Elytron Credential
> > Store. I haven't
> > > >>> joined the discussions about the credential store proposal
> > so I may be
> > > just
> > > >>> messing your thread :)
> > > >>>
> > > >>> On Tue, Aug 6, 2019 at 10:35 AM Sebastian Laskawiec <
> > > slaskawi at redhat.com <mailto:slaskawi at redhat.com>>
> > > >>> wrote:
> > > >>>
> > > >>>> The idea sounds interesting to me. Although, having in mind
> > our plans
> > > >>>> related to Keycloak.next, I'm not sure if we should provide
> > it out of
> > > the
> > > >>>> box.
> > > >>>>
> > > >>>> Perhaps we should provide a community-driven extension (as
> > a separate
> > > >>>> jar) to use this?
> > > >>>>
> > > >>>> On Tue, Aug 6, 2019 at 2:59 PM Pedro Igor Silva
> > <psilva at redhat.com <mailto:psilva at redhat.com>>
> > > >>>> wrote:
> > > >>>>
> > > >>>>> Hey Hynek,
> > > >>>>>
> > > >>>>> Elytron came into my mind because it provides an SPI for
> > plugging
> > > >>>>> different implementations based on a SPI [1]. There are
> > some OOTB
> > > >>>>> implementations such as a keystore-based and map-based.
> > > >>>>>
> > > >>>>> You should be able to delegate to other vault types or
> > even build
> > > your
> > > >>>>> own on top of some default implementation. Considering
> > that Elytron
> > > >>>>> Subsystem is available as a subsystem you also have the
> > necessary
> > > means to
> > > >>>>> manage your credential stores (via CLI, etc).
> > > >>>>>
> > > >>>>> [1]
> > > >>>>>
> > >
> >
> https://github.com/wildfly-security/wildfly-elytron/blob/1c42623a343e138ac4a31bd5dcfd8d2ccc47633e/credential/store/src/main/java/org/wildfly/security/credential/store/CredentialStoreSpi.java#L35
> > > >>>>>
> > > >>>>> On Tue, Aug 6, 2019 at 3:37 AM Hynek Mlnarik
> > <hmlnarik at redhat.com <mailto:hmlnarik at redhat.com>>
> > > >>>>> wrote:
> > > >>>>>
> > > >>>>>> Hi Pedro,
> > > >>>>>>
> > > >>>>>> Elytron Cred Store has been considered, any details would be
> > > >>>>>> appreciated. Specifically, does it support delegation to
> > other
> > > vault types?
> > > >>>>>> Is it able to delegate access to other vault types, e.g.
> > Kubernetes
> > > >>>>>> credentials? See [1] for further context.
> > > >>>>>>
> > > >>>>>> Pros and cons of other vault implementations are highly
> > appreciated
> > > as
> > > >>>>>> well. The number of built-in implementations mus be kept
> > low (one
> > > or two)
> > > >>>>>> for maintenance reasons, so we need convincing arguments for
> > > including any
> > > >>>>>> in Keycloak. On the other hand, support for other vault
> > types can be
> > > >>>>>> contributed as a Community Extension [2].
> > > >>>>>>
> > > >>>>>> --Hynek
> > > >>>>>>
> > > >>>>>> [1]
> > > >>>>>>
> > >
> >
> https://github.com/keycloak/keycloak-community/pull/18#discussion_r304860227
> > > >>>>>> [2] https://www.keycloak.org/extensions.html
> > > >>>>>>
> > > >>>>>> On Mon, Aug 5, 2019 at 2:55 PM Pedro Igor Silva
> > <psilva at redhat.com <mailto:psilva at redhat.com>>
> > > >>>>>> wrote:
> > > >>>>>>
> > > >>>>>>> Hi Sebastian,
> > > >>>>>>>
> > > >>>>>>> Elytron has a very powerful and flexible Credential
> > Store SPI
> > > (Peter
> > > >>>>>>> can
> > > >>>>>>> give more details) that can help managing credentials
> > based on
> > > keys.
> > > >>>>>>> You
> > > >>>>>>> could even use an implementation backed by a java key
> > store (with
> > > >>>>>>> in-memory
> > > >>>>>>> support).
> > > >>>>>>>
> > > >>>>>>> Wouldn't make sense to use it or at least check how the
> > design
> > > could
> > > >>>>>>> be
> > > >>>>>>> improved to fit our requirements?
> > > >>>>>>>
> > > >>>>>>> Regards.
> > > >>>>>>> Pedro Igor
> > > >>>>>>>
> > > >>>>>>> On Fri, Aug 2, 2019 at 6:39 AM Sebastian Laskawiec <
> > > >>>>>>> slaskawi at redhat.com <mailto:slaskawi at redhat.com>>
> > > >>>>>>> wrote:
> > > >>>>>>>
> > > >>>>>>>> Hey,
> > > >>>>>>>>
> > > >>>>>>>> We are considering an initial, file-based Vault [1]
> > implementation
> > > >>>>>>> that
> > > >>>>>>>> we'll ship out of the box. I imagine a minimum set of
> > requirements
> > > >>>>>>> as the
> > > >>>>>>>> following:
> > > >>>>>>>> - Easy to write by hand (for testing)
> > > >>>>>>>> - Works out of the box in Kubernetes (Kubernetes can
> > mount Secrets
> > > >>>>>>> as
> > > >>>>>>>> files)
> > > >>>>>>>> - Make sure we do not cache file content anywhere, so
> > we don't
> > > >>>>>>> compromise a
> > > >>>>>>>> secret value in Keycloak
> > > >>>>>>>>
> > > >>>>>>>> Essentially, there are two approaches for such an
> > implementation.
> > > >>>>>>>>
> > > >>>>>>>> The first option is to put all secrets into a shared file
> > > >>>>>>> representing
> > > >>>>>>>> key-value pairs (a properties file is a natural
> > candidate for such
> > > >>>>>>> an
> > > >>>>>>>> implementation). This approach very easy to use but
> > it's pretty
> > > >>>>>>> hard to
> > > >>>>>>>> search for a particular key in a file. We would need to
> > make sure
> > > >>>>>>> that we
> > > >>>>>>>> don't cache anything wile parsing the file (in
> > BufferedInputStream
> > > >>>>>>> for
> > > >>>>>>>> example). Such an implementation would also be pretty
> > slow, since
> > > >>>>>>> whenever
> > > >>>>>>>> we'd access the vault for a particular key, we would
> > potentially
> > > >>>>>>> need to
> > > >>>>>>>> search the whole file.
> > > >>>>>>>>
> > > >>>>>>>> The second option is more complicated. Imagine the
> > following file
> > > >>>>>>> structure
> > > >>>>>>>> (inside a vault directory):
> > > >>>>>>>> my-secret-1 (secret value in its content)
> > > >>>>>>>> my-secret-2 (secret value in its content)
> > > >>>>>>>> my-secret-3 (secret value in its content)
> > > >>>>>>>> In other words, each key is a file in a vault directory
> > and its
> > > >>>>>>> content
> > > >>>>>>>> corresponds the secret value. Such an implementation is
> > not very
> > > >>>>>>> easy to
> > > >>>>>>>> use as we'd need to create many small files. However,
> > it's super
> > > >>>>>>> fast for
> > > >>>>>>>> searching and we can securely read the value without a
> > risk of
> > > >>>>>>> compromising
> > > >>>>>>>> other secret values provided by the vault.
> > > >>>>>>>>
> > > >>>>>>>> I wonder what do you think about this? My personal take
> > on this is
> > > >>>>>>> that we
> > > >>>>>>>> should provide both implementations. The former (single
> > file)
> > > would
> > > >>>>>>> be used
> > > >>>>>>>> in our testsuite (because of simplicity) and the latter
> > (multiple
> > > >>>>>>> files) in
> > > >>>>>>>> production and in Kubernetes.
> > > >>>>>>>>
> > > >>>>>>>> Thanks,
> > > >>>>>>>> Sebastian
> > > >>>>>>>>
> > > >>>>>>>> [1]
> > > >>>>>>>>
> > > >>>>>>>>
> > > >>>>>>>
> > >
> >
> https://github.com/keycloak/keycloak-community/blob/master/design/secure-credentials-store.md
> > > >>>>>>>> _______________________________________________
> > > >>>>>>>> keycloak-dev mailing list
> > > >>>>>>>> keycloak-dev at lists.jboss.org
> > <mailto:keycloak-dev at lists.jboss.org>
> > > >>>>>>>> https://lists.jboss.org/mailman/listinfo/keycloak-dev
> > > >>>>>>>>
> > > >>>>>>> _______________________________________________
> > > >>>>>>> keycloak-dev mailing list
> > > >>>>>>> keycloak-dev at lists.jboss.org
> > <mailto:keycloak-dev at lists.jboss.org>
> > > >>>>>>> https://lists.jboss.org/mailman/listinfo/keycloak-dev
> > > >>>>>>>
> > > > _______________________________________________
> > > > keycloak-dev mailing list
> > > > keycloak-dev at lists.jboss.org <mailto:
> keycloak-dev at lists.jboss.org>
> > > > https://lists.jboss.org/mailman/listinfo/keycloak-dev
> > >
> > >
> > >
> > _______________________________________________
> > keycloak-dev mailing list
> > keycloak-dev at lists.jboss.org <mailto: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
--
Stefan Guilhen
Principal Software Engineer
Red Hat <https://www.redhat.com/>
sguilhen at redhat.com IM: sguilhen
@RedHat <https://twitter.com/redhat> Red Hat
<https://www.linkedin.com/company/red-hat> Red Hat
<https://www.facebook.com/RedHatInc>
<https://www.redhat.com/>
More information about the keycloak-dev
mailing list