-
Notifications
You must be signed in to change notification settings - Fork 174
Adding support for Custom CA Certificates during system build #2913
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
base: main
Are you sure you want to change the base?
Conversation
|
Hey @rdoxenham this is great work thank you very much 👍 I hope you don't mind I pushed a commit to fix the tests Other than that there is a bit of "metadata" to complete the change. Can you please do the following
Would be great thanks in advance |
|
This needs a stanza in the XML too. |
|
@rdoxenham ah sorry I saw you already added the docs topics... nevermind forget what I said :) |
I agree, it could also be added in a followup PR though |
I actually have some questions about this from #2721... do you expect for it to be something like: Otherwise, I'm curious as to what would be placed into each entry other than a path? |
Yes it would be good to add this new <certificates>
<certificate path="/some/path"/>
</certificates>We should allow for more than one entry though. In the commandline implementation we allow |
Are you ok with this being a follow-up change? |
|
I would prefer it to be part of this change. |
Understood. Can you and @schaefi help me with expected behaviour, please? We're introducing three different ways of specifying additional CA certificates to import:
In the current PR, (1) is favoured over (2). How do you envisage this happening with (3)? The way I'm thinking about it is still one or the other, in that if either (1) or (3) are specified, they override (2), yet if both (1) and (3) are specified then they're combined into a single list. Written more easily:
Thoughts? Is the original plan of ignoring |
|
None of them should be ignored. It's combinatorial. |
|
@rdoxenham Thinking more about it I believe the implementation to read certificates from the the kiwi runtime config might be an uncommon place. The
The two source of information should be combined. We have similar implementation already in kiwi for example for the handling of the repositories. You can specify them on the commandline and also in the image description. From an implementation perspective the way how we approach this is as follows:
When doing it that way you also automatically set the precedence for the data which is that the value from the command line may overwrite the data read from the image description I hope this helps to make you started. Thanks much |
|
@rdoxenham I hope you don't mind, I have added new commits that implements the suggested change from my last comment. Would be great if you can review the code changes and share your thoughts about it. With the latest changes we can now specify an optional
Thanks |
f5fd8ae to
0c09ac8
Compare
|
I we agree this is good to go, feel free to squash commits into one |
0c09ac8 to
94f0828
Compare
| <certificates> | ||
| <certificate path="/some/path"/> | ||
| </certificates> |
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.
Wouldn't it make sense to list files instead of directories? I don't like the idea of just wholesale importing random files.
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.
It's not importing random files. @rdoxenham has implemented it in a way that the files from the directory are checked against their prefix to match as a certificate file. I'd like to get feedback from Rhys first to see if that directory import is based on some design pattern.
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.
If there is no strong design pattern involved here I agree with you that specifying single files is better and more specific
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 problem is that even if there was, that pattern is localized to Rhys' setup. As this would be available to everyone, that is a risky proposition.
Plus, usually people import cert bundle files or specialty certs (like a local intranet certificate).
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.
ok I believe this is enough risk analysis to add a commit that moves this implementation to file based cert setup. But now let's wait for @rdoxenham as I have touched his code a lot :)
Thanks
This commit adds support for providing custom CA certificates during the build process. It allows a user to specify a path to a local directory, either by updating `kiwi.yml` with `ca_certificates:path`, or by passing `--ca-certificates=<directory>` via the command line. The code looks for all `*.pem`, `*.crt`, and `*.crt` files in the specified directory and imports them immediately post bootstrap (where the required CA update tools are available), but before any further packages are retrieved, solving for situations where the chroot environment needs certificates, e.g. when there's a proxy server in the build environment. Reference: OSInside#2721
Rename commandline option to ca-cert-path and allow it to be specified multiple times. Update setup_ca_certificates to work with a list
In addition to the commandline handling, the certificates should also be able to get specified in the image description
Update implementation for setup_ca_certificates to read from xml_state.get_certificates() only. In addition update the handling of the commandline argument ca-cert-path to add, create or overwrite into the image description data structure
Instead of dir based, only allow for file based cert setup. This commit was done due to review comments stating issues when syncing arbitrary directory content
e195584 to
293ab84
Compare
|
Hey @schaefi! Happy New Year :) my apologies for the slow reply to this. Thanks so much for taking the action to follow-up on this; I had planned on getting to it at some point in the first few weeks of 2026, but I'm very glad to see that you've made the progress, so I certainly do not mind you doing this 👍 As @Conan-Kudo suggests, the pattern that I was originally targeting was to more easily support our containerised Kiwi workflow; in this instance, we'd expect the user to simply bind mount a directory containing their certificates to the container, and we'd iterate over all certificate files found (I specifically chose crt/pem/cer files for this). However, I can see that this may introduce some risks. I think the path proposed here with files rather than directories can also work; it means a little more effort required for the containerised workflow, but I also see this as a fairly rare use-case, so I don't think it should become a blocker as far as I'm concerned. Thanks again! |
This commit adds support for providing custom CA certificates during the build process. It allows a user to specify a path to a local directory, either by updating
kiwi.ymlwithca_certificates:path, or by passing--ca-certificates=<directory>via the command line.The code looks for all
*.pem,*.crt, and*.crtfiles in the specified directory and imports them immediately post bootstrap (where the required CA update tools are available), but before any further packages are retrieved, solving for situations where the chroot environment needs certificates, e.g. when there's a proxy server in the build environment.Reference: #2721