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

Do not allow authd start when registration password file is empty. #3701

Closed
QU3B1M opened this issue Dec 22, 2022 · 11 comments · Fixed by #3721
Closed

Do not allow authd start when registration password file is empty. #3701

QU3B1M opened this issue Dec 22, 2022 · 11 comments · Fixed by #3721
Assignees

Comments

@QU3B1M
Copy link
Member

QU3B1M commented Dec 22, 2022

Target version Related issue Related PR
4.5.0 #15230 #15709

Description

A new condition was added to main-server.c that stops the manager and raises an error when authd.pass file is empty, this is to prevent agent registration by authd with a empty password.

Proposed test cases

The tests cases in tests/integration/test_authd/test_authd_use_password.py should also check the status of the manager service that now will be stopped after the error, a fixture must be added to start the manager (if stopped) before each test.

Also the following case should be added.

  • Password null
    1. Install wazuh-manager.
    2. Remove the content of /var/ossec/etc/authd.pass.
    3. Start wazuh-manager.
    4. Check the manager stops and logs the error.
  • Password only spaces
    1. Install wazuh-manager.
    2. Fill the file /var/ossec/etc/authd.pass with only spaces.

      echo " " > /var/ossec/etc/authd.pass

    3. Start wazuh-manager.
    4. Check the manager stops and logs the error.

Considerations

Steps to reproduce

  1. Create empty authd.pass in the wazuh-manager

    echo "" > /var/ossec/etc/authd.pass
    chmod 640 /var/ossec/etc/authd.pass
    chown root:wazuh /var/ossec/etc/authd.pass
  2. Enable password usage in wazuh-manager ossec.conf

    <use_password>yes</use_password>
  3. Restart the wazuh-manager to apply the changes

    systemctl restart wazuh-manager
  4. Register an agent using agent-auth:

    root@ip-172-31-8-99:/home/qa# /var/ossec/bin/agent-auth -m "172.31.3.204" -P "" -d
    2022/11/04 17:29:32 agent-auth[10916] debug_op.c:70 at _log(): DEBUG: Logging module auto-initialized
    2022/11/04 17:29:32 agent-auth[10916] main-client.c:228 at main(): DEBUG: Wazuh home directory: /var/ossec
    2022/11/04 17:29:32 agent-auth[10916] main-client.c:271 at main(): INFO: Started (pid: 10916).
    2022/11/04 17:29:32 agent-auth[10916] keys.c:328 at OS_ReadKeys(): DEBUG: (1751): File client.keys not found or empty.
    2022/11/04 17:29:32 agent-auth[10916] enrollment_op.c:121 at w_enrollment_request_key(): INFO: Requesting a key from server: 172.31.3.204
    2022/11/04 17:29:32 agent-auth[10916] ssl.c:83 at os_ssl_keys(): DEBUG: Returning CTX for client.
    2022/11/04 17:29:32 agent-auth[10916] enrollment_op.c:243 at w_enrollment_connect(): DEBUG: (1209): Connected to the Auth service at '172.31.3.204:1515'.
    2022/11/04 17:29:32 agent-auth[10916] enrollment_op.c:481 at w_enrollment_verify_ca_certificate(): DEBUG: Registering agent to unverified manager
    2022/11/04 17:29:32 agent-auth[10916] enrollment_op.c:272 at w_enrollment_send_message(): INFO: Using agent name as: ip-172-31-8-99
    2022/11/04 17:29:32 agent-auth[10916] enrollment_op.c:312 at w_enrollment_send_message(): DEBUG: Request sent to manager
    2022/11/04 17:29:32 agent-auth[10916] enrollment_op.c:337 at w_enrollment_process_response(): INFO: Waiting for server reply
    2022/11/04 17:29:32 agent-auth[10916] enrollment_op.c:348 at w_enrollment_process_response(): ERROR: Invalid request for new agent (from manager)
    2022/11/04 17:29:32 agent-auth[10916] enrollment_op.c:348 at w_enrollment_process_response(): ERROR: Unable to add agent (from manager)
    
    2022/11/04 17:29:32 wazuh-authd: INFO: New connection from 172.31.8.99
    2022/11/04 17:29:32 wazuh-authd: ERROR: Invalid request for new agent from: 172.31.8.99
    

Tasks

  • IT Refactoring
  • Add the case with empty password
@QU3B1M
Copy link
Member Author

QU3B1M commented Dec 23, 2022

Update 23/12/2022

It was found that some test cases are not passing in the PR because of an error, this issue will return to the previous state until its fixed

@QU3B1M
Copy link
Member Author

QU3B1M commented Dec 30, 2022

Update 30/12/2022

  • Check the failing tests locally:
    After the last commit on the PR some errors were fixed, but still have some failing tests:
    test_authd/test_authd_use_password.py::test_authd_force_options[Use_password_yes-Random password, request with correct password]
    test_authd/test_authd_use_password.py::test_authd_force_options[Use_password_yes-Random password, request with wrong password]
    test_authd/test_authd_use_password.py::test_authd_force_options[Use_password_yes-Random password, request without password]
  • Debugging and improving the failing tests
    I've found that the tests were not creating a backup of the authd.pass to restore it after each test, so after the test is finished, wazuh won't start again. But I still trying to figure out what is the specific fail of the first test, as it is an old testfile it is not that clear to understand.

@QU3B1M
Copy link
Member Author

QU3B1M commented Jan 2, 2023

Update 02/01/2023

An unexpected behavior was found in the cases where the authd.pass file does not exist and the use_password tag is configured, in those cases a random password should be generated for the agents to register, but instead authd breaks and the wazuh-manager cant start correctly.

The following log should appear with the random password:

2023/01/02 18:46:28 wazuh-authd: INFO: Accepting connections on port 1515. Random password chosen for agent authentication: da25b9ed229c66b9c720ae24ff899627

This issue will stay blocked until that bug is fixed

@QU3B1M
Copy link
Member Author

QU3B1M commented Jan 3, 2023

Update 03/01/2023

  • Added new automated test case
  • Jenkins and local executions to complete PR in progress
  • Bug tested manually 🟢

@QU3B1M
Copy link
Member Author

QU3B1M commented Jan 31, 2023

Update 31/01/2023 - blocked 🔐

@sandraalvrz
Copy link

Update 01/02/2023

  • Bug tested manually 🟢
  • Add new authd test to validate error when authd.pass is empty in progress

@sandraalvrz
Copy link

Update 10/02/2023

@QU3B1M
Copy link
Member Author

QU3B1M commented Feb 24, 2023

Update 24/02/2023 - Pending review 🚀

@QU3B1M
Copy link
Member Author

QU3B1M commented Feb 27, 2023

Update 27/02/2023 - Apply requested changes ✏️

  • The requested changes were applied, changing issue status to pending review

QU3B1M added a commit that referenced this issue Feb 28, 2023
Co-authored-by: Seyla Dámaris Gomez <[email protected]>
QU3B1M added a commit that referenced this issue Feb 28, 2023
QU3B1M added a commit that referenced this issue Feb 28, 2023
@QU3B1M
Copy link
Member Author

QU3B1M commented Mar 1, 2023

Conclusion 🟢

The test case execution was successful 🟢. Also, a new case was added (password filed only with spaces) that is expected to fail until the issue #16282 is resolved.

@damarisg
Copy link
Member

damarisg commented Mar 1, 2023

QA review

jmv74211 pushed a commit that referenced this issue Mar 2, 2023
* fix(#3701): minor changes

* feat(#3701): add empty password tests for authd

* style(#3701): fix indents and unnecessary quotes

* feat(#3701): finish new authd tests

* docs(#3701): changelog updated

* revert(#3701): testfile refactor reverted

* docs(#3701): add fixture description

* feat(#3701): add new test case

* fix(#3701): spaces filled password case is now xfail

* fix(#3701): password declared directly in the yaml

* docs(#3701): comment improvement

Co-authored-by: Seyla Dámaris Gomez <[email protected]>

* style(#3701): adapt test strucure

* docs(#3701): fix docstrings

* revert(#3701): remove yaml key validation inside test func

* fix(#3701): correct a typo and move constant to init

---------

Co-authored-by: Seyla Dámaris Gomez <[email protected]>
@damarisg damarisg closed this as completed Mar 2, 2023
QU3B1M added a commit that referenced this issue Mar 15, 2023
* fix(#3701): minor changes

* feat(#3701): add empty password tests for authd

* style(#3701): fix indents and unnecessary quotes

* feat(#3701): finish new authd tests

* docs(#3701): changelog updated

* revert(#3701): testfile refactor reverted

* docs(#3701): add fixture description

* feat(#3701): add new test case

* fix(#3701): spaces filled password case is now xfail

* fix(#3701): password declared directly in the yaml

* docs(#3701): comment improvement

Co-authored-by: Seyla Dámaris Gomez <[email protected]>

* style(#3701): adapt test strucure

* docs(#3701): fix docstrings

* revert(#3701): remove yaml key validation inside test func

* fix(#3701): correct a typo and move constant to init

---------

Co-authored-by: Seyla Dámaris Gomez <[email protected]>
Deblintrake09 pushed a commit that referenced this issue Apr 24, 2023
* fix(#3701): minor changes

* feat(#3701): add empty password tests for authd

* style(#3701): fix indents and unnecessary quotes

* feat(#3701): finish new authd tests

* docs(#3701): changelog updated

* revert(#3701): testfile refactor reverted

* docs(#3701): add fixture description

* feat(#3701): add new test case

* fix(#3701): spaces filled password case is now xfail

* fix(#3701): password declared directly in the yaml

* docs(#3701): comment improvement

Co-authored-by: Seyla Dámaris Gomez <[email protected]>

* style(#3701): adapt test strucure

* docs(#3701): fix docstrings

* revert(#3701): remove yaml key validation inside test func

* fix(#3701): correct a typo and move constant to init

---------

Co-authored-by: Seyla Dámaris Gomez <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants