Conversation
|
Skipping CI for Draft Pull Request. |
There was a problem hiding this comment.
Code Review
This pull request adds a test container fips-nginx to verify file downloads over HTTPS with FIPS-approved cryptography. A thorough security audit found no significant vulnerabilities, though a low-risk issue regarding input validation in build.sh was noted. However, a critical issue in the Containerfile will prevent the container from building, and several suggestions have been made to improve the build.sh script's robustness and cleanliness.
fips.enable.https
a9874f0 to
9c01bda
Compare
485b749 to
98fbbae
Compare
|
Hold on, looks this can only test x86_64, as I just setup the env on x86_64. Do we want to run on all multiarch? |
Hum, I don't see anything x86_64 specific here. This should work on all arches but I'm also OK testing just on x86_64 as the arch should not matter here. |
|
Thanks, this looks really good. The main nit that I have is that we should be running a RHEL based container as only those can be FIPS compliant. Now we don't really want to leave credentials on the server on GCP so we would have to work around that by:
That should get us very close to a "compliant" deployment. Edit: fixed the order to build your system and then upload the final image to the server on GCP. |
To verify that using TLS works in FIPS mode by having Ignition fetch a remote resource over HTTPS with FIPS compatible algorithms. tests/containers: build the container based on `nginx-126:10.1`. See Timothee's comment coreos#4477 (comment) Fixes https://issues.redhat.com/browse/COS-3487
To verify that using TLS works in FIPS mode by having Ignition fetch a remote resource over HTTPS with FIPS compatible algorithms. tests/containers: build the container based on `nginx-126:10.1`. See Timothee's comment coreos#4477 (comment) Fixes https://issues.redhat.com/browse/COS-3487
To verify that using TLS works in FIPS mode by having Ignition fetch a remote resource over HTTPS with FIPS compatible algorithms. tests/containers: build the container based on `nginx-126:10.1`. See Timothee's comment coreos#4477 (comment) Fixes https://issues.redhat.com/browse/COS-3487
To verify that using TLS works in FIPS mode by having Ignition fetch a remote resource over HTTPS with FIPS compatible algorithms. tests/containers: build the container based on `nginx-126:10.1`. See Timothee's comment coreos#4477 (comment) Fixes https://issues.redhat.com/browse/COS-3487
To verify that using TLS works in FIPS mode by having Ignition fetch a remote resource over HTTPS with FIPS compatible algorithms. tests/containers: build the container based on `nginx-126:10.1`. See Timothee's comment coreos#4477 (comment) Fixes https://issues.redhat.com/browse/COS-3487
To verify that using TLS works in FIPS mode by having Ignition fetch a remote resource over HTTPS with FIPS compatible algorithms. tests/containers: build the container based on `nginx-126:10.1`. See Timothee's comment coreos#4477 (comment) Fixes https://issues.redhat.com/browse/COS-3487
|
@HuijingHei: new pull request created: #4490 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
To verify that using TLS works in FIPS mode by having Ignition fetch a remote resource over HTTPS with FIPS compatible algorithms. tests/containers: build the container based on `nginx-126:10.1`. See Timothee's comment coreos#4477 (comment) Fixes https://issues.redhat.com/browse/COS-3487
|
@HuijingHei: new pull request created: #4491 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
Hey @HuijingHei.. I'm sorry I didn't pay attention to this PR when it was opened. I'm interested to know if this test covers more than the test that was added in #4373 - i.e. do they test the same things? Is this new test really needed? |
A little different with #4373, this is to ensure Ignition can fetch a remote resource over HTTPS with FIPS compatible algorithms. Maybe @travier has more context about this. Copy the steps from comment, hope this can be helpful.
|
this implies that there are requirements on the remote server for properly testing FIPs. Is that correct? I don't see how just enabling FIPs on the CoreOS instance we are testing doesn't ensure FIPs compliant crypto algorithms were used. |
|
Wait, I'm confused why this needs a permanent test fixture VM running in the cloud. Couldn't the test just provision two VMs? See e.g. the tang test which is very similar (sets up a service on the first VM, then Ignition queries that service on the other)? As for testing itself, I think we really want to test the negative case too. One idea to not actually pay for the cost of a separate test is:
Then we verify that the error is what we expect (failure to fetch the file resource). |
Yeah. That definitely is a question I have too.
probably.
I think what you are saying is that #4373 covers the positive test case such that the test coverage with this But you are also saying that the work in this PR could be repurposed to set up a "negative" test with a web server that only allows non-FIPS-compliant algorithms and verify it fails? Is all of that correct? |
IMU, the permanent VM could make the test simpler, but I can try with two VMs. Another thing is |
Yes, FIPS is not just a configuration on the client. Both client & server have to be FIPS enabled. In #4373, I don't know what the server in AWS is used as a configuration and if it only accepts FIPS approved algorithms.
Yes, this might indeed be possible but it would be much more difficult as we need to run FIPS enabled RHCOS on both sides and pass credentials to pull a RHEL nginx container image or build it in CI where here we built it once.
Having an negative test would indeed be good idea but I'm not sure about mixing both into a single one. That might make it harder to figure why it the test fails if there is an issue. Once we have the remote server, spinning one VM for each test isn't that much work. |
|
Ah, and it's not enough to generate the cert in CI, it has to be generated on a FIPS enabled system so it would have to be generated in the first VM. So the flow would look like:
|
We don't need to generate the cert in the container image, we can generate it anywhere as long as the host is FIPS enabled and then pass it to the container at runtime using volume mounts. |
Then in our CI env, how to save the generated cert files (e.g. fips-server.crt, fips-server.key)? |
But the client is configured to only request FIPs approved algorithms (otherwise the kernel would throw an error), right? Let's say that you are right, both client and server need to be FIPs enabled and won't work if not. What do FIPs enabled RHEL servers do for updates? Do they reach out to Red Hat's services and does that work? If so then we can assume stuff from Red Hat is FIPs enabled and find a URL of some artifact from there to download? Then we don't need to set up the second VM (unless we really want to do the negative test). FTR when I did #4373 I confirmed that the test failed prior to the Ignition fix so it's definitely testing something. |
The nginx instance can serve the cert itself and the client Ignition config can fetch it via its |
Either you generate them on the host where you're going to run the container (so this way it should be FIPS enabled), or you generate them once on a FIPS enabled RHCOS VM and directly include them here in the test. It does not matter if the key is not secret, that's not what we are testing. |
It's not the kernel that would fail but something in openssl. But yes, ideally that would be enough but I don't know enough to be sure. I could only find https://www.ibm.com/docs/en/rpa/30.0.x?topic=2-enabling-fips:
We'll have to ask Clement again.
I don't know and that's a good idea indeed. There are likely FIPS enabled and publicly accessible servers at Red Hat somewhere.
OK, so it would be good to try if the test fails with a FIPS nginx server but a non-FIPS RHCOS as Jonathan suggested. |
Post the test results: Test side: start test VM with fips enabled
Doing tests on server side with none FIPs enabled, or with FIPs enabled, the result is the same. So this proves Dusty is correct, it does not matter if server side is FIPs enabled or not. Or maybe I am wrongly understanding this. use Note that both the server and client are using rhcos-9.8.20260314-0-qemu.x86_64.qcow2, as rhcos-10.2 does not have |
If I'm right then this PR as merged is just a duplicate test of #4373 If we decided we wanted a negative test we could either reuse the work you did to set up a separate server or just find an existing server somewhere that doesn't allow FIPs compliant algorithms. |
I have no objection for the negative test, maybe can refer coreos.ignition.security.tls to setup 2 VMs to test.
|
To verify that using TLS works in FIPS mode by having Ignition
fetch a remote resource over HTTPS with FIPS compatible algorithms.
tests/containers: build the container based on
nginx-126:10.1.See Timothee's comment #4477 (comment)
Fixes https://issues.redhat.com/browse/COS-3487