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

Implementation of agent remove method in WazuhHandler #50

Open
wants to merge 10 commits into
base: system-refactor
Choose a base branch
from

Conversation

pro-akim
Copy link
Member

@pro-akim pro-akim commented Jun 7, 2023

Related Issue
#46

Description

This PR includes a method for removing agents from the manager.

Evidences

Running:

from wazuh_qa_framework.system.wazuh_handler import WazuhEnvironmentHandler
wh = WazuhEnvironmentHandler('inventory.yml')

agent = ['agent1', 'agent2']
print('1. Remove by cmd in parallel')
wh.remove_agents_from_manager(agent, manager=None, method='cmd', parallel=True , logs=False, restart=False)

print('2. Remove by cmd sequentially')
wh.remove_agents_from_manager(agent, manager=None, method='cmd', parallel=False , logs=False, restart=False)

print('3. Remove by API')
wh.remove_agents_from_manager(agent, manager=None, method='api', parallel=False , logs=True, restart=True)

print('4. Removing agents by API + delete logs')
wh.remove_agents_from_manager(agent, manager=None, method='api', parallel=False , logs=False, restart=False)

print('5. Removing agents by API + restart')
wh.remove_agents_from_manager(agent, manager=None, method='api', parallel=False , logs=False, restart=True)

Results:

1. Agents deleted
  1. Agents deleted

  2. Agents deleted

  3. Agents deleted + logs deleted

  4. Agents deleted + agent restarted

@pro-akim pro-akim linked an issue Jun 7, 2023 that may be closed by this pull request
@pro-akim pro-akim self-assigned this Jun 7, 2023
@pro-akim pro-akim requested a review from Rebits June 9, 2023 12:20
Copy link
Member

@Rebits Rebits left a comment

Choose a reason for hiding this comment

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

Good job. Some changes are required

Comment on lines 644 to 704

def get_api_token(self, host, user='wazuh', password='wazuh', auth_context=None, port=55000, check=False):
"""Return an API token for the specified user.

Args:
host (str): Hostname.
user (str, optional): API username. Default `wazuh`
password (str, optional): API password. Default `wazuh`
auth_context (dict, optional): Authorization context body. Default `None`
port (int, optional): API port. Default `55000`
check (bool, optional): Ansible check mode("Dry Run"),
by default it is enabled so no changes will be applied. Default `False`

Returns:
API token (str): Usable API token.
"""
login_endpoint = '/security/user/authenticate'
login_method = 'POST'
login_body = ''
if auth_context is not None:
login_endpoint = '/security/user/authenticate/run_as'
login_body = 'body="{}"'.format(json.dumps(auth_context).replace('"', '\\"').replace(' ', ''))

try:
token_response = self.get_host(host).ansible('uri', f"url=https://localhost:{port}{login_endpoint} "
f"user={user} password={password} "
f"method={login_method} {login_body} validate_certs=no "
f"force_basic_auth=yes",
check=check)
return token_response['json']['data']['token']
except KeyError:
raise KeyError(f'Failed to get token: {token_response}')

def make_api_call(self, host, port=55000, method='GET', endpoint='/', request_body=None, token=None, check=False):
"""Make an API call to the specified host.

Args:
host (str): Hostname.
port (int, optional): API port. Default `55000`
method (str, optional): Request method. Default `GET`
endpoint (str, optional): Request endpoint. It must start with '/'.. Default `/`
request_body ( dict, optional) : Request body. Default `None`
token (str, optional): Request token. Default `None`
check ( bool, optional): Ansible check mode("Dry Run"), by default it is enabled so no changes will be
applied. Default `False`

Returns:
API response (dict) : Return the response in JSON format.
"""
request_body = 'body="{}"'.format(
json.dumps(request_body).replace('"', '\\"').replace(' ', '')) if request_body else ''

token = self.get_api_token(host, user='wazuh', password='wazuh', auth_context=None, port=55000, check=False)

headers = {'Authorization': f'Bearer {token}'}
if request_body:
headers['Content-Type'] = 'application/json'

return self.get_host(host).ansible('uri', f'url="https://localhost:{port}{endpoint}" '
f'method={method} headers="{headers}" {request_body} '
f'validate_certs=no', check=check)
Copy link
Member

Choose a reason for hiding this comment

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

These methods do not belong to the host manager. In addition, there are already defined a wazuh_api module in the repository. Please use it instead

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in 508698d

Comment on lines 505 to 508
# Getting hostnames
host_names = []
for agent in agent_list:
host_names.append(self.run_command(agent, 'hostname')[1])
Copy link
Member

Choose a reason for hiding this comment

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

Agents typically utilize the hostname as their default agent name, although it is possible to modify this setting. For further information on how to make adjustments, you can refer to the documentation page provided here.

However, it seems that the current implementation does not support this particular scenario. To address this limitation, I recommend to create a separate method specifically dedicated to this purpose.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in 508698d

Comment on lines 510 to 523
# Getting id - hostnames from manager
agent_control = self.run_command('manager1', '/var/ossec/bin/manage_agents -l', True)[1]

Args:
agent_list (list, optional): Agent list. Defaults to None.
"""
pass
# Creating id_list from hostnames
agent_ids = []
for hostname in host_names:
hostname = hostname.replace('\r', '').replace('\n', '')
for line in agent_control.split('\n'):
if 'Name: ' + hostname in line:
id_value = line.split(',')[0].split(': ')[1].strip()
agent_ids.append(id_value)
break

return agent_ids
Copy link
Member

Choose a reason for hiding this comment

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

This implementation is a little difficult to read and it is error prone. Some suggestions:

  • Use by default the API due to it is easier to maintain, avoid stdout handling and is could be used to gather other agent information. You can get the id value of agents using this GET request: curl -k -X GET "https://localhost:55000/agents?pretty=true&sort=-id" -H "Authorization: Bearer $TOKEN"
  • If it is necessary to use Wazuh CLI, please create a separate method to do that, formatting correctly de obtained data

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in 508698d

Args:
hosts (_type_, hosts): host list.
"""
# Clean ossec.log and and cluster.log
Copy link
Member

Choose a reason for hiding this comment

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

Why not api.log

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in 508698d

# Clean ossec.log and and cluster.log
for host in hosts:
logs_path = self.get_logs_directory_path(host)
if self.get_host_variables(host)['os_name'] == 'windows':
Copy link
Member

Choose a reason for hiding this comment

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

Avoid to use low level operation, we already has a method to check if the host is a windows: is_windows

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in 508698d

"""
pass
manager = 'manager1' if manager is None else manager
parallel = False if method == 'api' else parallel
Copy link
Member

Choose a reason for hiding this comment

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

Why we can not remove agents if we don't use the API?

Copy link
Member Author

Choose a reason for hiding this comment

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

This line sets the "parallel" parameter to False in case the user wants to use the API as a method, as the API already considers parallel deletion.

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed in 508698d

else:
self.run_command(agent, f"service wazuh-agent restart", become=True, ignore_errors=False)

def remove_agents_from_manager(self, agent_list, manager=None, method='cmd', parallel=True, logs=False,
Copy link
Member

Choose a reason for hiding this comment

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

It is not supported the deletion of the client.keys. Also logs is not a meaningful name for a variable

Copy link
Member Author

Choose a reason for hiding this comment

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

Comments changed in 508698d

Comment on lines 638 to 639
def remove_agent_cmd(id):
self.run_command(manager, f"/var/ossec/bin/manage_agents -r {id}", True)
Copy link
Member

Choose a reason for hiding this comment

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

This should be integrated in a separate method

Copy link
Member Author

Choose a reason for hiding this comment

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

Worked in 508698d

@@ -576,26 +580,84 @@ def clean_client_keys(self, hosts=None):
"""
pass

def clean_logs(self, hosts):
Copy link
Member

Choose a reason for hiding this comment

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

This method does not support parallel logs clean. Please include it

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in 508698d

Comment on lines 658 to 660
# Restarting agents
if restart:
self.restart_agents(agent_list)
Copy link
Member

Choose a reason for hiding this comment

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

When #43 is merged, adapt this block to use it properly

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in 508698d

@pro-akim pro-akim requested a review from Rebits June 15, 2023 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement methods for removing agents
2 participants