[keycloak-dev] File-based Vault implementation
Marek Posolda
mposolda at redhat.com
Fri Aug 9 08:51:00 EDT 2019
On 09. 08. 19 10:52, Sebastian Laskawiec 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).
Yes, maybe that is good trade-off. Question is, if security is impacted
as MappedByteBuffer is partially using memory anyway. So don't you have
same security issue like if you cache the secret in memory directly in
your app?
Another point is especially related to PrivateKey and other Key objects.
We currently cache the objects directly in the local infinispan cache
(see for example AbstractRsaKeyProvider for details). The reason is,
that converting String in the PEM format to the PrivateKey object has
quite a performance penalty. Maybe converting from ByteBuffer to
PrivateKey is less expensive, so performance is fine, but not really
sure. Will be good to doublecheck. So pointing that as another thing to
consider.
Finally the protecting memory fully is never always possible as for
example clientSecrets are sent as HTTP parameters in the REST requests
(EG. refresh-token request and other OIDC backchannel requests), so
Strings will be defacto still somewhere in the memory for some time...
So just pointing again that IMO the main point of vault is to avoid
secrets in the DB. I am not sure about memory considering that you can't
never fully protect it and considering the performance. Probably depends
how big is performance impact, but if it's bigger, I would rather vote
to still cache secrets in infinispan/memory as we do now.
Marek
>
> 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
>
>
More information about the keycloak-dev
mailing list