From 5185f0f2fa6bcc3d9d14e447cca15ac1730b70a6 Mon Sep 17 00:00:00 2001
From: Angus Salkeld <asalkeld@mirantis.com>
Date: Thu, 17 Sep 2015 11:43:39 +1000
Subject: [PATCH] Restrict template file access to /etc/heat/templates/

Closes-bug: 1496277
Change-Id: I85613410477392df144f9573b753b8126445367b
---
 heat/common/urlfetch.py                    |  2 ++
 heat/engine/resources/template_resource.py |  3 ++-
 heat/tests/test_provider_template.py       |  5 +++--
 heat/tests/test_urlfetch.py                | 26 ++++++++++++++++++--------
 4 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/heat/common/urlfetch.py b/heat/common/urlfetch.py
index 9cec563..5e9d043 100644
--- a/heat/common/urlfetch.py
+++ b/heat/common/urlfetch.py
@@ -48,6 +48,8 @@ def get(url, allowed_schemes=('http', 'https')):
         raise URLFetchError(_('Invalid URL scheme %s') % components.scheme)
 
     if components.scheme == 'file':
+        if '/etc/heat/templates/' not in components.path:
+            raise URLFetchError(_('Access denied'))
         try:
             return urllib.request.urlopen(url).read()
         except urllib.error.URLError as uex:
diff --git a/heat/engine/resources/template_resource.py b/heat/engine/resources/template_resource.py
index 9fa0fc5..9e67ddf 100644
--- a/heat/engine/resources/template_resource.py
+++ b/heat/engine/resources/template_resource.py
@@ -32,7 +32,8 @@ def generate_class(name, template_name, env, files=None):
     if files is not None:
         data = files.get(template_name)
     if data is None:
-        data = TemplateResource.get_template_file(template_name, ('file',))
+        data = TemplateResource.get_template_file(template_name,
+                                                  ('http', 'https', 'file'))
     tmpl = template.Template(template_format.parse(data))
     props, attrs = TemplateResource.get_schemas(tmpl, env.param_defaults)
     cls = type(name, (TemplateResource,),
diff --git a/heat/tests/test_provider_template.py b/heat/tests/test_provider_template.py
index 9a0e585..a9654b9 100644
--- a/heat/tests/test_provider_template.py
+++ b/heat/tests/test_provider_template.py
@@ -659,7 +659,8 @@ class ProviderTemplateTest(common.HeatTestCase):
                                    'Resources': {}})
         self.m.StubOutWithMock(urlfetch, "get")
         urlfetch.get(test_templ_name,
-                     allowed_schemes=('file',)).AndReturn(minimal_temp)
+                     allowed_schemes=('http', 'https', 'file')).AndReturn(
+                         minimal_temp)
         self.m.ReplayAll()
 
         env_str = {'resource_registry': {'resources': {'fred': {
@@ -691,7 +692,7 @@ class ProviderTemplateTest(common.HeatTestCase):
         self.assertTrue(test_templ, "Empty test template")
         self.m.StubOutWithMock(urlfetch, "get")
         urlfetch.get(test_templ_name,
-                     allowed_schemes=('file',)
+                     allowed_schemes=('http', 'https', 'file')
                      ).AndRaise(urlfetch.URLFetchError(
                          _('Failed to retrieve template')))
         urlfetch.get(test_templ_name,
diff --git a/heat/tests/test_urlfetch.py b/heat/tests/test_urlfetch.py
index 47bcb88b..2b3336e 100644
--- a/heat/tests/test_urlfetch.py
+++ b/heat/tests/test_urlfetch.py
@@ -36,38 +36,48 @@ class Response(object):
 class UrlFetchTest(common.HeatTestCase):
     def setUp(self):
         super(UrlFetchTest, self).setUp()
+        self.file_url = 'file:///etc/heat/templates/a.yaml'
         self.m.StubOutWithMock(requests, 'get')
 
     def test_file_scheme_default_behaviour(self):
         self.m.ReplayAll()
         self.assertRaises(urlfetch.URLFetchError,
-                          urlfetch.get, 'file:///etc/profile')
+                          urlfetch.get, self.file_url)
         self.m.VerifyAll()
 
     def test_file_scheme_supported(self):
         data = '{ "foo": "bar" }'
-        url = 'file:///etc/profile'
 
         self.m.StubOutWithMock(six.moves.urllib.request, 'urlopen')
-        six.moves.urllib.request.urlopen(url).AndReturn(
+        six.moves.urllib.request.urlopen(self.file_url).AndReturn(
             six.moves.cStringIO(data))
         self.m.ReplayAll()
 
-        self.assertEqual(data, urlfetch.get(url, allowed_schemes=['file']))
+        self.assertEqual(data, urlfetch.get(self.file_url,
+                                            allowed_schemes=['file']))
         self.m.VerifyAll()
 
     def test_file_scheme_failure(self):
-        url = 'file:///etc/profile'
-
         self.m.StubOutWithMock(six.moves.urllib.request, 'urlopen')
-        six.moves.urllib.request.urlopen(url).AndRaise(
+        six.moves.urllib.request.urlopen(self.file_url).AndRaise(
             six.moves.urllib.error.URLError('oops'))
         self.m.ReplayAll()
 
         self.assertRaises(urlfetch.URLFetchError,
-                          urlfetch.get, url, allowed_schemes=['file'])
+                          urlfetch.get, self.file_url,
+                          allowed_schemes=['file'])
         self.m.VerifyAll()
 
+    def test_file_bad_path(self):
+        bad_paths = ['file:///etc/heat/heat.conf',
+                     'file:///foo.yaml',
+                     'file:///dev/zero#/etc/heat/templates/a.yaml']
+        for bad_path in bad_paths:
+            exc = self.assertRaises(urlfetch.URLFetchError,
+                                    urlfetch.get, bad_path,
+                                    allowed_schemes=['file'])
+            self.assertIn('Access denied', six.text_type(exc))
+
     def test_http_scheme(self):
         url = 'http://example.com/template'
         data = '{ "foo": "bar" }'
-- 
2.1.4