Revert "Revert "feat: Sigstore verification of policies on download""

flavio · flavio · commit 14d3cff92144 · 2022-01-10T14:53:12.000+01:00
This reverts commit dec0a7a.
diff --git a/Cargo.toml b/Cargo.toml
@@ -3,7 +3,8 @@ name = "policy-server"
 version = "0.2.5"
 authors = [
   "Flavio Castelli <fcastelli@suse.com>",
-  "Rafael Fernández López <rfernandezlopez@suse.com>"
+  "Rafael Fernández López <rfernandezlopez@suse.com>",
+  "Víctor Cuadrado Juan <vcuadradojuan@suse.de>"
 ]
 edition = "2018"
 
diff --git a/README.md b/README.md
@@ -60,7 +60,7 @@ For example, given the configuration file from above, the `namespace_simple` pol
 will be invoked with the `valid_namespace` parameter set to `kubewarden-approved`.
 
 Note well: it's possible to expose the same policy multiple times, each time with
-a different set of paraments.
+a different set of parameters.
 
 The Wasm file providing the Kubewarden Policy can be either loaded from
 the local filesystem or it can be fetched from a remote location. The behaviour
@@ -76,7 +76,7 @@ depends on the URL format provided by the user:
 
 The verbosity of policy-server can be configured via the `--log-level` flag.
 The default log level used is `info`, but `trace`, `debug`, `warn` and `error`
-levels are availble too.
+levels are available too.
 
 Policy server can produce logs events using different formats. The `--log-fmt`
 flag is used to choose the format to be used.
@@ -111,12 +111,11 @@ our [official docs](https://docs.kubewarden.io/operator-manual/tracing/01-quicks
 
 # Building
 
-You can either build `kubewarden-admission` from sources (see below) or you can
-use the container image we maintain inside of our
+You can use the container image we maintain inside of our
 [GitHub Container Registry](https://github.com/orgs/kubewarden/packages/container/package/policy-server).
 
-The `policy-server` binary can be built in this way:
+Alternatively, the `policy-server` binary can be built in this way:
 
 ```shell
-$ cargo build
+$ make build
 ```
diff --git a/src/cli.rs b/src/cli.rs
@@ -1,4 +1,4 @@
-use crate::settings::{read_policies_file, Policy};
+use crate::settings::{read_policies_file, read_verification_file, Policy, VerificationSettings};
 use anyhow::{anyhow, Result};
 use clap::{crate_authors, crate_description, crate_name, crate_version, App, Arg};
 use itertools::Itertools;
@@ -109,6 +109,13 @@ pub(crate) fn build_cli() -> App<'static, 'static> {
                 .env("KUBEWARDEN_SOURCES_PATH")
                 .help("YAML file holding source information (https, registry insecure hosts, custom CA's...)"),
         )
+        .arg(
+            Arg::with_name("verification-path")
+                .env("KUBEWARDEN_VERIFICATION_CONFIG_PATH")
+                .long("verification-path")
+                .default_value("verification.yml")
+                .help("YAML file holding verification information (URIs, keys, annotations...)"),
+        )
         .arg(
             Arg::with_name("docker-config-json-path")
                 .env("KUBEWARDEN_DOCKER_CONFIG_JSON_PATH")
@@ -121,7 +128,14 @@ pub(crate) fn build_cli() -> App<'static, 'static> {
                 .long("enable-metrics")
                 .required(false)
                 .takes_value(false)
-                .help("Enable metrics"),
+                .help("Enable metrics [env: KUBEWARDEN_ENABLE_METRICS=]"),
+        )
+        .arg(
+            Arg::with_name("enable-verification")
+                .long("enable-verification")
+                .required(false)
+                .takes_value(false)
+                .help("Enable Sigstore verification [env: KUBEWARDEN_ENABLE_VERIFICATION=]"),
         )
         .long_version(VERSION_AND_BUILTINS.as_str())
 }
@@ -157,6 +171,27 @@ pub(crate) fn policies(matches: &clap::ArgMatches) -> Result<HashMap<String, Pol
     })
 }
 
+pub(crate) fn verification_settings(matches: &clap::ArgMatches) -> Result<VerificationSettings> {
+    let verification_file = Path::new(matches.value_of("verification-path").unwrap_or("."));
+    match read_verification_file(verification_file) {
+        Err(e) => Err(anyhow!(
+            "error while loading verification info from {:?}: {}",
+            verification_file,
+            e
+        )),
+        Ok(vs) => {
+            if vs.verification_keys.is_empty() {
+                Err(anyhow!(
+                    "error while loading verification info from {:?}: contains 0 verification keys",
+                    verification_file,
+                ))
+            } else {
+                Ok(vs)
+            }
+        }
+    }
+}
+
 // Setup the tracing system. This MUST be done inside of a tokio Runtime
 // because some collectors rely on it and would panic otherwise.
 pub(crate) fn setup_tracing(matches: &clap::ArgMatches) -> Result<()> {
diff --git a/src/main.rs b/src/main.rs
@@ -6,6 +6,7 @@ use anyhow::Result;
 use opentelemetry::global::shutdown_tracer_provider;
 use policy_evaluator::callback_handler::CallbackHandlerBuilder;
 use policy_evaluator::policy_metadata::Metadata;
+use settings::VerificationSettings;
 use std::path::PathBuf;
 use std::{process, thread};
 use tokio::{runtime::Runtime, sync::mpsc, sync::oneshot};
@@ -40,6 +41,35 @@ fn main() -> Result<()> {
             .expect("error parsing the number of workers")
     });
 
+    // As of clap version 2, we have to do this check in separate
+    // steps.
+    //
+    // `--enable-metrics` does not take a value. However, using
+    // `env` from clap to link the `KUBEWARDEN_ENABLE_METRICS`
+    // envvar to this flag forcefully sets `takes_value` on the
+    // flag, making the usage of `--enable-metrics` argument weird
+    // (this should not take a value).
+    //
+    // The answer is therefore, to just set up the
+    // `--enable-metrics` flag in clap, and check manually whether
+    // the environment variable is set.
+    //
+    // The same is true for `--verify-policies` flag, and
+    // KUBEWARDEN_ENABLE_VERIFICATION env var.
+    //
+    // Check https://github.com/clap-rs/clap/issues/1476 for
+    // further details.
+    let metrics_enabled = matches.is_present("enable-metrics")
+        || std::env::var_os("KUBEWARDEN_ENABLE_METRICS").is_some();
+    let verify_enabled = matches.is_present("enable-verification")
+        || std::env::var_os("KUBEWARDEN_ENABLE_VERIFICATION").is_some();
+
+    let verification_settings: Option<VerificationSettings> = if verify_enabled {
+        Some(cli::verification_settings(&matches)?)
+    } else {
+        None
+    };
+
     ////////////////////////////////////////////////////////////////////////////
     //                                                                        //
     // Phase 1: setup the CallbackHandler. This is used by the synchronous    //
@@ -149,24 +179,6 @@ fn main() -> Result<()> {
             fatal_error(err.to_string());
         }
 
-        // As of clap version 2, we have to do this check in separate
-        // steps.
-        //
-        // `--enable-metrics` does not take a value. However, using
-        // `env` from clap to link the `KUBEWARDEN_ENABLE_METRICS`
-        // envvar to this flag forcefully sets `takes_value` on the
-        // flag, making the usage of `--enable-metrics` argument weird
-        // (this should not take a value).
-        //
-        // The answer is therefore, to just set up the
-        // `--enable-metrics` flag in clap, and check manually whether
-        // the environment variable is set.
-        //
-        // Check https://github.com/clap-rs/clap/issues/1476 for
-        // further details.
-        let metrics_enabled = matches.is_present("enable-metrics")
-            || std::env::var_os("KUBEWARDEN_ENABLE_METRICS").is_some();
-
         // The unused variable is required so the meter is not dropped early and
         // lives for the whole block lifetime, exporting metrics
         let _meter = if metrics_enabled {
@@ -186,6 +198,48 @@ fn main() -> Result<()> {
         );
         for (name, policy) in policies.iter_mut() {
             debug!(policy = name.as_str(), "download");
+
+            let mut verifier = policy_fetcher::verify::Verifier::new(sources.clone());
+            let mut verified_manifest_digest: Option<String> = None;
+
+            if verify_enabled {
+                // verify policy prior to pulling for all keys, and keep the
+                // verified manifest digest of last iteration, even if all are
+                // the same:
+                for key_value in verification_settings
+                    .as_ref()
+                    .unwrap()
+                    .verification_keys
+                    .values()
+                {
+                    verified_manifest_digest = Some(
+                        verifier
+                            .verify(
+                                &policy.url,
+                                docker_config.clone(),
+                                verification_settings
+                                    .as_ref()
+                                    .unwrap()
+                                    .verification_annotations
+                                    .clone(),
+                                key_value,
+                            )
+                            .await
+                            .map_err(|e| fatal_error(e.to_string()))
+                            .unwrap(),
+                    );
+                }
+                info!(
+                    name = name.as_str(),
+                    sha256sum = verified_manifest_digest
+                        .as_ref()
+                        .unwrap_or(&"unknown".to_string())
+                        .as_str(),
+                    status = "verified-signatures",
+                    "policy download",
+                );
+            }
+
             match policy_fetcher::fetch_policy(
                 &policy.url,
                 policy_fetcher::PullDestination::Store(PathBuf::from(policies_download_dir)),
@@ -195,6 +249,33 @@ fn main() -> Result<()> {
             .await
             {
                 Ok(fetched_policy) => {
+                    if verify_enabled {
+                        if verified_manifest_digest.is_none() {
+                            // when deserializing keys we check that have keys to
+                            // verify. We will always have a digest manifest
+                            fatal_error("Verification of policy failed".to_string());
+                            unreachable!();
+                        }
+                        verifier
+                            .verify_local_file_checksum(
+                                &fetched_policy.uri,
+                                docker_config.clone(),
+                                verified_manifest_digest.as_ref().unwrap(),
+                            )
+                            .await
+                            .map_err(|e| fatal_error(e.to_string()))
+                            .unwrap();
+                        info!(
+                            name = name.as_str(),
+                            sha256sum = verified_manifest_digest
+                                .as_ref()
+                                .unwrap_or(&"unknown".to_string())
+                                .as_str(),
+                            status = "verified-local-checksum",
+                            "policy download",
+                        );
+                    }
+
                     if let Ok(Some(policy_metadata)) =
                         Metadata::from_path(&fetched_policy.local_path)
                     {
diff --git a/src/settings.rs b/src/settings.rs
@@ -36,6 +36,18 @@ pub fn read_policies_file(path: &Path) -> Result<HashMap<String, Policy>> {
     Ok(ps)
 }
 
+#[derive(Deserialize, Debug, Clone)]
+pub struct VerificationSettings {
+    pub verification_keys: HashMap<String, String>,
+    pub verification_annotations: Option<HashMap<String, String>>,
+}
+
+pub fn read_verification_file(path: &Path) -> Result<VerificationSettings> {
+    let settings_file = File::open(path)?;
+    let vs: VerificationSettings = serde_yaml::from_reader(&settings_file)?;
+    Ok(vs)
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
diff --git a/verification.yml.example b/verification.yml.example
@@ -0,0 +1,15 @@
+---
+verification_keys:
+  key-name-irrelevant: |
+            -----BEGIN PUBLIC KEY-----
+            MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEX0HFTtCfTtPmkx5p1RbtwDE1EVzu
+            wjQs1cCRKb5Pz/yUspkQsN3FO4iyWodCy5j3o0CdIJD/1gvq98pf4IG9tA==
+            -----END PUBLIC KEY-----
+  another-key: |
+            -----BEGIN PUBLIC KEY-----
+            MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEX0HFTtCfTtPmkx5p1RbtwDE1EVzu
+            wjQs1cCRKb5Pz/yUspkQsN3FO4iyWodCy5j3o0CdIJD/1gvq98pf4IG9tA==
+            -----END PUBLIC KEY-----
+verification_annotations: # optional
+  env: prod
+  foo: bar

Original file line number	Diff line number	Diff line change
`@@ -3,7 +3,8 @@ name = "policy-server"`
`3`	`3`	`version = "0.2.5"`
`4`	`4`	`authors = [`
`5`	`5`	`"Flavio Castelli <fcastelli@suse.com>",`
`6`		`- "Rafael Fernández López <rfernandezlopez@suse.com>"`
	`6`	`+ "Rafael Fernández López <rfernandezlopez@suse.com>",`
	`7`	`+ "Víctor Cuadrado Juan <vcuadradojuan@suse.de>"`
`7`	`8`	`]`
`8`	`9`	`edition = "2018"`
`9`	`10`