Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add base class implementation for local users' passwords reset #465

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 59 additions & 0 deletions sonic_platform_base/local_users_passwords_reset_base.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
'''
local_users_passwords_reset_base.py

Abstract base class for implementing platform-specific
local users' passwords reset base functionality for SONiC
'''
try:
import json
import subprocess

from sonic_py_common.logger import Logger
except ImportError as e:
raise ImportError (str(e) + "- required module not found")

# Global logger class instance
logger = Logger()


DEFAULT_USERS_FILEPATH = '/etc/sonic/default_users.json'


class LocalUsersConfigurationResetBase(object):
"""
Abstract base class for resetting local users' passwords on the switch
"""
def should_trigger(self):
'''
define the condition to trigger
'''
# the condition to trigger start() method, the default implementation will be by checking if a long reboot press was detected.
raise NotImplementedError

@staticmethod
def reset_password(user, hashed_password, expire=False):
'''
This method is used to reset the user's password and expire it (optional) using Linux shell commands.
'''
# Use 'chpasswd' shell command to change password
subprocess.call([f"echo '{user}:{hashed_password}' | sudo chpasswd -e"], shell=True)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not concat command line. It is vulnerable to command argument injection. For example, hashed_password may include blank char.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maipbui to help review this part.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest not to use shell=True, instead use shell=False with an array of strings

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It did not work, the quotations are not working correctly when I split it into list and the pipe does not work

if expire:
# Use 'passwd' shell command to expire password
subprocess.call(['sudo', 'passwd', '-e', f'{user}'])

def start(self):
'''
The functionality defined is to restore original password and expire it for default local users.
It is done by reading default users file and resetting passwords using Linux shell commands.
'''
default_users = {}

# Fetch local users information from default_users
with open(DEFAULT_USERS_FILEPATH) as f:
default_users = json.load(f)

logger.log_info('Restoring default users\' passwords and expiring them')
for user in default_users.keys():
hashed_password = default_users.get(user, {}).get('password')
if hashed_password:
self.reset_password(user, hashed_password, expire=True)
63 changes: 63 additions & 0 deletions tests/local_users_passwords_reset_base_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
'''
Test LocalUsersConfigurationResetBase module
'''
import subprocess
import pytest
from mock import patch, mock_open
from sonic_platform_base.local_users_passwords_reset_base import LocalUsersConfigurationResetBase


DEFAULT_USERS_JSON_EXAMPLE_OUTPUT = '''
{
"admin": {
"expire": "false",
"password": "HASHED_PASSWORD_123"
}
}
'''


class TestLocalUsersConfigurationResetBase:
'''
Collection of LocalUsersConfigurationResetBase test methods
'''
@staticmethod
def test_local_users_passwords_reset_base():
'''
Verify unimplemented methods
'''
base = LocalUsersConfigurationResetBase()
not_implemented_methods = [
(base.should_trigger,)]

for method in not_implemented_methods:
expected_exception = False
try:
func = method[0]
args = method[1:]
func(*args)
except Exception as exc:
expected_exception = isinstance(exc, NotImplementedError)
assert expected_exception

@patch('subprocess.call')
def test_reset_passwords_method(self, mock_subproc_call):
'''
Test the reset passwords static method
'''
LocalUsersConfigurationResetBase.reset_password(
user='admin',
hashed_password='HASHED_PASSWORD_123',
expire=True)
mock_subproc_call.assert_any_call(["echo 'admin:HASHED_PASSWORD_123' | sudo chpasswd -e"], shell=True)
mock_subproc_call.assert_any_call(['sudo', 'passwd', '-e', 'admin'])

@patch('subprocess.call')
@patch("builtins.open", new_callable=mock_open, read_data=DEFAULT_USERS_JSON_EXAMPLE_OUTPUT)
def test_basic_flow_resetting_users_triggered(self, mock_open, mock_subproc_call):
'''
Test the basic flow of resetting local users when long button press is detected
'''
LocalUsersConfigurationResetBase().start()
mock_subproc_call.assert_any_call(["echo 'admin:HASHED_PASSWORD_123' | sudo chpasswd -e"], shell=True)
mock_subproc_call.assert_any_call(['sudo', 'passwd', '-e', 'admin'])
Loading