Skip to content
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

genpolicy: support insecure registries and hermetic environments #9008

Closed
burgerdev opened this issue Feb 2, 2024 · 3 comments · Fixed by #9294
Closed

genpolicy: support insecure registries and hermetic environments #9008

burgerdev opened this issue Feb 2, 2024 · 3 comments · Fixed by #9294
Labels
enhancement Improvement to an existing feature needs-review Needs to be assessed by the team.

Comments

@burgerdev
Copy link
Contributor

Which feature do you think can be improved?

Policy generation with genpolicy. Specifically, image pulling for dm-verity calculation.

How can it be improved?

Add support for insecure registries, at least local ones. Many build systems restrict network access for security and reproducibility, and hermetic build rules are encouraged (e.g. Bazel, Nix). Local insecure registries would be an option for this use case, and they are supported by OCI tooling like docker, podman, crane, skopeo.

Additional Information

What I'm trying to do:

crane registry serve --address localhost:8080 &

curl -sLO https://raw.githubusercontent.com/kubernetes/website/main/content/en/examples/pods/simple-pod.yaml
curl -sLO https://raw.githubusercontent.com/kata-containers/kata-containers/main/src/tools/genpolicy/rules.rego
curl -sLO https://raw.githubusercontent.com/kata-containers/kata-containers/main/src/tools/genpolicy/genpolicy-settings.json

genpolicy -y simple-pod.yaml

sed -i 's|nginx:|localhost:8080/nginx:|' simple-pod.yaml

genpolicy -y simple-pod.yaml. # <--- FAILS

Error message:

thread 'main' panicked at src/registry.rs:112:17:
Failed to pull container image manifest and config - error: RequestError(
    reqwest::Error {
        kind: Request,
        url: Url {
            scheme: "https",
            cannot_be_a_base: false,
            username: "",
            password: None,
            host: Some(
                Domain(
                    "localhost",
                ),
            ),
            port: Some(
                8080,
            ),
            path: "/v2/",
            query: None,
            fragment: None,
        },
        source: hyper::Error(
            Connect,
            Ssl(
                Error {
                    code: ErrorCode(
                        1,
                    ),
                    cause: Some(
                        Ssl(
                            ErrorStack(
                                [
                                    Error {
                                        code: 167772427,
                                        library: "SSL routines",
                                        function: "ssl3_get_record",
                                        reason: "wrong version number",
                                        file: "ssl/record/ssl3_record.c",
                                        line: 354,
                                    },
                                ],
                            ),
                        ),
                    ),
                },
                X509VerifyResult {
                    code: 0,
                    error: "ok",
                },
            ),
        ),
    },
)

Before raising this enhancement request

Have you looked at the limitations document?

Yes, I did not find a relevant limitation.

@burgerdev burgerdev added enhancement Improvement to an existing feature needs-review Needs to be assessed by the team. labels Feb 2, 2024
@burgerdev
Copy link
Contributor Author

Looking at the configuration options of oci-distribution, this use case seems expected - we could do

diff --git a/src/tools/genpolicy/src/registry.rs b/src/tools/genpolicy/src/registry.rs
index 6009dad6e..2ac31b7e9 100644
--- a/src/tools/genpolicy/src/registry.rs
+++ b/src/tools/genpolicy/src/registry.rs
@@ -13,7 +13,7 @@ use anyhow::{anyhow, bail, Result};
 use docker_credential::{CredentialRetrievalError, DockerCredential};
 use log::warn;
 use log::{debug, info, LevelFilter};
-use oci_distribution::client::{linux_amd64_resolver, ClientConfig};
+use oci_distribution::client::{linux_amd64_resolver, ClientConfig, ClientProtocol};
 use oci_distribution::{manifest, secrets::RegistryAuth, Client, Reference};
 use serde::{Deserialize, Serialize};
 use sha2::{digest::typenum::Unsigned, digest::OutputSizeUser, Sha256};
@@ -62,13 +62,14 @@ pub struct ImageLayer {
 }
 
 impl Container {
-    pub async fn new(use_cached_files: bool, image: &str) -> Result<Self> {
+    pub async fn new(use_cached_files: bool, insecure_registries: Vec<String>, image: &str) -> Result<Self> {
         info!("============================================");
         info!("Pulling manifest and config for {:?}", image);
         let reference: Reference = image.to_string().parse().unwrap();
         let auth = build_auth(&reference);
 
         let mut client = Client::new(ClientConfig {
+            protocol: ClientProtocol::HttpsExcept(insecure_registries),
             platform_resolver: Some(Box::new(linux_amd64_resolver)),
             ..Default::default()
         });

However, this additional argument would need to be added to quite a few intermediate function calls. Maybe we could pass a config struct with both use_cached_files and insecure_registries?

@burgerdev burgerdev changed the title genpolicy: support insecure registries genpolicy: support insecure registries and hermetic environments Feb 7, 2024
@burgerdev
Copy link
Contributor Author

I found another source of problems when using genpolicy in hermetic environments: the pause container needs to be configurable, too.

https://github.com/kata-containers/kata-containers/blob/0174568/src/tools/genpolicy/src/pod.rs#L832-L833

@danmihai1
Copy link
Member

I found another source of problems when using genpolicy in hermetic environments: the pause container needs to be configurable, too.

https://github.com/kata-containers/kata-containers/blob/0174568/src/tools/genpolicy/src/pod.rs#L832-L833

I agree. We're in the process of adding CI test coverage for genpolicy in the main branch - e.g., see #8922. As we work through these tests, we uncover and address these kinds of dark corners.

burgerdev added a commit to burgerdev/kata-containers that referenced this issue Mar 18, 2024
genpolicy is a handy tool to use in CI systems, to prepare workloads
before applying them to the Kubernetes API server. However, many modern
build systems like Bazel or Nix restrict network access, and rightfully
so, so any registry interaction must take place on localhost.
Configuring certificates for localhost is tricky at best, and since
there are no privacy concerns for localhost traffic, genpolicy should
allow to contact some registries insecurely. As this is a runtime
environment detail, not a target environment detail, configuring
insecure registries does not belong into the JSON settings, so it's
implemented as command line flags.

Fixes: kata-containers#9008

Signed-off-by: Markus Rudy <webmaster@burgerdev.de>
@katacontainersbot katacontainersbot moved this from To do to In progress in Issue backlog Mar 18, 2024
burgerdev added a commit to burgerdev/kata-containers that referenced this issue Apr 11, 2024
genpolicy is a handy tool to use in CI systems, to prepare workloads
before applying them to the Kubernetes API server. However, many modern
build systems like Bazel or Nix restrict network access, and rightfully
so, so any registry interaction must take place on localhost.
Configuring certificates for localhost is tricky at best, and since
there are no privacy concerns for localhost traffic, genpolicy should
allow to contact some registries insecurely. As this is a runtime
environment detail, not a target environment detail, configuring
insecure registries does not belong into the JSON settings, so it's
implemented as command line flags.

Fixes: kata-containers#9008

Signed-off-by: Markus Rudy <webmaster@burgerdev.de>
burgerdev added a commit to burgerdev/kata-containers that referenced this issue Apr 11, 2024
genpolicy is a handy tool to use in CI systems, to prepare workloads
before applying them to the Kubernetes API server. However, many modern
build systems like Bazel or Nix restrict network access, and rightfully
so, so any registry interaction must take place on localhost.
Configuring certificates for localhost is tricky at best, and since
there are no privacy concerns for localhost traffic, genpolicy should
allow to contact some registries insecurely. As this is a runtime
environment detail, not a target environment detail, configuring
insecure registries does not belong into the JSON settings, so it's
implemented as command line flags.

Fixes: kata-containers#9008

Signed-off-by: Markus Rudy <webmaster@burgerdev.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement to an existing feature needs-review Needs to be assessed by the team.
Projects
Issue backlog
  
In progress
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants