Bug 1895053: Allow builds to mount the proxy CA #831
Conversation
openshift-ci-robot
added
bugzilla/severity-high
|
@adambkaplan: This pull request references Bugzilla bug 1895053, which is valid. The bug has been updated to refer to the pull request using the external bug tracker. 3 validation(s) were run on this bug
In 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/test-infra repository. |
build/v1/types.go
Outdated
| // true, the contents of `/etc/pki/ca-trust` within the build are managed by the build | ||
| // container and cannot be modified during the build (example - via a Dockerfile `RUN` | ||
| // instruction). | ||
| MountProxyTrustedCA *bool `json:"mountProxyTrustedCA,omitempty" protobuf:"varint,10,opt,name=mountProxyTrustedCA"` |
There was a problem hiding this comment.
Pointer is used here in the event we want to support build defaults, which requires us to determine user intent.
There was a problem hiding this comment.
i wonder if we should just call this MountTrustedCA? we have been a bit wishy-washy on the concept, in some places we call it trusted CA, in others proxy CA. What are the other APIs a user/admin interacts with that would setup/populate the CAs that ultimately get mounted here?
|
Usage by openshift-controller-manager: openshift/openshift-controller-manager#154 |
|
Test to verify: openshift/origin#25778 |
build/v1/types.go
Outdated
| // as defined in the cluster's proxy configuration, into the build. When this field is set to | ||
| // true, the contents of `/etc/pki/ca-trust` within the build are managed by the build | ||
| // container and cannot be modified during the build (example - via a Dockerfile `RUN` | ||
| // instruction). |
There was a problem hiding this comment.
they can be modified, but the files (neither the original nor the modified versions) will be included in the output image.
the output image will contain whatever contents the original from image included for /etc/pki/ca-trust
build/v1/types.go
Outdated
| // | ||
| // When this field is set to true, the contents of `/etc/pki/ca-trust` within the build are | ||
| // managed by the build container, and any changes to this directory or its subdirectories (for | ||
| // example - via a Dockerfile `RUN` instruction) are not persisted in the build's output image. |
|
@adambkaplan: GitHub didn't allow me to request PR reviews from the following users: rolfedh. Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs. In 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/test-infra repository. |
adambkaplan
force-pushed
the
build-mount-cas
branch
from
c41c760
to
7faff4e
Compare
|
@adambkaplan: This pull request references Bugzilla bug 1895053, which is valid. 3 validation(s) were run on this bug
In 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/test-infra repository. |
adambkaplan
force-pushed
the
build-mount-cas
branch
from
7faff4e
to
0aea0b5
Compare
adambkaplan
changed the title
openshift-ci-robot
added
the
do-not-merge/work-in-progress
Add option that allows the cluster's proxy certificate authority to be mounted via a buildah bind mount. When the CA is bind mounted, the contents of `/etc/pki/ca-trust` are managed by the build container. The folder and its subdirectories can be manipulated; however any changes to the directory or its subdirectories are not persisted in the resulting image. For example, with this option set to true, a Dockerfile `RUN` instruction that changes the ownership of `/etc/pki/ca-trust/extracted` will not have the updated ownership appear in the build's output image.
adambkaplan
force-pushed
the
build-mount-cas
branch
from
0aea0b5
to
4aa72ae
Compare
adambkaplan
changed the title
openshift-ci-robot
removed
the
do-not-merge/work-in-progress
|
/assign @bparees Feature now works end to end with my WIP pull requests. |
|
cc @openshift/api-reviewers |
|
/approve |
openshift-ci-robot
added
the
approved
|
Trying to understand the downstream impact of this on builds. The impl looks to be doing a mount from inside of a container, which I expect will run afoul of attempts to limit the scope of pod security permissions. Could this not be modeled as a localvolume mount in the pod so the kubelet can manage the mounting and we can properly permission it? |
|
@deads2k in the implementation the trust bundle is mounted into the sub-containers buildah creates during the build [1]. These sub containers live within the build pod, inside the The path to getting the trust bundle into builds is as follows:
This lets custom PKIs be trusted for git clone and image source extraction operations. However, things running within the build itself (example - [1] https://github.com/openshift/builder/pull/218/files#r574068125 |
|
A proxy CA should not be confused with, "use this CA to trust all connections". It isn't used for that and usage is not uniform. It's also not projected into any user-facing pods today. If you want to project a "system" trust bundle of some kind into a pod, that would be a reasonable enhancement to write. If you want to support "use the cluster proxy for my build", this is also reasonable, but I would expect that to be an option that talks about "use-cluster-proxy", somehow handles cases where credentials are included in the env vars, and properly sets those env vars. This appears to be an attempt at something in the middle that uses a proxy CA bundle in a non-standard way and doesn't include the proxy env vars? |
I believe you can project a system trust bundle by adding the Instead, builds effectively generate their own system trust bundle by adding the proxy CA as an anchor and merging the trust stores together.
We also include the proxy env vars in builds. If the cluster has a proxy configured, the What may be of concern is that builds can override the proxy env vars, in which case the proxy CA could be abused as a "use this CA for everything" anchor. We do this today, and IMO should be treated as a separate bug. [1] https://docs.openshift.com/container-platform/4.6/networking/configuring-a-custom-pki.html#certificate-injection-using-operators_configuring-a-custom-pki |
| // When this field is set to true, the contents of `/etc/pki/ca-trust` within the build are | ||
| // managed by the build container, and any changes to this directory or its subdirectories (for | ||
| // example - within a Dockerfile `RUN` instruction) are not persisted in the build's output image. | ||
| MountTrustedCA *bool `json:"mountTrustedCA,omitempty" protobuf:"varint,10,opt,name=mountTrustedCA"` |
There was a problem hiding this comment.
MountClusterProxyCA if that's what you're mounting (I think it is)
|
|
||
| // mountTrustedCA bind mounts the cluster's trusted certificate authorities, as defined in | ||
| // the cluster's proxy configuration, into the build. This lets processes within a build trust | ||
| // components signed by custom PKI certificate authorities, such as private artifact |
There was a problem hiding this comment.
it's not appropriate to indicate that a proxy CA is used to validate private artifact repositories. that is more of a bug than a feature.
There was a problem hiding this comment.
To piggy back on Ben's comment - unfortunately we explicitly tell customers today to use the Proxy "trusted CA" as the trusted CA for the whole cluster [1]. Connecting to a private Artifactory or Nexus repository with a corporate cert is perhaps the most common reason why a customer would use this feature.
[1] https://docs.openshift.com/container-platform/4.6/networking/configuring-a-custom-pki.html
There was a problem hiding this comment.
unfortunately we explicitly tell customers today to use the Proxy "trusted CA" as the trusted CA for the whole cluster
it's not unfortunate, it was a deliberate choice that we built a mechanism to distribute the CAs generically, not specifically for proxy.
The mistake was that you have to tie the trusted CA bundle configmap to the proxy config (via a ref in the proxyconfig to the ca bundle CM) to make that happen, which makes it seem like the CAs are explicitly tied to proxy.
we said at the time we did this that if we got pushback/uses cases from customers to more finely subdivide their CA bundles, we would add additional mechanisms/configs to support that subdivision. So far i haven't heard of any.
| // When this field is set to true, the contents of `/etc/pki/ca-trust` within the build are | ||
| // managed by the build container, and any changes to this directory or its subdirectories (for | ||
| // example - within a Dockerfile `RUN` instruction) are not persisted in the build's output image. | ||
| MountTrustedCA *bool `json:"mountTrustedCA,omitempty" protobuf:"varint,10,opt,name=mountTrustedCA"` |
There was a problem hiding this comment.
If you want a way to manage trust, why not create a way to mount a configmap and use that? That seems both more useful and could be consistent with the proxy use-case.
the annotation that projects the CAs into the configmap was named Those CAs are already used for more than proxy (they are also used to trust registries that have custom certs, both for disconnected installs and at runtime for imagestream imports and image pulls). Does this have security implications admins need to understand/be aware of? yes. But until/unless we design additional CA injection mechanisms that distinguish different sets of CAs(which there was no appetite for at the time the current CA distribution mechanism was defined, hence why it was created as generic), it's what we have and i think it's reasonable for the build component to leverage it in this way, as other cluster components already do. |
|
I'm not personally impressed or convinced by using a questionable previous decision to justify continued questionable choices. I also don't have confidence that it will stay contained to builds in the future. Add to that the fact that we know it's not difficult to create viable alternative that reflects what you actually want if you can describe the boundaries on a particular injection and usage mechanism, it seems like a real shame to do this. But I won't actively attempt to block it. |
The build api does already provide ways to supply explicit CAs to be used, on a per build basis, for someone who wants to constrain the use of the CAs. This is about making builds follow the pattern used by other components in the system which is to consume the cluster trusted CA bundle, if desired. So for now this is about builds respecting the mechanism/config that exists. Yes we can certainly introduce "cluster scoped build CA bundles" in the future (along with something that is truly a proxy CA bundle, since today what we have is not a proxy CA bundle, it is just a cluster scoped trust bundle), though again the need for that is somewhat limited since builds already allow you to provide your own CAs on a per-build basis (it would be nice to have a way to do it at a cluster-scoped config level). So far i'm not aware of any users/customers requesting such a thing, but we'd entertain it (more likely in builds v2 however, as we are trying to constrain new feature additions to builds v1) |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: adambkaplan, bparees, rolfedh The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@adambkaplan: Some pull requests linked via external trackers have merged: The following pull requests linked via external trackers have not merged:
These pull request must merge or be unlinked from the Bugzilla bug in order for it to move to the next state. Once unlinked, request a bug refresh with Bugzilla bug 1895053 has not been moved to the MODIFIED state. In 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/test-infra repository. |
Add option that allows the cluster's proxy certificate authority to be
mounted via a buildah bind mount. When the CA is bind mounted, the
contents of
/etc/pki/ca-trustare managed by the build container.The folder and its subdirectories can be manipulated; however any changes
to the directory or its subdirectories are not persisted in the resulting
image.
For example, with this option set to true, a Dockerfile
RUNinstructionthat changes the ownership of
/etc/pki/ca-trust/extractedwill not havethe updated ownership appear in the build's output image.