[OSSA 2016-003] template-validate may read server local files (CVE-2015-5295)

[Steven Hardy]

in service.py validate_template, we do an env.get_class bypassing
 the global_environment(), which ends up calling
 template_resource.generate_class, which wrongly defaults the get_template_file
 allowed schemas to "('file',)"

 https://github.com/openstack/heat/blob/master/heat/engine/service.py#L958
 https://github.com/openstack/heat/blob/master/heat/engine/resources/template_resource.py#L31

 The net result of this is that any call to template-validate which
 specifies type: foo.yaml will read that file from the filesystem of the
 heat service - this actually means template-validate calls which should
 fail work on typical devstack env's where the client and heat-engine are
 co-located (it took me a while to work out why!!)

 I've not figured out any way for this to be exploitable, but it definitely
 seems wrong that we allow user-provided paths to be read like this,
 and there could be some risk if folks could work out a way to make
 validation blow up with a stack-trace containing any file contents.

[Steven Hardy (shardy)]

Note I raised this private security initially so we can discuss how this should be handled - AFAICT there's no way for this to be actively exploited, but the sort of risk I'm worried about is e.g if someone tried to pass a path to a hiera yaml file on the heat server box, where the hieradata could contain sensitive information. I don't think there's any way for validation to fail such as to expose that data, but it'd be good to get some more eyes on the code to prove that is the case (if so this can probably be public security IMO).

[Jeremy Stanley (fungi)]

Since this report concerns a possible security risk, an incomplete security advisory task has been added while the core security reviewers for the affected project or projects confirm the bug and discuss the scope of any vulnerability along with potential solutions.

[Garth Mollett (gmollett)]

I think there could at very least be a DoS through unbound memory allocation.

You can cause a call like this to happen:
urllib.request.urlopen("file:///dev/zero").read()

Which on many systems will lead to the python process being killed when it runs out of memory.

[Zane Bitter (zaneb)]

I believe we established somewhere (private email thread) that the filename had to end with .yaml or .template to trigger this code path.

[Angus Salkeld (asalkeld)]

Zane sure, but it should probably be constrained to "/etc/heat/templates/"

[Garth Mollett (gmollett)]

The hash character can be used to get round that check and open an arbitrary file.
This kills my test machine pretty well:

{"files": {}, "environment": {}, "template": {"heat_template_version": "2014-10-16", "description": "a", "resources": {"my_instance": {"type": "file:///dev/zero#a.yaml", "properties": {"flavor": "m1.small"}}}}}

[Garth Mollett (gmollett)]

I didn't reproduce, but from reading I think the "/etc/heat/teamplates" enforcement in proposed patch can be bypassed with path traversal like:

"file:///etc/heat/templates/../../../dev/zero#a.yaml"

I think will still open /dev/zero.

[Angus Salkeld (asalkeld)]

Good point, I'll check and see if I can get a normalized path.

[Angus Salkeld (asalkeld)]

new patch replaces previous one.

[Angus Salkeld (asalkeld)]

Thanks @garth I had to reboot my machine after trying "file:///etc/heat/templates/../../../dev/zero" - doh.

[Garth Mollett (gmollett)]

@angus oops, sorry I should have been more clear that was demonstrating unbound memory allocation still.

[Steven Hardy (shardy)]

@Angus - thanks for picking this up!

TBH I don't think restricting to opening paths in /etc/heat/templates is enough - we should *never* read *any* server-side files as a result of user provided type: filename or (user) resource_registry entries - that (AIUI) is the whole point of the allowed schemes in TemplateResource?

The /etc/heat/templates is only for consumption via the *global* environment, e.g template authors should *only* have access to those templates via their global resource_registry alias as defined in /etc/heat/environment.d - or at least that's the way I thought this was supposed to work?

[Angus Salkeld (asalkeld)]

Hi Steven, the validate flow is a bit different to create/update and we are not sure at that point whether is it user or system resource so it allows both. If you disallow "file" you will probably break stuff.

[Steven Hardy (shardy)]

Hey Angus, yeah I realize there are limitations with the validation in it's current form, which basically doesn't work at all with nested stacks - wouldn't it be better to just skip them completely rather than trying to load random files which may not bear any relation to what the user expects (e.g, how does a user of a public cloud heat have *any* idea what's in /etc/templates of their provider?)

That's one reason why I've been reworking validate to work more like create/preview:

https://review.openstack.org/#/c/223581/

Here, we actually pass the files map, so we can safely do nested validation:

https://review.openstack.org/#/c/224078/

I think this is a better long-term direction, but it obviously doesn't solve the sort term requirement for a backportable quick fix.

In the tests above - are those being done directly via curl, or using heatclient? Because we probably have fixes to do in the client too, e.g because AFAICT the trick with the comment #.yaml works there too:

https://github.com/openstack/python-heatclient/blob/master/heatclient/v1/shell.py#L796
https://github.com/openstack/python-heatclient/blob/master/heatclient/common/template_utils.py#L82

$ cat bad_tmpl.yaml
heat_template_version: 2013-05-23
resources:
  test:
    type: /etc/passwd#.yaml

If you do heat --debug template-validate bad_tmp.yaml you'll see that we read passwd and send it in the files map (even though we know current heat ignores the files at the API level.

I guess this latter case is more a safeguard against users doing something stupid, but we should probably make it more robust if possible.

[Steven Hardy (shardy)]

The sort of fix I was thinking was (not yet tried):

1. Look in resources.global_env().get_class(type_name) (same as elsewhere in service.py)
2. If we don't find the type, check if it matches foo.yaml/foo.template paths with any url prefix, but *don't* read any files
3. If it's not a path/template type then look in the params derived resource_registry for a type alias
4. If the Type isn't found now, it's a validation failure

Then, when the rework I linked above lands, we'd be able to extend (2) and (3) to consume the files passed in the files map from the client (which as mentioned, the client already passes, but the API ignores).

[Garth Mollett (gmollett)]

@steven I did my testing by sending the requests directly, for exactly the reason you said, I saw that heatclient suffers from the same issue and template-validate will validate (and blow up) in the client.

[Tristan Cacqueray (tristan-cacqueray)]

Here is the proposed impact description:

Title: Heat denial of service through template-validate
Reporter: Steven Hardy (Red Hat)
Products: Heat, python-heatclient
Affects: versions through 2014.2.3 and 2015.1 versions through 2015.1.1

Description:
Steven Hardy from Red Hat reported a vulnerability in Heat template validation. By referencing a local file like /dev/null, an authenticated user may trick the heatclient and/or the heat engine service to load arbritrary local file content resulting in a Denial of Service attack through memory exhaustion. Note that the file content is not written back to the user. All Heat setups are affected.

[Zane Bitter (zaneb)]

I think you meant /dev/zero, not /dev/null.

[Steven Hardy (shardy)]

Actually, it appears some of my remarks in comment #15 are wrong - it looks like the normal stack.Stack.validate() path is also affected by this, e.g my rework of template validation doesn't fix it, which means you can probably also trigger this via create/update too.

$ curl -g -i -X POST http://192.168.0.16:8004/v1/63b7b69cab914cb5beeaaee26b933aac/validate -H "User-Agent: python-heatclient" -H "Content-Type: application/json" -H "Accept: application/json" -H "X-Auth-Token: putatokenhere" -d '{"files": {}, "environment": {}, "template": {"heat_template_version": "2013-05-23", "resources": {"test": {"type": "file:///etc/passwd#.yaml"}}}}'

This fails with "The template is not a JSON object" even with my patches applied, when it should fail saying the type cannot be found.

[Jeremy Stanley (fungi)]

The exploit scenario described, referencing a large file or stream to consume system memory, doesn't seem like it would be limited to local paths. What mitigation is in place to prevent large remote files/streams via allowed protocol schemes from having a similar effect?

[Steve Baker (steve-stevebaker)]

On 22/09/15 02:33, Jeremy Stanley wrote:
> The exploit scenario described, referencing a large file or stream to
> consume system memory, doesn't seem like it would be limited to local
> paths. What mitigation is in place to prevent large remote files/streams
> via allowed protocol schemes from having a similar effect?
>
This particular code path only allows the file:// protocol

[Garth Mollett (gmollett)]

While the content of the file is not returned, there is also at very least a minor information leak here.
The error returned by the requests can be used to determine if a file exists and is readable by glance-engine.

[Tristan Cacqueray (tristan-cacqueray)]

Here is the updated impact description with /dev/zero, file existence leak and new affect line (assuming this won't make it for upcoming 2015.1.2):

Title: Heat denial of service through template-validate
Reporter: Steven Hardy (Red Hat)
Products: Heat, python-heatclient
Affects: <=2014.2.3, >=2015.1.0, <=2015.1.2

Description:
Steven Hardy from Red Hat reported a vulnerability in Heat template validation. By referencing a local file like /dev/zero, an authenticated user may trick the heatclient and/or the heat engine service to load arbritrary local file content resulting in a Denial of Service attack through memory exhaustion. Note that the file content is not written back to the user, though the user can determine if a file exists and if it is readable by glance-engine. All Heat setups are affected.

[Steven Hardy (shardy)]

I think you mean s/glance-engine/heat-engine/ above?

[Garth Mollett (gmollett)]

Yes I did. I was (failing at) multi-tasking.

[Tristan Cacqueray (tristan-cacqueray)]

Good catch :-)

Title: Heat denial of service through template-validate
Reporter: Steven Hardy (Red Hat)
Products: Heat, python-heatclient
Affects: <=2014.2.3, >=2015.1.0, <=2015.1.2

Description:
Steven Hardy from Red Hat reported a vulnerability in Heat template validation. By referencing a local file like /dev/zero, an authenticated user may trick the heatclient and/or the heat engine service to load arbritrary local file content resulting in a Denial of Service attack through memory exhaustion. Note that the file content is not written back to the user, though the user can determine if a file exists and if it is readable by heat-engine. All Heat setups are affected.

[Jeremy Stanley (fungi)]

Tristan's updated impact description in comment #27 looks good to me.

[Tristan Cacqueray (tristan-cacqueray)]

Can you please attach backports for stable/juno and kilo that applies cleanly. Also is the patch in comment #10 pre-approved by Heat-coresec ?

Also, is there a fix for python-heatclient yet ?

[Steve Baker (steve-stevebaker)]

Comment #27 looks good.

Regarding the fix in comment #10, operators could set their global environment to host their templates anywhere, not just /etc/heat/templates. A better fix might be to load the global environment and reject any file:// path which doesn't have an explicit files entry in the global env.

Regarding a heatclient fix we have a couple of options which we could discuss here:
- don't fix it, communicate to users that they should only launch templates from trusted sources
- fix it for templates loaded over the network (http, https) so that any file:// url in these templates are rejected

[Tristan Cacqueray (tristan-cacqueray)]

Is someone working on patch update according to comment #30 ?

[Steven Hardy (shardy)]

I'm not sure on the status of Angus's patch, I've been meaning to sync up with him about it.

There has been some (private) discussion around an alternative solution, triggered by https://review.openstack.org/#/c/238194/ which aims to fix a different issue, but ends up having some overlap with this issue (potentially) in terms of implementation.

The potential solution mentioned by Zane is to not generate classes via generate_class except for the global environment, which would mean that we'd never hit the code in heat-engine causing this vulnerability, because all TemplateResource objects instantiated from user provided environments would be directly instantiated template_resource.TemplateResource objects, not subclasses derived from generate_class.

Context is https://review.openstack.org/#/c/79291/3, which shows that generate_class was only added so we could allow resource-type-list to show resources defined in the global environment, e.g it's not actually needed for user provided environments.

I'm currently investigating this approach , to enable fixing bug #1508115, and will report back here (before posting anything in public) should that approach prove viable as a solution to both bugs.

Angus: I'd be interested to chat and hear your thoughts on that as a potential solution.

Tristan: How do we handle this should a fix prove viable as a side-effect to another bug which is not flagged as security impacting?

[Tristan Cacqueray (tristan-cacqueray)]

Steven, while it's not ideal to have a public fix for a private bug, for what it worth, the advance notification (pre-OSSA) can reference public review numbers. If both bugs 1508115 and bug 1496277 can be fixed with one patch, I'd say it's ok to do so directly on gerrit, as long as the security implication is not obvious.

However we need to make sure all the required piece to fix this bug are identified and referenced so that stakeholders can apply the fix.

[Zane Bitter (zaneb)]

If it wasn't referencing a private security bug I believe it would definitely be extremely non-obvious. With the reference to a hidden bug... I think a determined engineer could probably figure it out.

Maybe post a review referencing only bug 1508115 and mark it WIP; when it's got the o-k from a couple of cores upload a new patchset referencing this bug in the commit message and fast-merge it.

[Steven Hardy (shardy)]

@Zane: Yeah I was kinda thinking the same, only I assumed we'd probably just land the patch referencing bug 1508115 and document it as a dependency here, so it can be referenced in the pre-OSSA. I'm not sure if that works wrt the process outlined by Tristan though.

Hopefully I'll have a patch ready later today, I'll post it under bug 1508115 and we can discuss the next steps after it's had some reviews.

[Zane Bitter (zaneb)]

Steve and I have both looked at this and its proving remarkably resistant to a fix that doesn't create other regressions :( We still have some ideas though.

[Tristan Cacqueray (tristan-cacqueray)]

What Steven proposed in comment #35 works for me. If the fix for bug 1508115 also solve bug 1496277, then we'll wait for the fix to be merged before setting a disclosure date.

Note that we also needs backports (Kilo and Liberty at least, but maybe not Juno since it's about to be EOL).
If the original fix is not going to be cherry-picked on gerrit, then please attach required backports to this bug report.

[Steven Hardy (shardy)]

Unfortunately the fix we had in mind for bug 1508115 proved impractical for a number of reasons, so we had to go with a simpler interim fix, which does not have the side-effect of fixing this bug.

So we're still in the process of working through options - basically it boils down to what Angus said in comment #14, the subtext of which is there's considerable hidden complexity around our (inconsistent) application of the user_resource flag in the environment (you can't rely on it always being false for templates loaded from the global environment, which IMO is a bug), and also around our usage of the generate_class derived subclass for facade validation.

I hope we'll have better news soon but if not then we'll probably have to revisit the path restriction workaround posted by Angus in comment #7. My objections to that approach stand, but I now understand the reasons for it much better :(

[Tristan Cacqueray (tristan-cacqueray)]

We've raised the importance to critical since it's a trivial api denial of service.
Is there any progress on the fix ?

[Zane Bitter (zaneb)]

A little, and I'm hoping to spend more time on it over the next couple of days, but there remain some fundamental unanswered questions without which it will be difficult to have confidence in a fix - especially one that needs to be embargoed and thus has to work first time without review/testing feedback from the wider community.

[Zane Bitter (zaneb)]

Testing a fix now that is pretty tidy and I believe keeps everything working. Will try to post a patch tomorrow once I have some more unit tests written. Otherwise it will be early next week (due to Thanksgiving).

[Zane Bitter (zaneb)]

I posted a series here: https://review.openstack.org/#/q/status:open+project:openstack/heat+branch:master+topic:bug/1518458,n,z

The interesting one is https://review.openstack.org/#/c/250053/ which should mostly solve the problem. The exception is the validation code introduced by the fix for bug 1479565. The attached patch should solve the last part of the problem without breaking that fix.

Feeback is appreciated. There are still some murky parts around updates (by which time the previously global type mappings are treated as user type mappings), but in theory these patches can't make anything worse. In theory.

[Zane Bitter (zaneb)]

The patches above all got merged:

https://review.openstack.org/#/q/status:merged+project:openstack/heat+branch:master+topic:bug/1518458,n,z

So if we can get reviews posted here on the attached patch then I can propose this fix + backports.

[Steven Hardy (shardy)]

@Zane: Thanks for looking at this, +1 on the patch in comment #42 - I agree that we may find that self.user_resource isn't treated properly on update - it'd be worth testing a scenario like:

1. Create a stack with a TemplateResource defined in the global environment
2. Modify the file referenced by the global environment mapping
3. Update the stack from (1) and prove the changes are picked up.

IIRC this sequence does currently work, I'll pull your patch tomorrow and re-test, unless anyone else gets to it first.

[Tristan Cacqueray (tristan-cacqueray)]

Steven, any feedback from the test scenario described in comment #44 ?

Zane, the patch doesn't apply cleanly using "git am", can you please format the proposed patch using "git format-patch --stdout HEAD~1". Also it doesn't apply cleanly on stable/liberty nor stable/kilo. Ideally we would need three different patch for each branch.

IIUC, this patch needs to be applied on top of status:merged project:openstack/heat branch:master topic:bug/1518458...
Will those patches need be backported before disclosing this bug ?

[Steven Hardy (shardy)]

@Tristan - yes, sorry for the delay, I've now tested the following:

Via heatclient:

  1. Create stack with a TemplateResource defined in /etc/heat/environment.d/default.yaml referencing the local filesystem
  2. Modified the template file referenced
  3. Update the stack with no other changes.

Turns out the concerns mentioned in comments #42 and #44 were unfounded, AFAICS this still works as it did before, and on update the modified file is loaded into the global environment, and the changes do take effect.

Via curl:

  1. Created a stack referencing a nested stack defined in the "files" map - this worked as expected
  2. Repeat previous step modifying the "files" map key so the type definition in the template points to a valid local path, but it's undefined in the "files" map of the request. This fails with "Invalid URL scheme file" and StackValidationFailed

So, I'm +2 on the fix, from my (admittedly fairly limited) testing it doesn't seem to break anything, and it fixes the reported issue - thanks Zane!

The patch applies fine via git apply or patch -p1, but I'll attach a git am friendly version shortly.

[Zane Bitter (zaneb)]

Yes, we would need to backport that whole series to liberty & kilo. If we're agreed on this fix (it sounds like we are) then I can prepare backports of the whole series and attach them here. My current thinking is that I would squash them all into a single patch per branch.

[Tristan Cacqueray (tristan-cacqueray)]

Patch in comment #47 applied cleanly on master and tox succeed for py27 and pep8. Thanks!

[Tristan Cacqueray (tristan-cacqueray)]

Angus, can you please have a look at proposed patch #47, it seems like the fix is waiting for approval before being backported.

[Angus Salkeld (asalkeld)]

Hi, sorry I was on PTO. That patch looks fine to me (#47).

[Zane Bitter (zaneb)]

Actually the reason for the lack of progress on the backport is that I have been on PTO as well. I'm back now and it's on my list of things to do this week. I'll attach the backported version here when ready.

[Zane Bitter (zaneb)]

Here is the final version of the patch, with improved commit message.

[Zane Bitter (zaneb)]

Here is a backport for Liberty. The second "cherry-picked from" hash may need to be adjusted to match whatever hash the master commit eventually gets.

[Zane Bitter (zaneb)]

Here is a backport for Kilo. The second "cherry-picked from" hash may need to be adjusted to match whatever hash the master commit eventually gets.

[Tristan Cacqueray (tristan-cacqueray)]

Thanks Zane, I confirm this patch fix the issue (at least for rdo-kilo).
Also, tox passed on all three patchs.

@heat-coresec, please review proposed patch (comment #53, #54 and #55).

If patch are +2 here, I propose a disclosure date next week:
2016-01-19, 1500UTC

Would that works for you folks?

[Steven Hardy (shardy)]

Thanks for the updates Zane - all three patches look good to me +2

[Tristan Cacqueray (tristan-cacqueray)]

Though, the python-heatclient is still affected, I had to use direct curl in order to verify the issue was fixed for the API.

What's the status for the client, is it going to be fixed or should we add a Note to the advisory about it ?

[Zane Bitter (zaneb)]

I don't understand what it means for the client to be affected? I'm not worried about the user reading their own files.

[Tristan Cacqueray (tristan-cacqueray)]

Trying to reproduce using this sequence quickly consumes all system memory and crashed my setup:
cat > bad_template.yaml << EOF
heat_template_version: 2013-05-23
description": "a"
resources: {"my_instance": {"type": "file:///dev/zero#a.yaml", "properties": {"flavor": "m1.small"}}}
EOF
heat stack-create --template-file ./bad_template.yaml test_bad

I propose to add this Note to the advisory:
- Until the python-heatclient get updated, this bug also reproduces on the client side, users should only launch template from trusted sources.

[Zane Bitter (zaneb)]

Users should *always* launch templates only from trusted sources. There is no time when launching a template from an untrusted source would be a good idea, any more than running a shell script from an untrusted source would be.

There isn't a bug in the client, so there's nothing to update. There isn't anything we could do even in principle, because the user is allowed to reference any file they like (unlike on the server where they're all supposed to be installed in /etc/heat/). I don't think we should imply otherwise.

[Tristan Cacqueray (tristan-cacqueray)]

Alright, my bad, comment #30 suggested the python-heatclient could get fixed as well.

Here is the final impact description removing client affect:

Title: Heat denial of service through template-validate
Reporter: Steven Hardy (Red Hat)
Products: Heat
Affects: <=2015.1.2, ==5.0.0

Description:
Steven Hardy from Red Hat reported a vulnerability in Heat template validation. By referencing a local file like /dev/zero, an authenticated user may trick the heat engine service to load arbritrary local file content resulting in a Denial of Service attack through memory exhaustion. Note that the file content is not written back to the user, though the user can determine if a file exists and if it is readable by heat-engine. All Heat setups are affected.

[Steven Hardy (shardy)]

Well, there might be something we can do in heatclient to at least try to help users to (knowingly or unknowingly) do the wrong thing.

E.g, we could resolve "file:///dev/zero#a.yaml" and at least check that the resulting file we're reading is a regular file and perhaps ends with an expected extension, e.g yaml, yml, json etc.

But in general I agree with Zane, we tend to take the view that on the client side it's up to the user to ensure their configuration is sane and that they don't DoS themselves.

[Steven Hardy (shardy)]

Obviously I meant *not* do the wrong thing :)

[Tristan Cacqueray (tristan-cacqueray)]

Alright thanks you, the advance notification is sent with public disclosure date/time:
2016-01-19, 1500UTC

I also fixed a typo in impact description proposed in comment #62: s/arbritrary/arbitrary/

[Zane Bitter (zaneb)]

I realise that we don't officially support releases prior to Kilo even for security issues, but for the benefit of anyone running Juno, a completely untested patch is attached.

Affected releases are Icehouse, Juno, Kilo and Liberty.

[Zane Bitter (zaneb)]

And here is a completely untested Icehouse patch.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/269689

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/liberty
Review: https://review.openstack.org/269691

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/kilo
Review: https://review.openstack.org/269692

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/269697

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/269697
Committed: https://git.openstack.org/cgit/openstack/ossa/commit/?id=b3a71f9efa9daff818c0b9fe912b44e84d26a21c
Submitter: Jenkins
Branch: master

commit b3a71f9efa9daff818c0b9fe912b44e84d26a21c
Author: Tristan Cacqueray <email address hidden>
Date: Tue Jan 19 10:03:56 2016 -0500

    Adds OSSA-2016-003 (CVE-2015-5295)

    Related-Bug: #1496277
    Change-Id: Ife3bc05344b7449fd4c5a2e0a1aaa8048c5c7e21

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/269691
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=87116e262329e5cd309a0f1e35ffefd06080022c
Submitter: Jenkins
Branch: stable/liberty

commit 87116e262329e5cd309a0f1e35ffefd06080022c
Author: Zane Bitter <email address hidden>
Date: Tue Nov 24 12:29:38 2015 -0500

    Load template files only from their known source

    Modify get_class to ensure that user-defined resources cannot result in
    reads from the local filesystem. Only resources defined by the operator
    in the global environment should read local files.

    To make this work, this patch also adds a separate
    get_class_to_instantiate() method to the Environment.

    We were previously using get_class for two different purposes - to get a
    resource plugin on which we could perform introspection to obtain the
    properties and attributes schema, and to get a resource plugin we could
    instantiate to create a Resource object. These are both the same except in
    the case of a TemplateResource, where having two different use cases for
    the same piece of code was adding considerable extra complexity. Combining
    the use cases in this way also made the error handling confusing (leading
    to bug 1518458).

    This change separates out the two cases.

    Change-Id: I845e7d23c73242a4a4c9c40599690ab705c75caa
    Closes-Bug: #1496277
    Related-Bug: #1447194
    Related-Bug: #1518458
    Related-Bug: #1508115
    (cherry picked from commit 06a713c4456203cd561f16721dc8ac3bcbb37a3
                           and 26e6d5f6d776c1027c4f27058767952a58d15e25)

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/269692
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=fa19a617a79fd1cb0d892bb8ea87c4b9f6398c34
Submitter: Jenkins
Branch: stable/kilo

commit fa19a617a79fd1cb0d892bb8ea87c4b9f6398c34
Author: Zane Bitter <email address hidden>
Date: Tue Nov 24 12:29:38 2015 -0500

    Load template files only from their known source

    Modify get_class to ensure that user-defined resources cannot result in
    reads from the local filesystem. Only resources defined by the operator
    in the global environment should read local files.

    To make this work, this patch also adds a separate
    get_class_to_instantiate() method to the Environment.

    We were previously using get_class for two different purposes - to get a
    resource plugin on which we could perform introspection to obtain the
    properties and attributes schema, and to get a resource plugin we could
    instantiate to create a Resource object. These are both the same except in
    the case of a TemplateResource, where having two different use cases for
    the same piece of code was adding considerable extra complexity. Combining
    the use cases in this way also made the error handling confusing (leading
    to bug 1518458).

    This change separates out the two cases.

    Change-Id: I845e7d23c73242a4a4c9c40599690ab705c75caa
    Closes-Bug: #1496277
    Related-Bug: #1447194
    Related-Bug: #1518458
    Related-Bug: #1508115
    (cherry picked from commit 06a713c4456203cd561f16721dc8ac3bcbb37a3
                           and 26e6d5f6d776c1027c4f27058767952a58d15e25)

[Zane Bitter (zaneb)]

Here is an updated Icehouse patch with working unit tests.

[Zane Bitter (zaneb)]

Here is an updated Juno patch with working unit tests.

[Doug Hellmann (doug-hellmann)]

This issue was fixed in the openstack/heat 5.0.1 release.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/269689
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=26e6d5f6d776c1027c4f27058767952a58d15e25
Submitter: Jenkins
Branch: master

commit 26e6d5f6d776c1027c4f27058767952a58d15e25
Author: Zane Bitter <email address hidden>
Date: Mon Jan 11 18:43:43 2016 -0500

    Load template files only from their known source

    Modify get_class to ensure that user-defined resources cannot result in
    reads from the local filesystem. Only resources defined by the operator
    in the global environment should read local files.

    Change-Id: I845e7d23c73242a4a4c9c40599690ab705c75caa
    Closes-Bug: #1496277

[Zane Bitter (zaneb)]

Sergey Kraynev from Mirantis reported that the attached patches for the Juno and Icehouse releases do not completely resolve the issue. Specifically, they fix it for stack creates and updates but not for validate-template. I am attaching updated patches for Juno and Icehouse.

See bug 1549033 for more details.

[Garth Mollett (gmollett)]

@VMT/@Tristan Think we need a new CVE for the incomplete fix.

[Garth Mollett (gmollett)]

@Zane is it just the Juno and Icehouse backports that are incomplete?
If so, then ignore my above comment. They're outside of VMT support scope now AFAIK.

[Zane Bitter (zaneb)]

Correct, it was just Juno and Icehouse that had incomplete fixes, and it was also my understanding that the VMT does not make any attempt to support those. The Kilo and Liberty fixes are complete as originally merged.

[Thierry Carrez (ttx)]

This issue was fixed in the openstack/heat 6.0.0.0b3 development milestone.