From 3a540f868bb42c2ccd98b50d0772a5b168ed1fbb Mon Sep 17 00:00:00 2001
From: Stephen Gallagher <sgallagh@redhat.com>
Date: Wed, 16 Jun 2010 14:01:05 -0400
Subject: [PATCH 1/2] Handle (ignore) unknown options in get_domain() and get_service()

We will now eliminate any unknown options and providers to
guarantee that the domain is safe for use.
---
 src/config/SSSDConfig.py               |   36 ++++++++++++++++++++++++-------
 src/config/SSSDConfigTest.py           |   29 ++++++++++++++++++++++++-
 src/config/testconfigs/sssd-valid.conf |   17 +++++++++++++++
 3 files changed, 72 insertions(+), 10 deletions(-)

diff --git a/src/config/SSSDConfig.py b/src/config/SSSDConfig.py
index a05b5334ac03308120821e64cfb9a7e7d41c6640..f1ff02aaad54bcc460614304ba1f33b5ef31a1f0 100644
--- a/src/config/SSSDConfig.py
+++ b/src/config/SSSDConfig.py
@@ -944,7 +944,10 @@ class SSSDDomain(SSSDConfigObject):
         is_provider = option.rfind('_provider')
         if (is_provider > 0):
             provider = option[:is_provider]
-            self.add_provider(value, provider)
+            try:
+                self.add_provider(value, provider)
+            except NoSuchProviderError:
+                raise NoOptionError
         else:
             self.options[option] = value
 
@@ -1251,8 +1254,13 @@ class SSSDConfig(SSSDChangeConf):
             raise NoServiceError
 
         service = SSSDService(name, self.schema)
-        [service.set_option(opt['name'], opt['value'])
-         for opt in self.strip_comments_empty(self.options(name)) ]
+        for opt in self.strip_comments_empty(self.options(name)):
+            try:
+                service.set_option(opt['name'], opt['value'])
+            except NoOptionError:
+                # If we come across an option that we don't recognize,
+                # we should just ignore it and continue
+                pass
 
         return service
 
@@ -1448,12 +1456,24 @@ class SSSDConfig(SSSDChangeConf):
         # errors trying to read in their options
         providers = [ (x['name'],x['value']) for x in self.strip_comments_empty(self.options('domain/%s' % name))
                      if x['name'].rfind('_provider') > 0]
-        [domain.set_option(option, value)
-         for (option, value) in providers]
 
-        [domain.set_option(opt['name'], opt['value'])
-         for opt in self.strip_comments_empty(self.options('domain/%s' % name))
-         if (opt['name'], opt['value']) not in providers]
+        for (option, value) in providers:
+            try:
+                domain.set_option(option, value)
+            except NoOptionError:
+                # If we come across an option that we don't recognize,
+                # we should just ignore it and continue
+                pass
+
+        # Read in all the options from the configuration
+        for opt in self.strip_comments_empty(self.options('domain/%s' % name)):
+            if (opt['name'], opt['value']) not in providers:
+                try:
+                    domain.set_option(opt['name'], opt['value'])
+                except NoOptionError:
+                    # If we come across an option that we don't recognize,
+                    # we should just ignore it and continue
+                    pass
 
         # Determine if this domain is currently active
         domain.active = self.is_domain_active(name)
diff --git a/src/config/SSSDConfigTest.py b/src/config/SSSDConfigTest.py
index 8cbb0f9107fa39e8a6016cca8de73a7eb13424af..056e0f2acf230ad1a9156d075d44de8862647770 100755
--- a/src/config/SSSDConfigTest.py
+++ b/src/config/SSSDConfigTest.py
@@ -1054,6 +1054,8 @@ class SSSDConfigTestSSSDConfig(unittest.TestCase):
             'domain/IPA',
             'domain/LOCAL',
             'domain/LDAP',
+            'domain/INVALIDPROVIDER',
+            'domain/INVALIDOPTION',
             ]
 
         for section in control_list:
@@ -1192,6 +1194,11 @@ class SSSDConfigTestSSSDConfig(unittest.TestCase):
         # Negative Test - No such service
         self.assertRaises(SSSDConfig.NoServiceError, sssdconfig.get_service, 'nosuchservice')
 
+        # Positive test - Service with invalid option loads
+        # but ignores the invalid option
+        service = sssdconfig.get_service('pam')
+        self.assertFalse(service.options.has_key('nosuchoption'))
+
     def testNewService(self):
         sssdconfig = SSSDConfig.SSSDConfig(srcdir + "/etc/sssd.api.conf",
                                            srcdir + "/etc/sssd.api.d")
@@ -1276,7 +1283,10 @@ class SSSDConfigTestSSSDConfig(unittest.TestCase):
 
         control_list = [
             'PROXY',
-            'LDAP']
+            'LDAP',
+            'INVALIDPROVIDER',
+            'INVALIDOPTION',
+            ]
         inactive_domains = sssdconfig.list_inactive_domains()
 
         for domain in control_list:
@@ -1302,7 +1312,10 @@ class SSSDConfigTestSSSDConfig(unittest.TestCase):
             'IPA',
             'LOCAL',
             'PROXY',
-            'LDAP']
+            'LDAP',
+            'INVALIDPROVIDER',
+            'INVALIDOPTION',
+            ]
         domains = sssdconfig.list_domains()
 
         for domain in control_list:
@@ -1337,6 +1350,18 @@ class SSSDConfigTestSSSDConfig(unittest.TestCase):
         # Negative Test - No such domain
         self.assertRaises(SSSDConfig.NoDomainError, sssdconfig.get_domain, 'nosuchdomain')
 
+        # Positive Test - Domain with unknown provider
+        # Expected result: Domain is imported, but does not contain the
+        # unknown provider entry
+        domain = sssdconfig.get_domain('INVALIDPROVIDER')
+        self.assertFalse(domain.options.has_key('chpass_provider'))
+
+        # Positive Test - Domain with unknown option
+        # Expected result: Domain is imported, but does not contain the
+        # unknown option entry
+        domain = sssdconfig.get_domain('INVALIDOPTION')
+        self.assertFalse(domain.options.has_key('nosuchoption'))
+
     def testNewDomain(self):
         sssdconfig = SSSDConfig.SSSDConfig(srcdir + "/etc/sssd.api.conf",
                                            srcdir + "/etc/sssd.api.d")
diff --git a/src/config/testconfigs/sssd-valid.conf b/src/config/testconfigs/sssd-valid.conf
index 3c2dda80c135a6a4b7dcbbbf9e96ad9927fdc4cc..42eeb61c7681dc57e140966d1e30cffb56fd8749 100644
--- a/src/config/testconfigs/sssd-valid.conf
+++ b/src/config/testconfigs/sssd-valid.conf
@@ -36,8 +36,25 @@ id_provider  =    ldap
 auth_provider=ldap
 debug_level = 0
 
+# Domain containing an invalid provider
+[domain/INVALIDPROVIDER]
+ldap_id_use_start_tls = true
+id_provider  =    ldap
+auth_provider=ldap
+debug_level = 0
+chpass_provider = chpass
+
+# Domain containing an invalid option
+[domain/INVALIDOPTION]
+ldap_id_use_start_tls = true
+id_provider  =    ldap
+auth_provider=ldap
+debug_level = 0
+nosuchoption = True
+
 [pam]
 debug_level = 0
+nosuchoption = True
 
 [dp]
 debug_level = 0
-- 
1.7.0.1

