IPA: Only treat malformed HBAC rules as fatal if deny rules are enabled

https://fedorahosted.org/sssd/ticket/2603

If deny rules are not in effect, we can skip malformed HBAC rules
because at worst we will deny access. If deny rules are in effect, we
need to error out to be on the safe side and avoid skipping a deny rule.

Reviewed-by: Pavel Březina <pbrezina@redhat.com>

1 files changed, 54 insertions, 14 deletions

diff --git a/src/providers/ipa/ipa_hbac_common.c b/src/providers/ipa/ipa_hbac_common.c
index 8436b7e2d..a7e338e99 100644
--- a/src/providers/ipa/ipa_hbac_common.c
+++ b/src/providers/ipa/ipa_hbac_common.c
@@ -403,18 +403,21 @@ static errno_t
 hbac_eval_user_element(TALLOC_CTX *mem_ctx,
                        struct sss_domain_info *domain,
                        const char *username,
+                       bool deny_rules,
                        struct hbac_request_element **user_element);
 
 static errno_t
 hbac_eval_service_element(TALLOC_CTX *mem_ctx,
                           struct sss_domain_info *domain,
                           const char *servicename,
+                          bool deny_rules,
                           struct hbac_request_element **svc_element);
 
 static errno_t
 hbac_eval_host_element(TALLOC_CTX *mem_ctx,
                        struct sss_domain_info *domain,
                        const char *hostname,
+                       bool deny_rules,
                        struct hbac_request_element **host_element);
 
 static errno_t
@@ -452,17 +455,20 @@ hbac_ctx_to_eval_request(TALLOC_CTX *mem_ctx,
             ret = ENOMEM;
             goto done;
         }
-        ret = hbac_eval_user_element(eval_req, user_dom,
-                                     pd->user, &eval_req->user);
+        ret = hbac_eval_user_element(eval_req, user_dom, pd->user,
+                                     hbac_ctx->get_deny_rules,
+                                     &eval_req->user);
     } else {
-        ret = hbac_eval_user_element(eval_req, domain,
-                                     pd->user, &eval_req->user);
+        ret = hbac_eval_user_element(eval_req, domain, pd->user,
+                                     hbac_ctx->get_deny_rules,
+                                     &eval_req->user);
     }
     if (ret != EOK) goto done;
 
     /* Get the PAM service and service groups */
-    ret = hbac_eval_service_element(eval_req, domain,
-                                    pd->service, &eval_req->service);
+    ret = hbac_eval_service_element(eval_req, domain, pd->service,
+                                    hbac_ctx->get_deny_rules,
+                                    &eval_req->service);
     if (ret != EOK) goto done;
 
     /* Get the source host */
@@ -477,8 +483,9 @@ hbac_ctx_to_eval_request(TALLOC_CTX *mem_ctx,
         rhost = pd->rhost;
     }
 
-    ret = hbac_eval_host_element(eval_req, domain,
-                                 rhost, &eval_req->srchost);
+    ret = hbac_eval_host_element(eval_req, domain, rhost,
+                                 hbac_ctx->get_deny_rules,
+                                 &eval_req->srchost);
     if (ret != EOK) goto done;
 
     /* The target host is always the current machine */
@@ -490,8 +497,9 @@ hbac_ctx_to_eval_request(TALLOC_CTX *mem_ctx,
         goto done;
     }
 
-    ret = hbac_eval_host_element(eval_req, domain,
-                                 thost, &eval_req->targethost);
+    ret = hbac_eval_host_element(eval_req, domain, thost,
+                                 hbac_ctx->get_deny_rules,
+                                 &eval_req->targethost);
     if (ret != EOK) goto done;
 
     *request = talloc_steal(mem_ctx, eval_req);
@@ -507,6 +515,7 @@ static errno_t
 hbac_eval_user_element(TALLOC_CTX *mem_ctx,
                        struct sss_domain_info *domain,
                        const char *username,
+                       bool deny_rules,
                        struct hbac_request_element **user_element)
 {
     errno_t ret;
@@ -564,8 +573,15 @@ hbac_eval_user_element(TALLOC_CTX *mem_ctx,
         ret = get_ipa_groupname(users->groups, domain->sysdb, member_dn,
                                 &users->groups[num_groups]);
         if (ret != EOK && ret != ERR_UNEXPECTED_ENTRY_TYPE) {
-            DEBUG(SSSDBG_MINOR_FAILURE, "Parse error on [%s]\n", member_dn);
-            goto done;
+            if (deny_rules) {
+                DEBUG(SSSDBG_OP_FAILURE, "Parse error on [%s]: %s\n",
+                      member_dn, sss_strerror(ret));
+                goto done;
+            } else {
+                DEBUG(SSSDBG_MINOR_FAILURE,
+                      "Skipping malformed entry [%s]\n", member_dn);
+                continue;
+            }
         } else if (ret == EOK) {
             DEBUG(SSSDBG_TRACE_LIBS, "Added group [%s] for user [%s]\n",
                       users->groups[num_groups], users->name);
@@ -601,6 +617,7 @@ static errno_t
 hbac_eval_service_element(TALLOC_CTX *mem_ctx,
                           struct sss_domain_info *domain,
                           const char *servicename,
+                          bool deny_rules,
                           struct hbac_request_element **svc_element)
 {
     errno_t ret;
@@ -671,7 +688,18 @@ hbac_eval_service_element(TALLOC_CTX *mem_ctx,
         ret = get_ipa_servicegroupname(tmp_ctx, domain->sysdb,
                                        (const char *)el->values[i].data,
                                        &name);
-        if (ret != EOK && ret != ERR_UNEXPECTED_ENTRY_TYPE) goto done;
+        if (ret != EOK && ret != ERR_UNEXPECTED_ENTRY_TYPE) {
+            if (deny_rules) {
+                DEBUG(SSSDBG_OP_FAILURE, "Parse error on [%s]: %s\n",
+                                         (const char *)el->values[i].data,
+                                         sss_strerror(ret));
+                goto done;
+            } else {
+                DEBUG(SSSDBG_MINOR_FAILURE, "Skipping malformed entry [%s]\n",
+                                            (const char *)el->values[i].data);
+                continue;
+            }
+        }
 
         /* ERR_UNEXPECTED_ENTRY_TYPE means we had a memberOf entry that wasn't a
          * service group. We'll just ignore those (could be
@@ -699,6 +727,7 @@ static errno_t
 hbac_eval_host_element(TALLOC_CTX *mem_ctx,
                        struct sss_domain_info *domain,
                        const char *hostname,
+                       bool deny_rules,
                        struct hbac_request_element **host_element)
 {
     errno_t ret;
@@ -777,7 +806,18 @@ hbac_eval_host_element(TALLOC_CTX *mem_ctx,
         ret = get_ipa_hostgroupname(tmp_ctx, domain->sysdb,
                                     (const char *)el->values[i].data,
                                     &name);
-        if (ret != EOK && ret != ERR_UNEXPECTED_ENTRY_TYPE) goto done;
+        if (ret != EOK && ret != ERR_UNEXPECTED_ENTRY_TYPE) {
+            if (deny_rules) {
+                DEBUG(SSSDBG_OP_FAILURE, "Parse error on [%s]: %s\n",
+                                         (const char *)el->values[i].data,
+                                         sss_strerror(ret));
+                goto done;
+            } else {
+                DEBUG(SSSDBG_MINOR_FAILURE, "Skipping malformed entry [%s]\n",
+                                            (const char *)el->values[i].data);
+                continue;
+            }
+        }
 
         /* ERR_UNEXPECTED_ENTRY_TYPE means we had a memberOf entry that wasn't a
          * host group. We'll just ignore those (could be