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.

Sign up for GitHub

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

Jump to bottom

Support OpenSSL 3.0 SymCrypt provider and engine for bookworm #18088

Merged
merged 1 commit into from
May 23, 2024
Merged

Support OpenSSL 3.0 SymCrypt provider and engine for bookworm #18088

merged 1 commit into from
May 23, 2024
+44 −25

Conversation

xumia
Copy link
Collaborator

@xumia xumia commented Feb 13, 2024 •
edited
Loading

Why I did it

Support OpenSSL 3.0 SymCrypt provider and engine for bookworm
Restore the feature support for SymCrypt-OpenSSL.
For bookworm, using OpenSSL 3.0.11, it supports both provider and engine. The engine is in deprecating, it will be deprecated in the long term. Currently, some of the applications which still use the low-level OpenSSL APIs are not ready to migrate to OpenSSL provider, so OpenSSL engine will still be used for some time.
The OpenSSL SymCrypt provider and engine are included in the openssl-symcrypt debian package (>=1.0-preview).

Work item tracking
  • Microsoft ADO (number only): 27655936

How I did it

Integrate the OpenSSL SymCrypt provider and engine into SONiC to restore the SONiC FIPS feature.

How to verify it

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211
  • 202305

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

@k-v1
Copy link
Contributor

k-v1 commented Feb 13, 2024

@xumia
Is it possible to also merge this fix (#18084) for rules/sonic-fips.mk?

zjswhhh
zjswhhh reviewed Mar 12, 2024
View reviewed changes
Copy link
Contributor

@zjswhhh zjswhhh left a comment

Choose a reason for hiding this comment

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

Hi @xumia -is there any ETA for this fix?

@xumia xumia force-pushed the support-fips-openssl-3.0 branch from 0482776 to f48bb1c Compare April 10, 2024 23:26
@xumia
Copy link
Collaborator Author

xumia commented Apr 10, 2024

Hi @xumia -is there any ETA for this fix?

The upstream symcrypt repo to support provider + engine is not ready yet. Some of the OpenSSL UTs are failed. It is only for test now. Maybe in this month.

@saiarcot895 saiarcot895 mentioned this pull request Apr 22, 2024
Merged
6 tasks
@xumia xumia force-pushed the support-fips-openssl-3.0 branch from 12a1541 to 16cc57d Compare May 17, 2024 01:54
@xumia
Copy link
Collaborator Author

xumia commented May 17, 2024

/azp run ms_conflict

@azure-pipelines Azure Pipelines
Copy link

No pipelines are associated with this pull request.

@wumiaont
Copy link
Contributor

/azpw ms_conflict

@wumiaont
Copy link
Contributor

@qiluo-msft @zjswhhh help to review this PR? Need to get into master to meet the 202405 due date (end of May).

@xumia xumia force-pushed the support-fips-openssl-3.0 branch from 16cc57d to 69d3047 Compare May 21, 2024 23:16
@xumia xumia marked this pull request as ready for review May 21, 2024 23:16
@xumia xumia requested review from qiluo-msft and lguohan as code owners May 21, 2024 23:16
@xumia xumia changed the title Support OpenSSL 3.0 SymCrypt provider for bookworm Support OpenSSL 3.0 SymCrypt provider and engine for bookworm May 21, 2024
@xumia xumia requested a review from saiarcot895 May 21, 2024 23:41
@saiarcot895
Copy link
Contributor

saiarcot895 commented May 22, 2024 •
edited
Loading

@xumia At least in the Broadcom build (likely other platforms as wel), it doesn' t look like the FIPS version of openssh is getting built. From the logs:

[ building ] [ target/debs/bookworm/openssh-server_9.2p1-2+deb12u1_amd64.deb ]

Is there some dependency that needs to be updated?

Edit: never mind, it's getting packaged in:

+ sudo dpkg --root=./fsroot-broadcom -i target/debs/bookworm/openssh-sftp-server_9.2p1-2+deb12u2+fips_amd64.deb
Selecting previously unselected package openssh-sftp-server.
(Reading database ... 51526 files and directories currently installed.)
Preparing to unpack .../openssh-sftp-server_9.2p1-2+deb12u2+fips_amd64.deb ...
Unpacking openssh-sftp-server (1:9.2p1-2+deb12u2+fips) ...
Setting up openssh-sftp-server (1:9.2p1-2+deb12u2+fips) ...
+ sudo dpkg --root=./fsroot-broadcom -i target/debs/bookworm/openssh-server_9.2p1-2+deb12u2+fips_amd64.deb
Selecting previously unselected package openssh-server.
(Reading database ... 51530 files and directories currently installed.)
Preparing to unpack .../openssh-server_9.2p1-2+deb12u2+fips_amd64.deb ...
Unpacking openssh-server (1:9.2p1-2+deb12u2+fips) ...
Setting up openssh-server (1:9.2p1-2+deb12u2+fips) ...
debconf: unable to initialize frontend: Dialog
debconf: (Dialog frontend will not work on a dumb terminal, an emacs shell buffer, or without a controlling terminal.)
debconf: falling back to frontend: Readline
debconf: unable to initialize frontend: Readline
debconf: (This frontend requires a controlling tty.)
debconf: falling back to frontend: Teletype
Running in chroot, ignoring request.
invoke-rc.d: policy-rc.d denied execution of restart.

@xumia
Copy link
Collaborator Author

xumia commented May 22, 2024

@xumia At least in the Broadcom build (likely other platforms as wel), it doesn' t look like the FIPS version of openssh is getting built. From the logs:

[ building ] [ target/debs/bookworm/openssh-server_9.2p1-2+deb12u1_amd64.deb ]

Is there some dependency that needs to be updated?

Edit: never mind, it's getting packaged in:

+ sudo dpkg --root=./fsroot-broadcom -i target/debs/bookworm/openssh-sftp-server_9.2p1-2+deb12u2+fips_amd64.deb
Selecting previously unselected package openssh-sftp-server.
(Reading database ... 51526 files and directories currently installed.)
Preparing to unpack .../openssh-sftp-server_9.2p1-2+deb12u2+fips_amd64.deb ...
Unpacking openssh-sftp-server (1:9.2p1-2+deb12u2+fips) ...
Setting up openssh-sftp-server (1:9.2p1-2+deb12u2+fips) ...
+ sudo dpkg --root=./fsroot-broadcom -i target/debs/bookworm/openssh-server_9.2p1-2+deb12u2+fips_amd64.deb
Selecting previously unselected package openssh-server.
(Reading database ... 51530 files and directories currently installed.)
Preparing to unpack .../openssh-server_9.2p1-2+deb12u2+fips_amd64.deb ...
Unpacking openssh-server (1:9.2p1-2+deb12u2+fips) ...
Setting up openssh-server (1:9.2p1-2+deb12u2+fips) ...
debconf: unable to initialize frontend: Dialog
debconf: (Dialog frontend will not work on a dumb terminal, an emacs shell buffer, or without a controlling terminal.)
debconf: falling back to frontend: Readline
debconf: unable to initialize frontend: Readline
debconf: (This frontend requires a controlling tty.)
debconf: falling back to frontend: Teletype
Running in chroot, ignoring request.
invoke-rc.d: policy-rc.d denied execution of restart.

The FIPS version of OpenSSH will be built as one of the extra packages depended by all of the platform images.

@wumiaont
Copy link
Contributor

wumiaont commented May 22, 2024 •
edited
Loading

One observation on the openssh fips image unpack. Buildimage has non fips openssh as well as openssh fips debians together. It looks to me FIPS one is always unpacked and used. This means when FIPS is disabled, sonic still uses the openssh fips libraries/binaries. Is this by design?
I have some curiosity of how the openssh FIPS patch will work if FIPS is not enabled. Openssh fips patches microsoft-symcrypt-fips.patch where it's doing SCOSSL_ENGINE_Initialize().

zjswhhh
zjswhhh approved these changes May 22, 2024
View reviewed changes
Copy link
Contributor

@zjswhhh zjswhhh left a comment

Choose a reason for hiding this comment

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

lgtm

@xumia
Copy link
Collaborator Author

xumia commented May 23, 2024

One observation on the openssh fips image unpack. Buildimage has non fips openssh as well as openssh fips debians together. It looks to me FIPS one is always unpacked and used. This means when FIPS is disabled, sonic still uses the openssh fips libraries/binaries. Is this by design? I have some curiosity of how the openssh FIPS patch will work if FIPS is not enabled. Openssh fips patches microsoft-symcrypt-fips.patch where it's doing SCOSSL_ENGINE_Initialize().

For the first question, when INCLUDE_FIPS is set, it is by design to use the libraries.
For the second question, when INCLUDE_FIPS is set, but FIPS is disabled in the runtime, we should not initialize the SCOSSL_ENGINE_Initialize. Maybe we can simply remove the microsoft-symcrypt-fips.patch, since the ENGINE_load_builtin_engines has already called in the OpenSSH ssh_libcrypto_init, the SymCrypt will be loaded if FIPS enabled. I will have a test. @wumiaont, it is a good catch for fips disabled in the runtime case, thanks.

@wumiaont
Copy link
Contributor

@qiluo-msft Please help to review. Thanks.

@lguohan lguohan merged commit 65c4a7d into sonic-net:master May 23, 2024
20 checks passed
@zhangyanzhao zhangyanzhao mentioned this pull request May 30, 2024
Closed
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zjswhhh zjswhhh zjswhhh approved these changes

@saiarcot895 saiarcot895 saiarcot895 approved these changes

@qiluo-msft qiluo-msft Awaiting requested review from qiluo-msft qiluo-msft is a code owner

@lguohan lguohan Awaiting requested review from lguohan lguohan is a code owner

Assignees
No one assigned
Labels
Bookworm
Projects
SONiC 202405 Release
Status: ✅ Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@xumia @k-v1 @wumiaont @saiarcot895 @zjswhhh @lguohan