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 the SONiC FIPS configure introduction #997

Merged
merged 3 commits into from
Jul 8, 2022

Conversation

xumia
Copy link
Contributor

@xumia xumia commented May 7, 2022

Add the SONiC FIPS configure introduction

  1. Add the new config option sonic_fips
  2. Add the config introduction for uboot/aboot

PR:

PR title state context
[sonic-buildimage] Support symcrypt fips config for aboot/uboot GitHub issue/pull request detail GitHub pull request check contexts
[sonic-utilities] Support to enable fips for the command sonic_installer GitHub issue/pull request detail GitHub pull request check contexts

@qiluo-msft
Copy link
Contributor

Could you add all the code PRs into this PR description? You may find example in #985

@xumia
Copy link
Contributor Author

xumia commented Jul 7, 2022

Could you add all the code PRs into this PR description? You may find example in #985

Updated, thanks.

@@ -163,6 +180,16 @@ ENABLE_FIPS ?= n
```
If the ENABLE_FIPS_FEATURE is not set, then the option ENABLE_FIPS is useless.

## SONiC FIPS Command lines
### The command line to enable or disable FIPS
sonic-installer set-fips <image> [--enable-fips=[true|false]]
Copy link
Contributor

@qiluo-msft qiluo-msft Jul 7, 2022

Choose a reason for hiding this comment

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

Is there a default value for --enable-fips? #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to use --enable-fips/--disable-fips. The default value is to enable FIPS, if the option not specified.

@qiluo-msft
Copy link
Contributor

@Staphylo Could you help review?

Copy link
Contributor

@qiluo-msft qiluo-msft left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@Staphylo Staphylo left a comment

Choose a reason for hiding this comment

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

LGTM, just one note but feel free to ignore it.

@@ -163,6 +180,20 @@ ENABLE_FIPS ?= n
```
If the ENABLE_FIPS_FEATURE is not set, then the option ENABLE_FIPS is useless.

## SONiC FIPS Command lines
### The command line to enable or disable FIPS
sonic-installer set-fips <image> [--enable-fips|--disable-fips]
Copy link
Contributor

Choose a reason for hiding this comment

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

set-fips --enable-fips feels redundant compared to just set-fips --enable but this is just nitpicking.
If the plan is to have future knobs to this set-fips command then it's probably better to keep it the way you currently have it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Staphylo , thanks for your comment.
The option --enable-fips can be skipped, the option --enable-fips is not necessary.
To enable it:

sonic-installer set-fips

To disable it:

sonic-installer set-fips --disable-fips

@xumia xumia merged commit 3b9aca7 into sonic-net:master Jul 8, 2022
xumia added a commit to sonic-net/sonic-utilities that referenced this pull request Jul 8, 2022
What I did
Support to enable fips for the command sonic_installer
See sonic-net/SONiC#997

How I did it
sonic-installer set-fips  [--enable-fips|--disable-fips]
sonic-installer get-fips
xumia added a commit to sonic-net/sonic-utilities that referenced this pull request Aug 10, 2022
What I did
Cherry-pick #2154
Support to enable fips for the command sonic_installer
See sonic-net/SONiC#997

How I did it
sonic-installer set-fips  [--enable-fips|--disable-fips]
sonic-installer get-fips
malletvapid23 added a commit to malletvapid23/Sonic-Utility that referenced this pull request Aug 3, 2023
What I did
Support to enable fips for the command sonic_installer
See sonic-net/SONiC#997

How I did it
sonic-installer set-fips  [--enable-fips|--disable-fips]
sonic-installer get-fips
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.

3 participants