Open Stack

On 4/27/15, 05:39, "Kuvaja, Erno" <kuvaja at hp.com> wrote:

>Hi all,
> 
>As you probably know CIS was expanded from Juno metadefs work this cycle
>based on spec [1] provided. The implementation was merged in quite a rush
>just before feature freeze.
> 
>During the spec review [2] for client functionality for CIS it came to
>our attention that the implementation exposes Elasticsearch perhaps too
>openly via it’s API (namely the creation of datasets allows API consumer
>uploading arbitrary
> files via the create request).
> 
>Call to action: Please review the CIS functionality again for security
>threats and bring them up so we can form a plan if we need to address
>those and request RC3 before release.
> 
>I have couple of major concerns about this workflow:
>1)     
>I was shocked after reading following statement from the client spec
>review discussion: “””During the Kilo release, we - by we I mean the team
>responsible for implementing the CIS - have discussed such scenario, that
>exposing Elasticsearch
> capabilities to the user consuming the CIS API can bring some serious
>security impact.””” This discussion nor the scenario was never brought to
>attention of the wider Glance community. The spec bluntly states that
>there is no security impact from the implementation
> and the concerns should have been brought up so reviewers would have had
>better chance to catch possible threats.
>2)     
>“””Would like to also address your concern that proposed shape of spec
>allows user to upload arbitrary documents to Elasticsearch (ES is the
>engine used under the hood, we should rather talk about uploading
>documents to CIS service)
> which are not related to Glance in any way (images & metadefs in current
>implementation).””” “””Personally I don't think that discussion about IF
>is a valid topic, because we've already implemented backend for CIS at
>the Glance side and you cannot say A without
> saying B.””” As long as the code is developed under the Glance project
>and reviewed by glance-core it’s outrageous to claim that possible issues
>are not related to Glance in any way. Discussion about if the API is
>implemented by the spec and fits to the mission
> statement is really valid at this point and needs to be thoroughly
>discussed. We need to find the root cause of this attitude and fix it
>before it damages the relationships within the community in a way that
>cannot be fixed.
>3)     
>We had two huge pieces of code merging in at the very end of the
>development cycle Artifacts and CIS and the pressure to merge them in
>(unfortunately not review but merge) was high. On the artifacts side we
>had pretty open discussion
> about the state, the concerns and plans of timelines address those
>concerns. With CIS we unfortunately did not have this openness. Was it
>reflection of 1 & 2 or something else, I do not know, but I surely would
>like to.
> 
>I would like you to look back into those two specs and the comments, look
>back into the implementation and raise any urgent concerns and please
>lets try to have good and healthy base for discussion in the Vancouver
>Summit how we will continue
> forward from this! As Stable Branch Liaison I would really like to know
>what we (and who that we are) are supporting for next couple of cycles,
>as glance-core I would like to know any concerns about used technology or
>implementation people might have and as
> Glance community member I’d like to see us working together towards
>these things and definitely not have these “we” vs. “them”/”you”
>discussions anymore. Bluntly if we need to split the team, let’s do it
>officially, there is room under big tent for every group
> who wants to be with themselves.
> 
>Best Regards,
>Erno
> 
>[1] 
>http://specs.openstack.org/openstack/glance-specs/specs/kilo/catalog-index
>-service.html 
><http://specs.openstack.org/openstack/glance-specs/specs/kilo/catalog-inde
>x-service.html>
>[2] https://review.openstack.org/#/c/173718/
> 
>

I’m admittedly one of the people who read what was in the client
specification and became very nervous. The portion under “Proposed Change”
that discusses how the glance client is going to create a document in
CIS/Elasticsearch does not make reference to the fact that the indices
will be validated by CIS. The code that implements CIS’s endpoints is
actually relatively small (in terms of lines of code). I quickly scanned
the controller portion and found that we reject anything that isn’t a
registered index: 
https://github.com/openstack/glance/blob/582f8563e866f167ae1de1a2309c1a1e24
84442a/glance/search/api/v0_1/search.py#L136 At a glance this seems great.
We only have one index defined by the plugins
https://github.com/openstack/glance/tree/582f8563e866f167ae1de1a2309c1a1e24
84442a/glance/search/plugins (“glance”) and two document types.

There’s a slight problem with this though. We load the plugins dynamically
https://github.com/openstack/glance/blob/582f8563e866f167ae1de1a2309c1a1e24
84442a/glance/common/utils.py#L735 (as anyone really would expect us to)
which means new plugins can be created for any service that is willing to
create one and install it properly. With that done, we /could/ have CIS
become a centralized Elasticsearch API in OpenStack as managed by Glance.

The solution that seems obvious for this is to disallow plugins to declare
their index name (using PluginClass.get_index_name) but I don’t think that
warrants an RC3 or will necessarily make it into 2015.1.1.

If CIS is going to become a fully supported (or non-experimental) Glance
API in Liberty, I think we should really make sure that it is a service
that can only create documents for Glance. Since the API is Experimental,
I think it’s safe to say the API for the Plugins will be considered
experimental and so removing get_index_name from plugin classes will not
break the world.

Cheers,
Ian