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

feat: add openssl module #29

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

feat: add openssl module #29

wants to merge 2 commits into from

Conversation

pvalena
Copy link
Collaborator

@pvalena pvalena commented Jan 20, 2025

install() {
local ossl_files="${dracutbasedir}/ossl-files"

local openssl_cnf="$($ossl_files --config)"

Check warning

Code scanning / shellcheck

SC2155 Warning

Declare and assign separately to avoid masking return values.
/etc/pki/tls/openssl.d/* \
/etc/crypto-policies/back-ends/opensslcnf.config \
/usr/lib64/ossl-modules/*.so \
$($ossl_files --engines --providers)

Check warning

Code scanning / shellcheck

SC2046 Warning

Quote this to prevent word splitting.
$($ossl_files --engines --providers)

mkdir -p "${initrd_openssl_cnf%/*}"
${dracutbasedir}/ossl-config >"${initrd_openssl_cnf}"

Check warning

Code scanning / shellcheck

SC2086 Warning

Double quote to prevent globbing and word splitting.
@pvalena
Copy link
Collaborator Author

pvalena commented Feb 14, 2025

@neverpanic PTAL.

COPR build: https://copr.fedorainfracloud.org/coprs/pvalena/dracut-rhel10/build/8653837/
Test LOG: https://gist.github.com/pvalena/10060fb5c64a4bfea872013b5e196ead#file-test_dracut_openssl_rhel-10-x86_64_2025-02-14-log-L109 (initrd build with set -x)

AFAIR I did only one change to src/ossl/Makefile to use the distro default CFLAGS.

I have not tested any runtime in initrd (should be same as before; the included files did not change).

Comment on lines +20 to +21
/etc/pki/tls/openssl.cnf \
/etc/pki/tls/openssl.d/* \
Copy link
Contributor

Choose a reason for hiding this comment

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

These can be removed, the invocation of ossl-config makes sure that it exists and does not use any includes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks; I wasn't sure about that. Shloudn't I keep it just for redundancy though?

Copy link
Contributor

Choose a reason for hiding this comment

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

I wouldn't. A modified /etc/pki/tls/openssl.cnf could contain .include /opt/foobar/module.cnf, which we wouldn't copy, and could case an issue. .include /etc/pki/tls/openssl.d/ isn't the only include statement that could be in this file.

For that reason, I'd also remove the openssl.d directory. It shouldn't be needed anymore.

/etc/pki/tls/openssl.cnf \
/etc/pki/tls/openssl.d/* \
/etc/crypto-policies/back-ends/opensslcnf.config \
/usr/lib64/ossl-modules/*.so \
Copy link
Contributor

Choose a reason for hiding this comment

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

Technically redundant, since the line after that will contain the same files. If we're aiming for the minimal initramfs, we should probably remove this line to save some space.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, exactly my thoughts :). Thanks for confirmation.

install() {
local ossl_files="${dracutbasedir}/ossl-files"

local openssl_cnf="$($ossl_files --config)"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
local openssl_cnf="$($ossl_files --config)"
local openssl_cnf="$($ossl-files --config)"

As I understand the Makefiles, you installed this as ossl-files. The same applies below.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't have it in PATH, but I have it in dracut dir, so this is a shell variable instead, difined two lines above this one :). Sorry for the confusion.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, you're right. I was confused because I missed the second $ in front.

Technically, this should be "$("$ossl_files" --config)", then, though. Although it's unlikely that there ever will be a space in $ossl_files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants