Skip to content

Commit

Permalink
Resolve PR comments
Browse files Browse the repository at this point in the history 
Signed-off-by: maipbui <maibui@microsoft.com>
  • Loading branch information
@maipbui
maipbui committed Nov 7, 2022
1 parent 3ba0aae commit e1bb857
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 30 deletions.
24 changes: 12 additions & 12 deletions scripts/caclmgrd
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,12 @@ class ControlPlaneAclManager(daemon_base.DaemonBase):
ipv6_address_cmd3 = ['head', '-1']
return self.run_commands_pipe(ipv6_address_cmd0, ipv6_address_cmd1, ipv6_address_cmd2, ipv6_address_cmd3)

def log_output(self, exitcodes, stdout):
if not any(exitcodes):
self.log_error("Error running command '{}'".format(cmd))
elif stdout:
return stdout.rstrip('\n')

def run_commands(self, commands):
"""
Given a list of shell commands, run them in order
Expand All @@ -185,11 +191,7 @@ class ControlPlaneAclManager(daemon_base.DaemonBase):
proc = subprocess.Popen(cmd, universal_newlines=True, stdout=subprocess.PIPE)

(stdout, stderr) = proc.communicate()

if proc.returncode != 0:
self.log_error("Error running command '{}'".format(cmd))
elif stdout:
return stdout.rstrip('\n')
self.log_output([proc.returncode], stdout)
return ""

def run_commands_pipe(self, cmd0, cmd1, cmd2, cmd3):
Expand All @@ -200,11 +202,7 @@ class ControlPlaneAclManager(daemon_base.DaemonBase):
cmd0, cmd1, cmd2, cmd3: List of strings
"""
exitcodes, stdout = getstatusoutput_noshell_pipe(cmd0, cmd1, cmd2, cmd3)
if not any(exitcodes):
self.log_error("Error running command", cmd0, cmd1, cmd2, cmd3)
elif stdout:
return stdout
return ""
return self.log_output(exitcodes, stdout)

def parse_int_to_tcp_flags(self, hex_value):
tcp_flags_str = ""
Expand Down Expand Up @@ -375,8 +373,10 @@ class ControlPlaneAclManager(daemon_base.DaemonBase):
self.run_commands(iptables_cmds)

def get_chain_list(self, iptable_ns_cmd_prefix, exclude_list):
command = iptable_ns_cmd_prefix + "iptables -L -v -n | grep Chain | awk '{print $2}'"
chain_list = self.run_commands([command]).splitlines()
cmd0 = iptable_ns_cmd_prefix + ['iptables', '-L', '-v', '-n']
cmd1 = ['grep', 'Chain']
cmd2 = ['awk', '{print $2}']
chain_list = self.run_commands_pipe(cmd0, cmd1, cmd2).splitlines()

for chain in exclude_list:
if chain in chain_list:
Expand Down
31 changes: 13 additions & 18 deletions scripts/hostcfgd
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import syslog
import signal
import re
import jinja2
from shlex import split
from sonic_py_common import device_info
from sonic_py_common.general import check_output_pipe
from swsscommon.swsscommon import ConfigDBConnector, DBConnector, Table, SonicDBConfig
Expand Down Expand Up @@ -796,10 +795,8 @@ class AaaCfg(object):

def modify_single_file(self, filename, operations=None):
if operations:
e_operations_str = ' -e '.join(operations)
e_operations_list = split(e_operations_str)
with open(filename+'.new', 'w') as f:
subprocess.call(["sed", "-e"] + e_operations_list + [filename], stdout=f)
subprocess.call(["sed", "-e"] + operations + [filename], stdout=f)
subprocess.call(["mv", '-f', filename, filename+'.old'])
subprocess.call(['mv', '-f', filename+'.new', filename])

Expand Down Expand Up @@ -883,25 +880,25 @@ class AaaCfg(object):
# /etc/pam.d/sudo is not handled, because it would change the existing
# behavior. It can be modified once a config knob is added for sudo.
if os.path.isfile(PAM_AUTH_CONF):
self.modify_single_file(ETC_PAMD_SSHD, [ "'/^@include/s/common-auth$/common-auth-sonic/'" ])
self.modify_single_file(ETC_PAMD_LOGIN, [ "'/^@include/s/common-auth$/common-auth-sonic/'" ])
self.modify_single_file(ETC_PAMD_SSHD, [ "/^@include/s/common-auth$/common-auth-sonic/" ])
self.modify_single_file(ETC_PAMD_LOGIN, [ "/^@include/s/common-auth$/common-auth-sonic/" ])
else:
self.modify_single_file(ETC_PAMD_SSHD, [ "'/^@include/s/common-auth-sonic$/common-auth/'" ])
self.modify_single_file(ETC_PAMD_LOGIN, [ "'/^@include/s/common-auth-sonic$/common-auth/'" ])
self.modify_single_file(ETC_PAMD_SSHD, [ "/^@include/s/common-auth-sonic$/common-auth/" ])
self.modify_single_file(ETC_PAMD_LOGIN, [ "/^@include/s/common-auth-sonic$/common-auth/" ])

# Add tacplus/radius in nsswitch.conf if TACACS+/RADIUS enable
if 'tacacs+' in authentication['login']:
if os.path.isfile(NSS_CONF):
self.modify_single_file(NSS_CONF, [ "'/^passwd/s/ radius//'" ])
self.modify_single_file(NSS_CONF, [ "'/tacplus/b'", "'/^passwd/s/compat/tacplus &/'", "'/^passwd/s/files/tacplus &/'" ])
self.modify_single_file(NSS_CONF, [ "/^passwd/s/ radius//" ])
self.modify_single_file(NSS_CONF, [ "/tacplus/b", '-e', "/^passwd/s/compat/tacplus &/", '-e', "/^passwd/s/files/tacplus &/" ])
elif 'radius' in authentication['login']:
if os.path.isfile(NSS_CONF):
self.modify_single_file(NSS_CONF, [ "'/^passwd/s/tacplus //'" ])
self.modify_single_file(NSS_CONF, [ "'/radius/b'", "'/^passwd/s/compat/& radius/'", "'/^passwd/s/files/& radius/'" ])
self.modify_single_file(NSS_CONF, [ "/radius/b", '-e', "/^passwd/s/compat/& radius/", '-e', "/^passwd/s/files/& radius/" ])
else:
if os.path.isfile(NSS_CONF):
self.modify_single_file(NSS_CONF, [ "'/^passwd/s/tacplus //g'" ])
self.modify_single_file(NSS_CONF, [ "'/^passwd/s/ radius//'" ])
self.modify_single_file(NSS_CONF, [ "/^passwd/s/tacplus //g" ])
self.modify_single_file(NSS_CONF, [ "/^passwd/s/ radius//" ])

# Add tacplus authorization configration in nsswitch.conf
tacacs_authorization_conf = None
Expand Down Expand Up @@ -1012,9 +1009,7 @@ class PasswHardening(object):

def modify_single_file_inplace(self, filename, operations=None):
if operations:
i_operations_str = ' -i '.join(operations)
i_operations_list = split(i_operations_str)
cmd = ["sed", '-i'] + i_operations_list + [filename]
cmd = ["sed", '-i'] + operations + [filename]
syslog.syslog(syslog.LOG_DEBUG, "modify_single_file_inplace: cmd - {}".format(cmd))
subprocess.call(cmd)

Expand Down Expand Up @@ -1061,14 +1056,14 @@ class PasswHardening(object):
self.passwd_aging_expire_modify(curr_expiration, 'MAX_DAYS')

# Aging policy for new users
self.modify_single_file_inplace(ETC_LOGIN_DEF, ["\'/^PASS_MAX_DAYS/c\PASS_MAX_DAYS " +str(curr_expiration)+"\'"])
self.modify_single_file_inplace(ETC_LOGIN_DEF, ["/^PASS_MAX_DAYS/c\PASS_MAX_DAYS " +str(curr_expiration)])

if self.is_passwd_aging_expire_update(curr_expiration_warning, 'WARN_DAYS'):
# Aging policy for existing users
self.passwd_aging_expire_modify(curr_expiration_warning, 'WARN_DAYS')

# Aging policy for new users
self.modify_single_file_inplace(ETC_LOGIN_DEF, ["\'/^PASS_WARN_AGE/c\PASS_WARN_AGE " +str(curr_expiration_warning)+"\'"])
self.modify_single_file_inplace(ETC_LOGIN_DEF, ["/^PASS_WARN_AGE/c\PASS_WARN_AGE " +str(curr_expiration_warning)])

def passwd_aging_expire_modify(self, curr_expiration, age_type):
normal_accounts = self.get_normal_accounts()
Expand Down

0 comments on commit e1bb857

Please sign in to comment.