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

feat: adds certificate validation on provider proxy #700

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

stalniy
Copy link
Contributor

@stalniy stalniy commented Jan 22, 2025

Why

We need to validate provider self-signed certificate

What

  1. Fetch provider certificates from chain and check whether SSL certificate returned by provider's host is one of those on chain
  2. adds nodemon for easier local development
  3. removes unused packages
  4. creates internal @akasnetwork/net package for sharing blockchain configuration
  5. added unit and functional tests
  6. modifies GHA for provider proxy to run both types of tests
  7. added esbuild to bundle internal packages together with provider-proxy

refs: #170

@stalniy

This comment was marked as outdated.

// will have certificate for every request
const didHandshake = Object.keys(serverCert).length > 0;

if (didHandshake && options.network && options.providerAddress) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

temporary ignore validation if options.network and options.providerAddress are empty because deploy-web also require adjustments, just didn't want to create massive PR

@stalniy stalniy force-pushed the feat/validate-certs branch 10 times, most recently from e5164a5 to f907e42 Compare January 24, 2025 17:31
@stalniy stalniy marked this pull request as ready for review January 24, 2025 17:31
@stalniy stalniy requested a review from a team as a code owner January 24, 2025 17:31
@stalniy stalniy force-pushed the feat/validate-certs branch from f907e42 to 2f46f23 Compare January 24, 2025 17:35
@stalniy stalniy force-pushed the feat/validate-certs branch from 2f46f23 to 1d5ae08 Compare January 24, 2025 17:43
Copy link
Contributor

@baktun14 baktun14 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very well done and solid implementation! 🚀

@@ -16,6 +16,7 @@ const whitelist = [
"https://console-beta.akash.network"
];

app.disable("x-powered-by");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does that do?

body: options.body,
headers: new Headers(options.headers),
agent: httpsAgent
const queryParams = `?filter.state=valid&filter.owner=${providerAddress}&pagination.limit=${this.certificatesAmountToFetch}`;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing to note is that it's possible for addresses to have more than 100 certificates. Although very unlikely, it has happened in the past and resulted in hard to find bugs. This is how we load them in the frontend and we have the same function in the indexer as well.

Comment on lines +16 to +17
{ shortName: "ST", value: "Virginia" },
{ name: "localityName", value: "Blacksburg" },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where are these values taken from?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants