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: grpc over tls #5990

Merged
merged 16 commits into from
Nov 30, 2023
Merged

Conversation

brianp
Copy link
Contributor

@brianp brianp commented Nov 28, 2023

Description

Add configuration and support for operating grpc over tls to enhance the overall security of communicating nodes, wallets, and miners.

This includes a self-signed certificate generation function for using locally, although it's highly recommended to generate valid, and verifiable certificates if you plan on opening any service up to the internet.

TODO (before merge, eta: 1day): Complete

  • Expand to the merge miner
  • Expand to the wallet
  • Display a warning about security when generating self signed certificates

Motivation and Context

Mo' security mo' better.

Closes: #5808

How Has This Been Tested?

Locally

What process can a PR reviewer use to test or verify this change?

Mostly read about how to set it up, and see if that makes sense.

Breaking Changes

  • None
  • Requires data directory on base node to be deleted
  • Requires hard fork
  • Other - Please specify

@ghpbot-tari-project ghpbot-tari-project added P-conflicts Process - The PR has merge conflicts that need to be resolved P-acks_required Process - Requires more ACKs or utACKs P-reviews_required Process - Requires a review from a lead maintainer to be merged labels Nov 28, 2023
@ghpbot-tari-project ghpbot-tari-project removed the P-conflicts Process - The PR has merge conflicts that need to be resolved label Nov 29, 2023
@ghpbot-tari-project ghpbot-tari-project added the P-conflicts Process - The PR has merge conflicts that need to be resolved label Nov 29, 2023
@ghpbot-tari-project ghpbot-tari-project removed the P-conflicts Process - The PR has merge conflicts that need to be resolved label Nov 29, 2023
Copy link

Test Results (CI)

1 253 tests   1 253 ✔️  10m 11s ⏱️
     39 suites         0 💤
       1 files           0

Results for commit 44e3134.

@brianp brianp marked this pull request as ready for review November 29, 2023 18:11
Copy link

Test Results (Integration tests)

  2 files  12 suites   48m 44s ⏱️
32 tests 29 ✔️ 0 💤 3
37 runs  31 ✔️ 0 💤 6

For more details on these failures, see this check.

Results for commit 44e3134.

@ghpbot-tari-project ghpbot-tari-project removed the P-reviews_required Process - Requires a review from a lead maintainer to be merged label Nov 30, 2023
@SWvheerden SWvheerden merged commit b80f7e3 into tari-project:development Nov 30, 2023
13 of 14 checks passed
SWvheerden added a commit that referenced this pull request Nov 30, 2023
Description
---
Add configuration and support for operating grpc over tls to enhance the
overall security of communicating nodes, wallets, and miners.

This includes a self-signed certificate generation function for using
locally, although it's highly recommended to generate valid, and
verifiable certificates if you plan on opening any service up to the
internet.

~TODO (before merge, eta: 1day)~: Complete
- ~Expand to the merge miner~
- ~Expand to the wallet~
- ~Display a warning about security when generating self signed
certificates~

Motivation and Context
---
Mo' security mo' better.

Closes: #5808

How Has This Been Tested?
---
Locally

What process can a PR reviewer use to test or verify this change?
---
Mostly read about how to set it up, and see if that makes sense.

Breaking Changes
---

- [x] None
- [ ] Requires data directory on base node to be deleted
- [ ] Requires hard fork
- [ ] Other - Please specify
SWvheerden added a commit that referenced this pull request Dec 1, 2023
Description
---
Add configuration and support for operating grpc over tls to enhance the
overall security of communicating nodes, wallets, and miners.

This includes a self-signed certificate generation function for using
locally, although it's highly recommended to generate valid, and
verifiable certificates if you plan on opening any service up to the
internet.

~TODO (before merge, eta: 1day)~: Complete
- ~Expand to the merge miner~
- ~Expand to the wallet~
- ~Display a warning about security when generating self signed
certificates~

Motivation and Context
---
Mo' security mo' better.

Closes: #5808

How Has This Been Tested?
---
Locally

What process can a PR reviewer use to test or verify this change?
---
Mostly read about how to set it up, and see if that makes sense.

Breaking Changes
---

- [x] None
- [ ] Requires data directory on base node to be deleted
- [ ] Requires hard fork
- [ ] Other - Please specify
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-acks_required Process - Requires more ACKs or utACKs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GRPC requests are done using http
3 participants