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: support multiple allowIPs for a remote connection from one of KMS servers #707

Merged
merged 2 commits into from
Aug 30, 2023

Conversation

jaeseung-bae
Copy link
Contributor

@jaeseung-bae jaeseung-bae commented Aug 29, 2023

Description

  • ASIS: support only one ip address for a connection from a remote KMS server
  • TOBE: support multiple ip addresses as allowList for a connection from one of remote KMS servers

@codecov
Copy link

codecov bot commented Aug 29, 2023

Codecov Report

Merging #707 (486c71a) into main (449aa31) will increase coverage by 0.04%.
Report is 6 commits behind head on main.
The diff coverage is 78.57%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #707      +/-   ##
==========================================
+ Coverage   66.54%   66.58%   +0.04%     
==========================================
  Files         285      285              
  Lines       37915    37919       +4     
==========================================
+ Hits        25229    25249      +20     
+ Misses      10878    10867      -11     
+ Partials     1808     1803       -5     
Files Changed Coverage Δ
config/config.go 79.36% <ø> (ø)
config/toml.go 74.19% <ø> (ø)
privval/utils.go 33.33% <0.00%> (ø)
privval/internal/ip_filter.go 70.37% <88.88%> (-7.90%) ⬇️
node/node.go 61.49% <100.00%> (ø)
privval/signer_listener_endpoint.go 88.81% <100.00%> (ø)

... and 10 files with indirect coverage changes

@jaeseung-bae jaeseung-bae self-assigned this Aug 29, 2023
@jaeseung-bae jaeseung-bae marked this pull request as ready for review August 29, 2023 06:55
@ulbqb ulbqb added the C: enhancement Classification: New feature or its request, or improvement in maintainability of code label Aug 30, 2023
Copy link
Member

@zemyblue zemyblue left a comment

Choose a reason for hiding this comment

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

Is there a valid address checking?
Is it possible using url not ip? if then, are there any security issues?

@jaeseung-bae
Copy link
Contributor Author

Is there a valid address checking?

There is no validation logic for valid address.

Is it possible using url not ip? if then, are there any security issues?

For now, it's not possible because when accepting a connection what I can receive is (net.Conn, error) which has only net.Addr for local and remote. There's no way to find out url.

@zemyblue
Copy link
Member

Is there a valid address checking?

There is no validation logic for valid address.

Is it possible using url not ip? if then, are there any security issues?

For now, it's not possible because when accepting a connection what I can receive is (net.Conn, error) which has only net.Addr for local and remote. There's no way to find out url.

Then, what happen if I write 127.0.1?

@jaeseung-bae
Copy link
Contributor Author

Then, what happen if I write 127.0.1?

None of the connections will be accepted because no address match to 127.0.1

ulbqb
ulbqb previously approved these changes Aug 30, 2023
privval/internal/ip_filter.go Show resolved Hide resolved
config/config.go Outdated Show resolved Hide resolved
torao
torao previously approved these changes Aug 30, 2023
Co-authored-by: Torao Takami <koiroha@gmail.com>
@jaeseung-bae jaeseung-bae dismissed stale reviews from torao and ulbqb via 486c71a August 30, 2023 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: enhancement Classification: New feature or its request, or improvement in maintainability of code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants