[Gluster-devel] GlusterFS SSL behaviour change breaks openstack Manila GlusterFS Native driver

[Deepak Shetty]

On Jan 29, 2015 8:20 PM, "Jeff Darcy" <jdarcy at redhat.com> wrote:
>
> > The recent commit (548547b2e) with subject "transport: fix default
behavior
> > for SSL authorization" changes the behaviour of GlusterFS SSL in a way
that
> > causes to break the Manila usecase.
> >
> > I am willing to change on the Manila side, but before I do, i wanted to
> > quickly put my thoughts on the above change you made and ask some Qs.
> >
> > Before the 548547b2e patch :
> >
> > 1) To enable glusterfs in SSL mode, I used to enable client.ssl and
> > server.ssl options
> > This ensured that no clients (tenants in openstack lingo) with right
> > certificate can access/mount the gluster volume, because default
behaviour
> > was AUTH_REJECT
> >
> > 2) As part of Manila allow acces to share API - i used to enable
> > ssl.auth-allow <CN>, this made the tenant with the right certificate
mount
> > the volume and others couldn't
> >
> > 3) As part of Manila deny access to share API - I used to remove (reset)
> > ssl.auth-allow, this ensured that the tenant couldn't mount, even if he
has
> > the right certificate, at the same time ensuring that GlusterFS volume
is
> > SSLenabled (bcos of client and server .ssl being on)
> >
> > After the 548547b2e patch :
> >
> > 1) Refer to #1 above. This isn't true anymore, because not having the
> > ssl.auth-allow means anyone with the certificate can access, because
default
> > behaviour changed to AUTH_DONT_CARE (iiuc)
>
> > 2) Refer to #2 above. This remains same
>
> > 3) Refer to #3 above. Now removing ssl.auth-allow doesn't work anymore,
> > because removing it means the tenants can still connect (if they try
to) and
> > deny access is broken - thus my Manila native driver is broken .
>
> This is half correct.  The ssl-allow behavior is not broken.  It's fixed,
as
> one could see by reading the bug report associated with the patch.
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1179208
>
> However, the Manila driver is broken.

The Manila driver was written based on the ssl behavior at that time, since
behavior changed, it's broken now :)

>
> > Q1) So, given the behaviour, is there a way to keep the volume SSL
enabled
> > and yet deny access to clients ? (which was earlier possible via
> > ssl.auth-allow's old behaviour)
> >
> > Now, if i want to comply with the new behaviour, as part of deny access
...
> >
> > 3a) I need to remove all the 3 options (ssl.auth-allow, client.ssl,
> > server.ssl) and add a auth.reject * (because we don't have
ssl.auth-reject
> > option support) to ensure the volume cannot be mounted by
tenants/clients,
> > tho' local clients (trusted ones) should be able to connect. This
> > effectively means I disable SSL and use IP based auth.reject *, but
havign
> > SSL based volume was the requirement to begin with, so disablign SSL
seems
> > like compromising security here.
> >
> > -- or --
> >
> > 3b) I can also do as ... remove ssl.auth-allow, and disable one of
> > client/server.ssl option, but then trusted clients (incl quotad etc)
also
> > won't be able to connect
> >
> > -- or --
> >
> > 3c) As part of deny access i keep client/server.ssl on & change
> > ssl.auth-allow <CN> where CN=some randomly generated UUID and hope no
one
> > has the cert with that UUID as CN, this won't work for trusted clients
too
>
> No hope is needed.  If their certificate is not one we accept, then they
> can't connect no matter what their CN is - even if it's the same CN as
some
> other certificate we do accept.  As long as we don't generate a "null"
cert
> with a CN that matches a real tenant, there shouldn't be a problem.
>
> > --or --
> >
> > 3d) As part of deny access, i keep client/server.ssl on & ensure that
> > ssl.auth-allow is a list of CNs where CN1 = gluster server's CN, CN2 =
> > randomly generated UUID. Now local trusted clients should be able to
> > connect, so server administration isn't broken and external clients
cannot
> > connect. This effectively means whenever i modify ssl.auth-allow from my
> > Manila driver, i ensure that keep CN1 as-is and add/remove CN2 based on
> > allow/deny access behaviour needed.
>
> Why do you need a second randomly generated UUID?  If you want to deny all
> tenant access while still allowing internal clients to connect, all you
> need to do is set ssl-allow=CN1.  If you want to allow some specific
tenant
> to connect, you set ssl-allow=CN1,CN2 where CN2 is the real tenant ID.
> Either way, since ssl-allow is non-empty, other tenants will not be able
to
> connect.

Agree.

>
> > -- or --
> >
> > 3e) Have support for ssl.auth-reject option in GlusterFS. As part of
deny
> > access remove ssl.auth-allow, keep client.ssl, server.ssl options, & do
> > ssl.auth-reject *, but this will break trusted clients again! But this
is in
> > contrast to the auth.reject * behaviour where trusted clients are able
to
> > connect. Because git commit 548547b2e aimed to make ssl's auth-allow
> > behaviour consistent with non-ssl behaviour, here too when we add
> > ssl.auth-reject support, we should figure a way to allow trusted
clients to
> > be able to connect as long as client/server.ssl on is present.
> >
> > Looking at the options above ...
> >
> > 1) #3e seems logical to me, but isn't implemented yet.
>
> Patches are always welcome.  ;)
>
> > 2) #3d has the addnl overhead of maintaining the list of CNs as part of
> > ssl.auth-allow get/set, which isn't ideal (compared to #3e) , but given
the
> > status quo , seems to be the only way forward.
>
> Aren't you maintaining such a list (or its equivalent) already, as part of
> your own configuration?  The only additional piece is the cluster's own
CN,
> which doesn't seem like that much of a burden to remember.

Unfortunately, at the time i wrote manila driver, if u remember, i couldn't
get quota working with ssl (
http://thread.gmane.org/gmane.comp.file-systems.gluster.devel/8108) so
didn't had a need to have trusted client's ssl CN in the ssl allow list. My
access/deny was based on presence and absence of ssl.auth-allow <client
CN>. With quota working now + SSL behaviour changing,  I need to maintain a
list of CNs, but ideally if we had made the ssl.auth-allow behaviour
similar to auth.allow/auth.reject, then we really don't need to maintain a
list of CNs ( as i said in #3e above).

>
> > 3) #3a can be done too, but is defintely not clean way, and is more of a
> > workaround than solution
> >
> > On a side note:
> > Option #3d also means Manila code does a get first , appends either
random
> > UUID or correct CN (depending on deny/allow access) and then sets it.
This
> > also means the Manila code knows what the server CN is and potentially
can
> > create a SSL cert with that CN
>
> ...which will do no good unless that *cert* - not CN - becomes trusted on
> the servers.
>
> > and since we trust the client CA
>
> Can you elaborate on that?  Are you using a non-zero certificate depth?
If
> so, why?  Even without any change in ssl-allow behavior, trusting
arbitrary
> clients as CAs seems like a security hole big enough to drive a container
> ship through.

Even for non-zero depth case, The mutual trust between tenants and server
will be pre-setup by the admin, which means

the CA of the client will be trusted on the server side, so any certificate
generated with server CN signed by the same CA will be trusted by the
server. It depends on how stringent the CA certs are protected on the
client side. In general i was wondering if it made sense to expose the
trusted client's CNs to the untrusted client ? Its something that used
internal to the gluster server and should remain concealed i feel.


>
> > it can
> > connect to server as server itself, but I believe we can safely assume
that
> > Manila servcies run inside openstack nodes which are part of the
deployement
> > hence safe enuf not to be hacked this way :).
> >
> > Thoughts ?
>
> One more, in addition to those above.  Anyone who depends on specific SSL
> behavior, and whose needs might conflict with those of other SSL users,
should
> stay active reviewing patches and commenting on the associated bug reports
> before any changes are merged.

Agree :)


thanx,

deepak
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.gluster.org/pipermail/gluster-devel/attachments/20150129/9301954e/attachment-0001.html>