CLIENT: SUDO: force check of server credentials

as a general hardening

Reviewed-by: Justin Stephenson <[email protected]>
Reviewed-by: Pavel Březina <[email protected]>
Reviewed-by: Sumit Bose <[email protected]>

src/sss_client/common.c

@@ -1133,7 +1133,7 @@ int sss_sudo_make_request(enum sss_cli_command cmd,
 {
     return sss_cli_make_request_with_checks(cmd, rd, SSS_CLI_SOCKET_TIMEOUT,
                                             repbuf, replen, errnop,
-                                            SSS_SUDO_SOCKET_NAME, false, false);
+                                            SSS_SUDO_SOCKET_NAME, true, false);
 }
 
 int sss_autofs_make_request(enum sss_cli_command cmd,

src/tests/intg/getsockopt_wrapper.c

@@ -31,7 +31,7 @@ static bool is_dbus_socket(int fd)
     return NULL != strstr(unix_socket->sun_path, "system_bus_socket");
 }
 
-static bool peer_is_pam(int fd)
+static bool peer_path_has(int fd, const char *str)
 {
     int ret;
     struct sockaddr_storage addr = { 0 };
@@ -45,10 +45,10 @@ static bool peer_is_pam(int fd)
 
     unix_socket = (struct sockaddr_un *)&addr;
 
-    return NULL != strstr(unix_socket->sun_path, "pipes/pam");
+    return NULL != strstr(unix_socket->sun_path, str);
 }
 
-static bool peer_is_sssctl(const struct ucred *cr)
+static bool peer_is(const struct ucred *cr, const char *str)
 {
     char proc_path[32];
     char cmd_line[255] = { 0 };
@@ -71,7 +71,7 @@ static bool peer_is_sssctl(const struct ucred *cr)
         close(proc_fd);
         if (ret > 0) {
             cmd_line[ret] = 0;
-            if (strncmp(cmd_line, "sssctl", 6) == 0) {
+            if (strstr(cmd_line, str) != NULL) {
                 return true;
             }
         }
@@ -87,11 +87,15 @@ static void fake_peer_uid_gid(uid_t *uid, gid_t *gid)
     val = getenv("SSSD_INTG_PEER_UID");
     if (val != NULL) {
         *uid = atoi(val);
+    } else {
+        *uid = -1;
     }
 
     val = getenv("SSSD_INTG_PEER_GID");
     if (val != NULL) {
         *gid = atoi(val);
+    } else {
+        *gid = -1;
     }
 }
 
@@ -120,7 +124,10 @@ int getsockopt(int sockfd, int level, int optname,
         cr = optval;
         if (cr->uid != 0 && is_dbus_socket(sockfd)) {
             cr->uid = 0;
-        } else if (peer_is_pam(sockfd) || peer_is_sssctl(cr)) {
+        } else if (peer_path_has(sockfd, "pipes/pam") ||
+                   peer_path_has(sockfd, "pipes/sudo") ||
+                   peer_is(cr, "sssctl") ||
+                   peer_is(cr, "sss_sudo_cli")) {
             fake_peer_uid_gid(&cr->uid, &cr->gid);
         }
     }

src/tests/intg/test_sudo.py

@@ -169,7 +169,10 @@ def create_conf_fixture(request, contents):
 
 def create_sssd_process():
     """Start the SSSD process"""
-    if subprocess.call(["sssd", "-D", "--logger=files"]) != 0:
+    my_env = os.environ.copy()
+    my_env['SSSD_INTG_PEER_UID'] = "0"
+    my_env['SSSD_INTG_PEER_GID'] = "0"
+    if subprocess.call(["sssd", "-D", "--logger=files"], env=my_env) != 0:
         raise Exception("sssd start failed")
 
 
@@ -238,12 +241,16 @@ def test_sudo_rule_for_user(add_common_rules, sudocli_tool):
     """
     Test that user1 is allowed in the rule but user2 is not
     """
-    user1_rules = get_call_output([sudocli_tool, "user1"])
+    my_env = os.environ.copy()
+    my_env['SSSD_INTG_PEER_UID'] = "0"
+    my_env['SSSD_INTG_PEER_GID'] = "0"
+    user1_rules = get_call_output([sudocli_tool, "user1"], custom_env=my_env)
+    print(user1_rules)
     reply = SudoReply(user1_rules)
     assert len(reply.sudo_rules.rules) == 1
     assert reply.sudo_rules.rules[0]['cn'] == 'user1_allow_less_shadow'
 
-    user2_rules = get_call_output([sudocli_tool, "user2"])
+    user2_rules = get_call_output([sudocli_tool, "user2"], custom_env=my_env)
     reply = SudoReply(user2_rules)
     assert len(reply.sudo_rules.rules) == 0
 
@@ -273,13 +280,16 @@ def test_sudo_rule_duplicate_sudo_user(add_double_qualified_rules,
     Test that despite user1 and user1@LDAP meaning the same user,
     the rule is still usable
     """
+    my_env = os.environ.copy()
+    my_env['SSSD_INTG_PEER_UID'] = "0"
+    my_env['SSSD_INTG_PEER_GID'] = "0"
     # Try several users to make sure we don't mangle the list
     for u in ["user1", "user2", "user3"]:
-        user_rules = get_call_output([sudocli_tool, u])
+        user_rules = get_call_output([sudocli_tool, u], custom_env=my_env)
         reply = SudoReply(user_rules)
         assert len(reply.sudo_rules.rules) == 1
         assert reply.sudo_rules.rules[0]['cn'] == 'user1_allow_less_shadow'
 
-    user4_rules = get_call_output([sudocli_tool, "user4"])
+    user4_rules = get_call_output([sudocli_tool, "user4"], custom_env=my_env)
     reply = SudoReply(user4_rules)
     assert len(reply.sudo_rules.rules) == 0

src/tests/intg/util.py

@@ -82,7 +82,7 @@ def restore_envvar_file(name):
     os.rename(backup_path, path)
 
 
-def get_call_output(cmd, stderr_output=subprocess.PIPE, check=False):
+def get_call_output(cmd, stderr_output=subprocess.PIPE, check=False, custom_env=None):
     """
         Executes the provided command.
         When check is set to True, this function will throw an exception
@@ -93,7 +93,7 @@ def get_call_output(cmd, stderr_output=subprocess.PIPE, check=False):
             or (sys.version_info.major == 3 and sys.version_info.minor < 7)):
         try:
             output = subprocess.check_output(cmd, universal_newlines=True,
-                                             stderr=stderr_output)
+                                             stderr=stderr_output, env=custom_env)
         except subprocess.CalledProcessError as err:
             if (not check):
                 output = err.output
@@ -102,5 +102,6 @@ def get_call_output(cmd, stderr_output=subprocess.PIPE, check=False):
         return output
 
     process = subprocess.run(cmd, check=check, text=True,
-                             stdout=subprocess.PIPE, stderr=stderr_output)
+                             stdout=subprocess.PIPE, stderr=stderr_output,
+                             env=custom_env)
     return process.stdout