Add domain proxy to buildah-oci-ta #1822
Conversation
There was a problem hiding this comment.
While the changes here look rather minimal, it feels like it changes the core functionality of the task. Would it not be better to just Kustomize the task (or to use some form of task generation process like we do for the remote builds) to modify the normal buildah tasks and create new tasks for this use case?
| - name: BUILD_IMAGE | ||
| description: The buildah image to use. | ||
| type: string | ||
| default: quay.io/konflux-ci/buildah-task:latest@sha256:b2d6c32d1e05e91920cd4475b2761d58bb7ee11ad5dff3ecb59831c7572b4d0c |
There was a problem hiding this comment.
We used to have a parameter like this, but we removed it in #792. If there is a legitimate use case where we need to enable this to be customized then we need to ensure that we have an EC policy preventing it by default.
Is there a reason why you wouldn't just prefer to make an additional task to update this image with Kustomize? Presumably your use cases would only work with a very particular image ... and that image should be kept up to date?
There was a problem hiding this comment.
That's correct that the use case only works with a particular image. The image will indeed be kept up to date.
Happy to look into using kustomize instead to change the image, however...
I believe the reason we don't want to create an additional task is because we'd have to keep the script in sync with builah-oci-ta, which has been a real pain for us the past few months as we've been developing our solutions for Middleware (PNC).
Please correct me if I'm misunderstanding your suggestion!
There was a problem hiding this comment.
There is large script block where the domain proxy integrates, I don't thing it can be manipulated by Kustomize.
@arewm can you point us to the "task generation process like we do for the remote builds" ?
| - name: ENABLE_DOMAIN_PROXY | ||
| description: Determines if domain proxy will be used when hermetic mode is enabled. | ||
| type: string | ||
| default: "false" | ||
| - name: DOMAIN_PROXY_BYTE_BUFFER_SIZE | ||
| description: The byte buffer size to use for the domain proxy. | ||
| type: string | ||
| default: 32768 | ||
| - name: DOMAIN_PROXY_DOMAIN_SOCKET | ||
| description: The domain socket to use for the domain proxy. | ||
| type: string | ||
| default: /tmp/domain-socket.sock | ||
| - name: DOMAIN_PROXY_CONNECTION_TIMEOUT | ||
| description: The connection timeout in milliseconds to use for the domain proxy. | ||
| type: string | ||
| default: 10000 | ||
| - name: DOMAIN_PROXY_IDLE_TIMEOUT | ||
| description: The idle timeout in milliseconds to use for the domain proxy. | ||
| type: string | ||
| default: 30000 | ||
| - name: DOMAIN_PROXY_TARGET_ALLOWLIST | ||
| description: Comma separated list of allowed target hosts for the domain proxy. | ||
| type: string | ||
| default: "" | ||
| - name: DOMAIN_PROXY_ENABLE_INTERNAL_PROXY | ||
| description: Determines if internal proxy will be used when domain proxy is enabled. | ||
| type: string | ||
| default: "false" | ||
| - name: DOMAIN_PROXY_INTERNAL_PROXY_HOST | ||
| description: Host of proxy used internally by the domain proxy. | ||
| type: string | ||
| default: "" | ||
| - name: DOMAIN_PROXY_INTERNAL_PROXY_PORT | ||
| description: Port of proxy used internally by the domain proxy. | ||
| type: string | ||
| default: "" | ||
| - name: DOMAIN_PROXY_INTERNAL_PROXY_USER | ||
| description: User of proxy used internally by the domain proxy. | ||
| type: string | ||
| default: "" | ||
| - name: DOMAIN_PROXY_INTERNAL_PROXY_PASSWORD | ||
| description: Password of proxy used internally by the domain proxy. | ||
| type: string | ||
| default: "" | ||
| - name: DOMAIN_PROXY_INTERNAL_NON_PROXY_HOSTS | ||
| description: Comma separated list of target hosts that bypass the proxy used internally by the domain proxy. | ||
| type: string | ||
| default: "" | ||
| - name: DOMAIN_PROXY_HTTP_PORT | ||
| description: The HTTP port to use for the domain proxy. | ||
| type: string | ||
| default: 8080 |
There was a problem hiding this comment.
Which of these values have to potential to affect the security posture of the buildah task? Those parameters need to have a policy to limit the allowable values.
There was a problem hiding this comment.
Currently the values of the following are unknown so we can't limit them:
- DOMAIN_PROXY_TARGET_ALLOWLIST
- DOMAIN_PROXY_INTERNAL_PROXY_HOST
- DOMAIN_PROXY_INTERNAL_PROXY_PORT
- DOMAIN_PROXY_INTERNAL_PROXY_USER
- DOMAIN_PROXY_INTERNAL_PROXY_PASSWORD
- DOMAIN_PROXY_INTERNAL_NON_PROXY_HOSTS
They're also subject to change according to the build context, e.g. some will be obtained on the fly from the pipeline (potentially from components deployed as part of pipeline).
The other parameters can technically be removed (except for ENABLE_DOMAIN_PROXY), as the defaults within the Domain Proxy are work just fine for current test scenarios. But there may come a point where the following may need to be changed:
- DOMAIN_PROXY_BYTE_BUFFER_SIZE
- DOMAIN_PROXY_IDLE_TIMEOUT
- DOMAIN_PROXY_HTTP_PORT
There was a problem hiding this comment.
In terms of security, we probably need a generic solution where the task can be used, not just for the PNC integration. Maybe this is just a matter of terminology what operation modes this task provides.
There already is a HERMETIC parameter, which allows users to run the build with or without isolation. We are essentially adding a 3rd option here, a "controlled" internet access.
The parameters DOMAIN_PROXY_INTERNAL_PROXY_* are used to define a "tracking proxy" and the domain proxy guarantees that all the request from the isolated environment go through this tracking proxy. Except the hosts listed in DOMAIN_PROXY_INTERNAL_NON_PROXY_HOSTS.
Then it's up to users what king of "tracking proxy" they use, if the "tracking proxy" used does not track anything, the build is pretty similar to a non hermetic build. In case of PNC integration, users won't have access to manipulate this parameters.
An option to make this case more clear is to change the HERMETIC (true|false) parameter to NETWORK_ISOLATION (HERMETIC|CONTROLLED|NONE). And make the domain proxy activation conditional based on this parameter.
|
|
||
| # With expansion | ||
| cat >> /app/build-script.sh << EOF | ||
| $command |
There was a problem hiding this comment.
Does this run with the domain proxy while also having the network restricted from the build? I haven't tested this out, but it seems like the domain proxy would not have any value if there is no connectivity allowed in the first place.
There was a problem hiding this comment.
Good question. I should have been more clear in my PR description.
There are two components.
- Domain Proxy Client. This runs inside the network restricted environment.
- Domain Proxy Server. This runs outside the network restricted environment and performs the external calls to allowed hosts.
The components interact via Unix domain socket connection.
See the Domain Proxy readme here: https://github.com/project-ncl/domain-proxy
There was a problem hiding this comment.
This is how the domain proxy provides a controller access to the internet from the unshare restricted namespace:
Middleware (PNC) needs Domain Proxy to prevent downloading artifacts from unauthorised hosts as part of build process.
BUILD_IMAGEparameter is needed in order to override the buildah-task image with a Domain Proxy image containingdomain-proxy-serveranddomain-proxy-clientbinaries. Domain Proxy image is built on top of latest buildah-task image.A better solution might be to mount the
domain-proxy-serveranddomain-proxy-clientbinaries in a volume. But then it's unknown how publishing them might work?Domain Proxy parameters and environment variables could be reduced (with some discussion) to only those whose values are likley to change, as most of them have sensible defaults within the Domain Proxy code.
The Domain Proxy consists of two components detailed here
https://issues.redhat.com/browse/JBS-82