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

remove oidc providers check from signing nodes #334

Merged
merged 6 commits into from
Oct 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 0 additions & 15 deletions DEPLOY.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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>`

Expand Down
12 changes: 2 additions & 10 deletions infra/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
]
}

Expand Down
9 changes: 0 additions & 9 deletions infra/modules/signer/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 0 additions & 4 deletions infra/modules/signer/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
12 changes: 2 additions & 10 deletions infra/partner/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand All @@ -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
]
}
4 changes: 0 additions & 4 deletions infra/partner/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
1 change: 0 additions & 1 deletion infra/terraform-dev.tfvars
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
{
Expand Down
4 changes: 0 additions & 4 deletions infra/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
10 changes: 0 additions & 10 deletions integration-tests/src/env/containers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -525,16 +525,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(),
Expand Down
10 changes: 0 additions & 10 deletions integration-tests/src/env/local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
6 changes: 3 additions & 3 deletions mpc-recovery/src/leader_node/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down Expand Up @@ -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,
)
Expand Down Expand Up @@ -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,
)
Expand Down
31 changes: 1 addition & 30 deletions mpc-recovery/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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);
Expand All @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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);
Expand Down
45 changes: 38 additions & 7 deletions mpc-recovery/src/oauth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down Expand Up @@ -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),
Expand All @@ -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!(
Expand Down Expand Up @@ -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"),
}
Expand All @@ -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;
Expand Down
Loading