remove oidc providers check from signing nodes (#334)

* Remove OIDC providers check from signing nodes
near · Oct 26, 2023 · 012b2aa · 012b2aa
1 parent f95f57a
commit 012b2aa
Show file tree

Hide file tree

Showing 14 changed files with 48 additions and 124 deletions.
diff --git a/DEPLOY.md b/DEPLOY.md
@@ -63,20 +63,6 @@ You also need to grab the AES cipher key that was printed after `Cipher 0:`; it
 
 Save it to GCP Secret Manager under the name of your choosing (e.g. `mpc-recovery-cipher-prod`). This name will be referred to as `GCP_CIPHER_SECRET_ID`.
 
-### OIDC Providers
-Create one more secret secret under the name of your choosing (e.g. `mpc-recovery-oidc-providers-prod`). This name will be referred to as `GCP_OIDC_PROVIDERS_SECRET_ID`.
-
-Populate it with whatever your Pagoda point of contact told you to use, it should look similar to below:
-
-```
-[
-  {
-    issuer   = "https://securetoken.google.com/near-fastauth-prod",
-    audience = "near-fastauth-prod"
-  }
-]
-```
-
 ## Building Docker Image
 
 Build the mpc-recovery docker image from this folder and make sure to tag it for convenience:
@@ -92,7 +78,6 @@ Go to `infra/partner` and copy `template.tfvars` as `prod.tfvars`. Edit `prod.tf
 * Set `env` to `prod`
 * Set `project` to `<GCP_PROJECT_ID>`
 * Set `node_id` to whatever your point of contact with Pagoda has given you (ask them if they did not). It is very important you use this specific ID for your node's configuration
-* Set `oidc_providers_secret_id` to `<GCP_OIDC_PROVIDERS_SECRET_ID>`
 * Set `cipher_key_secret_id` to `<GCP_CIPHER_SECRET_ID>`
 * Set `sk_share_secret_id` to `<GCP_SK_SHARE_SECRET_ID>`
 

diff --git a/infra/main.tf b/infra/main.tf
@@ -86,12 +86,6 @@ resource "google_secret_manager_secret_iam_member" "secret_share_secret_access"
   member    = "serviceAccount:${google_service_account.service_account.email}"
 }
 
-resource "google_secret_manager_secret_iam_member" "oidc_providers_secret_access" {
-  secret_id = var.oidc_providers_secret_id
-  role      = "roles/secretmanager.secretAccessor"
-  member    = "serviceAccount:${google_service_account.service_account.email}"
-}
-
 resource "google_secret_manager_secret_iam_member" "account_creator_secret_access" {
   secret_id = var.account_creator_sk_secret_id
   role      = "roles/secretmanager.secretAccessor"
@@ -120,16 +114,14 @@ module "signer" {
 
   node_id = count.index
 
-  oidc_providers_secret_id = var.oidc_providers_secret_id
-  cipher_key_secret_id     = var.signer_configs[count.index].cipher_key_secret_id
-  sk_share_secret_id       = var.signer_configs[count.index].sk_share_secret_id
+  cipher_key_secret_id = var.signer_configs[count.index].cipher_key_secret_id
+  sk_share_secret_id   = var.signer_configs[count.index].sk_share_secret_id
 
   jwt_signature_pk_url = var.jwt_signature_pk_url
 
   depends_on = [
     google_secret_manager_secret_iam_member.cipher_key_secret_access,
     google_secret_manager_secret_iam_member.secret_share_secret_access,
-    google_secret_manager_secret_iam_member.oidc_providers_secret_access
   ]
 }
 

diff --git a/infra/modules/signer/main.tf b/infra/modules/signer/main.tf
@@ -49,15 +49,6 @@ resource "google_cloud_run_v2_service" "signer" {
           }
         }
       }
-      env {
-        name = "OIDC_PROVIDERS"
-        value_source {
-          secret_key_ref {
-            secret  = var.oidc_providers_secret_id
-            version = "latest"
-          }
-        }
-      }
       env {
         name  = "MPC_RECOVERY_JWT_SIGNATURE_PK_URL"
         value = var.jwt_signature_pk_url

diff --git a/infra/modules/signer/variables.tf b/infra/modules/signer/variables.tf
@@ -29,10 +29,6 @@ variable "sk_share_secret_id" {
   type = string
 }
 
-variable "oidc_providers_secret_id" {
-  type = string
-}
-
 variable "jwt_signature_pk_url" {
   type = string
 }
diff --git a/infra/partner/main.tf b/infra/partner/main.tf
@@ -62,12 +62,6 @@ resource "google_secret_manager_secret_iam_member" "secret_share_secret_access"
   member    = "serviceAccount:${google_service_account.service_account.email}"
 }
 
-resource "google_secret_manager_secret_iam_member" "oidc_providers_secret_access" {
-  secret_id = var.oidc_providers_secret_id
-  role      = "roles/secretmanager.secretAccessor"
-  member    = "serviceAccount:${google_service_account.service_account.email}"
-}
-
 /*
  * Create a partner signer node
  */
@@ -83,15 +77,13 @@ module "signer" {
 
   node_id = var.node_id
 
-  cipher_key_secret_id     = var.cipher_key_secret_id
-  sk_share_secret_id       = var.sk_share_secret_id
-  oidc_providers_secret_id = var.oidc_providers_secret_id
+  cipher_key_secret_id = var.cipher_key_secret_id
+  sk_share_secret_id   = var.sk_share_secret_id
 
   jwt_signature_pk_url = var.jwt_signature_pk_url
 
   depends_on = [
     google_secret_manager_secret_iam_member.cipher_key_secret_access,
     google_secret_manager_secret_iam_member.secret_share_secret_access,
-    google_secret_manager_secret_iam_member.oidc_providers_secret_access
   ]
 }
diff --git a/infra/partner/variables.tf b/infra/partner/variables.tf
@@ -29,10 +29,6 @@ variable "sk_share_secret_id" {
   type = string
 }
 
-variable "oidc_providers_secret_id" {
-  type = string
-}
-
 variable "jwt_signature_pk_url" {
   type = string
 }
diff --git a/infra/terraform-dev.tfvars b/infra/terraform-dev.tfvars
@@ -4,7 +4,6 @@ docker_image = "us-east1-docker.pkg.dev/pagoda-discovery-platform-dev/mpc-recove
 
 account_creator_id           = "mpc-recovery-dev-creator.testnet"
 account_creator_sk_secret_id = "mpc-recovery-account-creator-sk-dev"
-oidc_providers_secret_id     = "mpc-allowed-oidc-providers-dev"
 fast_auth_partners_secret_id = "mpc-fast-auth-partners-dev"
 signer_configs = [
   {

diff --git a/infra/variables.tf b/infra/variables.tf
@@ -39,10 +39,6 @@ variable "account_creator_sk_secret_id" {
   type = string
 }
 
-variable "oidc_providers_secret_id" {
-  type = string
-}
-
 variable "fast_auth_partners_secret_id" {
   type = string
 }

diff --git a/integration-tests/src/env/containers.rs b/integration-tests/src/env/containers.rs
@@ -524,16 +524,6 @@ impl SignerNode<'_> {
             web_port: Self::CONTAINER_PORT,
             sk_share: Some(serde_json::to_string(&sk_share)?),
             cipher_key: Some(hex::encode(cipher_key)),
-            oidc_providers_filepath: None,
-            oidc_providers: Some(
-                serde_json::json!([
-                    {
-                        "issuer": ctx.issuer,
-                        "audience": ctx.audience_id,
-                    },
-                ])
-                .to_string(),
-            ),
             gcp_project_id: ctx.gcp_project_id.clone(),
             gcp_datastore_url: Some(ctx.datastore.address.clone()),
             jwt_signature_pk_url: ctx.oidc_provider.jwt_pk_url.clone(),

diff --git a/integration-tests/src/env/local.rs b/integration-tests/src/env/local.rs
@@ -37,16 +37,6 @@ impl SignerNode {
             web_port,
             sk_share: Some(serde_json::to_string(&sk_share)?),
             cipher_key: Some(hex::encode(cipher_key)),
-            oidc_providers_filepath: None,
-            oidc_providers: Some(
-                serde_json::json!([
-                    {
-                        "issuer": ctx.issuer,
-                        "audience": ctx.audience_id,
-                    },
-                ])
-                .to_string(),
-            ),
             gcp_project_id: ctx.gcp_project_id.clone(),
             gcp_datastore_url: Some(ctx.datastore.local_address.clone()),
             jwt_signature_pk_url: ctx.oidc_provider.jwt_pk_local_url.clone(),

diff --git a/mpc-recovery/src/leader_node/mod.rs b/mpc-recovery/src/leader_node/mod.rs
@@ -322,7 +322,7 @@ async fn process_user_credentials(
 ) -> Result<UserCredentialsResponse, LeaderNodeError> {
     verify_oidc_token(
         &request.oidc_token,
-        &state.partners.oidc_providers(),
+        Some(&state.partners.oidc_providers()),
         &state.reqwest_client,
         &state.jwt_signature_pk_url,
     )
@@ -354,7 +354,7 @@ async fn process_new_account(
     let new_user_account_id = request.near_account_id;
     let oidc_token_claims = verify_oidc_token(
         &request.oidc_token,
-        &state.partners.oidc_providers(),
+        Some(&state.partners.oidc_providers()),
         &state.reqwest_client,
         &state.jwt_signature_pk_url,
     )
@@ -497,7 +497,7 @@ async fn process_sign(
     // Check OIDC token
     verify_oidc_token(
         &request.oidc_token,
-        &state.partners.oidc_providers(),
+        Some(&state.partners.oidc_providers()),
         &state.reqwest_client,
         &state.jwt_signature_pk_url,
     )

diff --git a/mpc-recovery/src/lib.rs b/mpc-recovery/src/lib.rs
@@ -18,7 +18,7 @@ use near_crypto::{InMemorySigner, SecretKey};
 use near_fetch::signer::KeyRotatingSigner;
 use near_primitives::types::AccountId;
 
-use crate::firewall::allowed::{OidcProviderList, PartnerList};
+use crate::firewall::allowed::PartnerList;
 use crate::gcp::GcpService;
 use crate::sign_node::migration;
 
@@ -136,12 +136,6 @@ pub enum Cli {
         /// The web port for this server
         #[arg(long, env("MPC_RECOVERY_WEB_PORT"))]
         web_port: u16,
-        /// JSON list of related items to be used to verify OIDC tokens.
-        #[arg(long, env("OIDC_PROVIDERS"))]
-        oidc_providers: Option<String>,
-        /// Filepath to a JSON list of related items to be used to verify OIDC tokens.
-        #[arg(long, value_parser, env("OIDC_PROVIDERS_FILEPATH"))]
-        oidc_providers_filepath: Option<PathBuf>,
         /// GCP project ID
         #[arg(long, env("MPC_RECOVERY_GCP_PROJECT_ID"))]
         gcp_project_id: String,
@@ -251,8 +245,6 @@ pub async fn run(cmd: Cli) -> anyhow::Result<()> {
             sk_share,
             cipher_key,
             web_port,
-            oidc_providers,
-            oidc_providers_filepath,
             gcp_project_id,
             gcp_datastore_url,
             jwt_signature_pk_url,
@@ -268,16 +260,6 @@ pub async fn run(cmd: Cli) -> anyhow::Result<()> {
             .global();
             let gcp_service =
                 GcpService::new(env.clone(), gcp_project_id, gcp_datastore_url).await?;
-            let oidc_providers = OidcProviderList {
-                entries: load_entries(
-                    &gcp_service,
-                    &env,
-                    node_id.to_string().as_str(),
-                    oidc_providers,
-                    oidc_providers_filepath,
-                )
-                .await?,
-            };
             let cipher_key = load_cipher_key(&gcp_service, &env, node_id, cipher_key).await?;
             let cipher_key = hex::decode(cipher_key)?;
             let cipher_key = GenericArray::<u8, U32>::clone_from_slice(&cipher_key);
@@ -294,7 +276,6 @@ pub async fn run(cmd: Cli) -> anyhow::Result<()> {
                 node_key: sk_share,
                 cipher,
                 port: web_port,
-                oidc_providers,
                 jwt_signature_pk_url,
             };
             run_sign_node(config).await;
@@ -502,8 +483,6 @@ impl Cli {
                 web_port,
                 cipher_key,
                 sk_share,
-                oidc_providers,
-                oidc_providers_filepath,
                 gcp_project_id,
                 gcp_datastore_url,
                 jwt_signature_pk_url,
@@ -530,14 +509,6 @@ impl Cli {
                     buf.push("--sk-share".to_string());
                     buf.push(share);
                 }
-                if let Some(providers) = oidc_providers {
-                    buf.push("--oidc-providers".to_string());
-                    buf.push(providers);
-                }
-                if let Some(providers_filepath) = oidc_providers_filepath {
-                    buf.push("--oidc-providers-filepath".to_string());
-                    buf.push(providers_filepath.to_str().unwrap().to_string());
-                }
                 if let Some(gcp_datastore_url) = gcp_datastore_url {
                     buf.push("--gcp-datastore-url".to_string());
                     buf.push(gcp_datastore_url);

diff --git a/mpc-recovery/src/oauth.rs b/mpc-recovery/src/oauth.rs
@@ -11,7 +11,7 @@ use crate::sign_node::oidc::OidcToken;
 // Firebase: https://firebase.google.com/docs/auth/admin/verify-id-tokens#verify_id_tokens_using_a_third-party_jwt_library
 pub async fn verify_oidc_token(
     token: &OidcToken,
-    oidc_providers: &OidcProviderList,
+    oidc_providers: Option<&OidcProviderList>,
     client: &reqwest::Client,
     jwt_signature_pk_url: &str,
 ) -> anyhow::Result<IdTokenClaims> {
@@ -41,7 +41,7 @@ pub async fn verify_oidc_token(
 fn validate_jwt(
     token: &OidcToken,
     public_key: &[u8],
-    oidc_providers: &OidcProviderList,
+    oidc_providers: Option<&OidcProviderList>,
 ) -> anyhow::Result<IdTokenClaims> {
     tracing::info!(
         oidc_token = format!("{:.5}...", token),
@@ -57,8 +57,12 @@ fn validate_jwt(
         ..
     } = &claims;
 
-    if !oidc_providers.contains(issuer, audience) {
-        anyhow::bail!("UnauthorizedTokenIssuerOrAudience: iss={issuer}, aud={audience}");
+    // If no OIDC providers are specified in the allowlist, we allow any issuer and audience.
+    // Should be used in signing nodes only.
+    if let Some(oidc_providers) = oidc_providers {
+        if !oidc_providers.contains(issuer, audience) {
+            anyhow::bail!("UnauthorizedTokenIssuerOrAudience: iss={issuer}, aud={audience}");
+        }
     }
 
     tracing::info!(
@@ -145,11 +149,11 @@ mod tests {
         };
 
         // Valid token and claims
-        validate_jwt(&token, &public_key_der, &oidc_providers).unwrap();
+        validate_jwt(&token, &public_key_der, Some(&oidc_providers)).unwrap();
 
         // Invalid public key
         let (invalid_public_key, _invalid_private_key) = get_rsa_pem_key_pair();
-        match validate_jwt(&token, &invalid_public_key, &oidc_providers) {
+        match validate_jwt(&token, &invalid_public_key, Some(&oidc_providers)) {
             Ok(_) => panic!("Token validation should fail"),
             Err(e) => assert_eq!(e.to_string(), "InvalidSignature"),
         }
@@ -169,12 +173,39 @@ mod tests {
             Ok(t) => OidcToken::new(t.as_str()),
             Err(e) => panic!("Failed to encode token: {}", e),
         };
-        match validate_jwt(&token, &public_key_der, &oidc_providers) {
+        match validate_jwt(&token, &public_key_der, Some(&oidc_providers)) {
             Ok(_) => panic!("Token validation should fail on invalid issuer or audience"),
             Err(e) => assert_eq!(e.to_string(), "UnauthorizedTokenIssuerOrAudience: iss=unauthorized_issuer, aud=unauthorized_audience", "{:?}", e),
         }
     }
 
+    #[test]
+    fn test_validate_jwt_without_oidc() {
+        let (private_key_der, public_key_der): (Vec<u8>, Vec<u8>) = get_rsa_pem_key_pair();
+
+        let my_claims = IdTokenClaims {
+            iss: "test_issuer".to_string(),
+            sub: "test_subject".to_string(),
+            aud: "test_audience".to_string(),
+            exp: (Utc::now() + Duration::hours(1)).timestamp() as usize,
+        };
+
+        let token = match encode(
+            &Header::new(Algorithm::RS256),
+            &my_claims,
+            &EncodingKey::from_rsa_pem(&private_key_der).unwrap(),
+        ) {
+            Ok(t) => OidcToken::new(t.as_str()),
+            Err(e) => panic!("Failed to encode token: {}", e),
+        };
+
+        // Valid token and claims
+        match validate_jwt(&token, &public_key_der, None) {
+            Ok(_) => (),
+            Err(e) => panic!("Token validation should succeed: {}", e),
+        }
+    }
+
     pub fn get_rsa_pem_key_pair() -> (Vec<u8>, Vec<u8>) {
         let mut rng = OsRng;
         let bits: usize = 2048;