-
Notifications
You must be signed in to change notification settings - Fork 744
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 Secure Upgrade Test #6816
Add Secure Upgrade Test #6816
Conversation
in this test case we validate non successful install of a given non secure image file adjusting existing upgrade test to be able to run on canonical setup by removing redundant imports
in this test case we validate non successful install of a given non secure image file adjusting existing upgrade test to be able to run on canonical setup by removing redundant imports
in this test case we validate non successful install of a given non secure image file adjusting existing upgrade test to be able to run on canonical setup by removing redundant imports
The pre-commit check detected issues in the files touched by this pull request. For old issues, it is not mandatory to fix them because they were not caused by this change. It is unfair to blame Detailed pre-commit check results: To run the pre-commit checks locally, you can follow below steps:
|
in this test case we validate non successful install of a given non secure image file adjusting existing upgrade test to be able to run on canonical setup by removing redundant imports
The pre-commit check detected issues in the files touched by this pull request. For old issues, it is not mandatory to fix them because they were not caused by this change. It is unfair to blame Detailed pre-commit check results: To run the pre-commit checks locally, you can follow below steps:
|
in this test case we validate non successful install of a given non secure image file adjusting existing upgrade test to be able to run on canonical setup by removing redundant imports
The pre-commit check detected issues in the files touched by this pull request. For old issues, it is not mandatory to fix them because they were not caused by this change. It is unfair to blame Detailed pre-commit check results: To run the pre-commit checks locally, you can follow below steps:
|
in this test case we validate non successful install of a given non secure image file adjusting existing upgrade test to be able to run on canonical setup by removing redundant imports
The pre-commit check detected issues in the files touched by this pull request. For old issues, it is not mandatory to fix them because they were not caused by this change. It is unfair to blame Detailed pre-commit check results: To run the pre-commit checks locally, you can follow below steps:
|
@@ -0,0 +1,48 @@ | |||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a description of how to run the test.
I mean target_image and type parameters
logger.info("Expected fail, msg : {}".format(err_msg)) | ||
pytest_assert("Failure: CMS signature verification failed" in str(err_msg), "failure was not due to security limitations") | ||
finally: | ||
pytest_assert(result=="image install failure", "install non secure image should not succeed") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't want to change the image in case the test failed or passed.
need to be sure at the end of the test the "next" image is the same as the "current" at the beginning of the test.
Use the next commands:
sonic-installer list / show boot
sonic-installer set-default
finally: | ||
pytest_assert(result=="image install failure", "install non secure image should not succeed") | ||
logger.info("Cold reboot the DUT") | ||
reboot(duthost, localhost) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the reboot is not required in the test
from tests.common.fixtures.ptfhost_utils import copy_ptftests_directory # lgtm[py/unused-import] | ||
from tests.common.fixtures.ptfhost_utils import change_mac_addresses # lgtm[py/unused-import] | ||
from tests.common.fixtures.ptfhost_utils import remove_ip_addresses # lgtm[py/unused-import] | ||
from tests.common.fixtures.ptfhost_utils import copy_arp_responder_py # lgtm[py/unused-import] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please restore it.
the usage of these fixtures exist in the tests
in this test case we validate non successful install of a given non secure image file adjusting existing upgrade test to be able to run on canonical setup by removing redundant imports
The pre-commit check detected issues in the files touched by this pull request. For old issues, it is not mandatory to fix them because they were not caused by this change. It is unfair to blame Detailed pre-commit check results: To run the pre-commit checks locally, you can follow below steps:
|
in this test case we validate non successful install of a given non secure image file
The pre-commit check detected issues in the files touched by this pull request. For old issues, it is not mandatory to fix them because they were not caused by this change. It is unfair to blame Detailed pre-commit check results: To run the pre-commit checks locally, you can follow below steps:
|
in this test case we validate non successful install of a given non secure image file
The pre-commit check detected issues in the files touched by this pull request. For old issues, it is not mandatory to fix them because they were not caused by this change. It is unfair to blame Detailed pre-commit check results: To run the pre-commit checks locally, you can follow below steps:
|
in this test case we validate non successful install of a given non secure image file
The pre-commit check detected issues in the files touched by this pull request. For old issues, it is not mandatory to fix them because they were not caused by this change. It is unfair to blame Detailed pre-commit check results: To run the pre-commit checks locally, you can follow below steps:
|
in this test case we validate non successful install of a given non secure image file
The pre-commit check detected issues in the files touched by this pull request. For old issues, it is not mandatory to fix them because they were not caused by this change. It is unfair to blame Detailed pre-commit check results: To run the pre-commit checks locally, you can follow below steps:
|
in this test case we validate non successful install of a given non secure image file
The pre-commit check detected issues in the files touched by this pull request. For old issues, it is not mandatory to fix them because they were not caused by this change. It is unfair to blame Detailed pre-commit check results: To run the pre-commit checks locally, you can follow below steps:
|
in this test case we validate non successful install of a given non secure image file
The pre-commit check detected issues in the files touched by this pull request. For old issues, it is not mandatory to fix them because they were not caused by this change. It is unfair to blame Detailed pre-commit check results: To run the pre-commit checks locally, you can follow below steps:
|
in this test case we validate non successful install of a given non secure image file
in this test case we validate non successful install of a given non secure image file
in this test case we validate non successful install of a given non secure image file
in this test case we validate non successful install of a given non secure image file
in this test case we validate non successful install of a given non secure image file
reboot based on the california law.
The pre-commit check detected issues in the files touched by this pull request. For old issues, it is not mandatory to fix them because they were not caused by this change. It is unfair to blame Detailed pre-commit check results: To run the pre-commit checks locally, you can follow below steps:
|
This pull request introduces 1 alert when merging 5e9520d into b2b45b7 - view on LGTM.com new alerts:
Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog. |
…nitial" This reverts commit 5e9520d.
@azmyali98 can you please confirm that if the test run on a system without secured boot enable, the test is skipped and not raised error? |
@AntonHryshchuk if comments were handled from your POV can you approve? |
LGTM, @wangxin , could you please take a look for it? |
@azmyali98 could you please refer to the open question by Xin? |
Hi Liat,
I will give it a priority, I am currently loaded with our features 🙂
Azmy Ali
…________________________________
From: Liat Grozovik ***@***.***>
Sent: 10 May 2023 21:33
To: sonic-net/sonic-mgmt ***@***.***>
Cc: Azmy Ali ***@***.***>; Mention ***@***.***>
Subject: Re: [sonic-net/sonic-mgmt] Add Secure Upgrade Test (PR #6816)
@azmyali98<https://github.com/azmyali98> could you please refer to the open question by Xin?
—
Reply to this email directly, view it on GitHub<#6816 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AL6IWUYYU2PAO5L5WMINS3DXFPNO5ANCNFSM6AAAAAAR7A5TLA>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
External e-mail, be judicious when opening attachments or links
|
@liat-grozovik, fixed :) |
@wangxin could you please help to approve so we can merge? |
What is the motivation for this PR? We want to add a new secure upgrade test to validate non successful upgrade to non secure image. In details: If we have a secured image installed on a secured system, trying to install a non-secure image on it should fail and we should expect a relevant message indicating so. How did you verify/test it? By taking a secured system with secured image installed on it and a path to non secure image we created privately, we ran the test. Supported testbed topology if it's a new test case? Any topology is supported for the test. Documentation link to feature HLD: sonic-net/SONiC#1024
What is the motivation for this PR? We want to add a new secure upgrade test to validate non successful upgrade to non secure image. In details: If we have a secured image installed on a secured system, trying to install a non-secure image on it should fail and we should expect a relevant message indicating so. How did you verify/test it? By taking a secured system with secured image installed on it and a path to non secure image we created privately, we ran the test. Supported testbed topology if it's a new test case? Any topology is supported for the test. Documentation link to feature HLD: sonic-net/SONiC#1024
Description of PR
Summary:
Type of change
Approach
What is the motivation for this PR?
We want to add a new secure upgrade test to validate non successful upgrade to non secure image.
In details:
If we have a secured image installed on a secured system, trying to install a non-secure image on it
should fail and we should expect a relevant message indicating so.
How did you verify/test it?
By taking a secured system with secured image installed on it and a path to non secure image we created privately,
we ran the test.
Supported testbed topology if it's a new test case?
Any topology is supported for the test.
Documentation
link to feature HLD: sonic-net/SONiC#1024