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
ipxe: Add options to specify trusted root certificate and enable image trust management #52593
Conversation
…e trust management These changes are necessary for the "Trusted root certificates" and "Code signing" sections of ipxe's Cryptography configuration guide (http://ipxe.org/crypto). "DOWNLOAD_PROTO_HTTPS" is defaulted to `true` for backward compatibility (it was hard-coded before).
I tested the resulting |
|
||
|
||
enabledOptions = [ "DOWNLOAD_PROTO_HTTPS" ]; | ||
enabledOptions = lib.optional optionDownloadProtoHttps "DOWNLOAD_PROTO_HTTPS" |
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.
Hmm why add an option for it?
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.
I thought rather than arbitrarily hard-coding certain options while exposing others, it might be better to allow each supported option to be set by the user. I defaulted this option to its previous value (enabled), so there should be no difference from the perspective of current users of this module.
@@ -27,10 +30,12 @@ stdenv.mkDerivation { | |||
[ "ECHO_E_BIN_ECHO=echo" "ECHO_E_BIN_ECHO_E=echo" # No /bin/echo here. | |||
"ISOLINUX_BIN_LIST=${syslinux}/share/syslinux/isolinux.bin" | |||
"LDLINUX_C32=${syslinux}/share/syslinux/ldlinux.c32" | |||
] ++ lib.optional (embedScript != null) "EMBED=${embedScript}"; | |||
] ++ lib.optional (trustedRootCert != null) "TRUST=${trustedRootCert}" | |||
++ lib.optional (embedScript != null) "EMBED=${embedScript}"; |
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.
CERT=
is missing from this list, so you cant embed a cert into the binary
|
||
|
||
enabledOptions = [ "DOWNLOAD_PROTO_HTTPS" ]; | ||
enabledOptions = lib.optional optionDownloadProtoHttps "DOWNLOAD_PROTO_HTTPS" | ||
++ lib.optional optionImageTrustCmd "IMAGE_TRUST_CMD"; |
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.
could maybe also add CONSOLE_SERIAL
and POWEROFF_CMD
as optional commands
Thank you for your contributions.
|
Motivation for this change
These changes are necessary for the "Trusted root certificates" and
"Code signing" sections of ipxe's Cryptography configuration guide
(http://ipxe.org/crypto).
"DOWNLOAD_PROTO_HTTPS" is defaulted to
true
for backwardcompatibility (it was hard-coded before).
Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)nix path-info -S
before and after)