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

Link OSS #18228

Merged
merged 8 commits into from
Dec 8, 2022
Merged

Link OSS #18228

merged 8 commits into from
Dec 8, 2022

Conversation

ccapurso
Copy link
Contributor

@ccapurso ccapurso commented Dec 5, 2022

No description provided.

Comment on lines +40 to +46
grpcServer := grpc.NewServer(
grpc.KeepaliveParams(keepalive.ServerParameters{
Time: 2 * time.Second,
}),
grpc.MaxSendMsgSize(math.MaxInt32),
grpc.MaxRecvMsgSize(math.MaxInt32),
)

Check failure

Code scanning / Semgrep Scanner

Found an insecure gRPC server without 'grpc.Creds()' or options with credentials. This allows for a connection without encryption to this server. A malicious attacker could tamper with the gRPC message, which could compromise the machine. Include credentials derived from an SSL certificate in order to create a secure gRPC connection. You can create credentials using 'credentials.NewServerTLSFromFile("cert.pem", "cert.key")'.

Found an insecure gRPC server without 'grpc.Creds()' or options with credentials. This allows for a connection without encryption to this server. A malicious attacker could tamper with the gRPC message, which could compromise the machine. Include credentials derived from an SSL certificate in order to create a secure gRPC connection. You can create credentials using 'credentials.NewServerTLSFromFile("cert.pem", "cert.key")'.
Comment on lines +39 to +45
grpcServer := grpc.NewServer(
grpc.KeepaliveParams(keepalive.ServerParameters{
Time: 2 * time.Second,
}),
grpc.MaxSendMsgSize(math.MaxInt32),
grpc.MaxRecvMsgSize(math.MaxInt32),
)

Check failure

Code scanning / Semgrep Scanner

Found an insecure gRPC server without 'grpc.Creds()' or options with credentials. This allows for a connection without encryption to this server. A malicious attacker could tamper with the gRPC message, which could compromise the machine. Include credentials derived from an SSL certificate in order to create a secure gRPC connection. You can create credentials using 'credentials.NewServerTLSFromFile("cert.pem", "cert.key")'.

Found an insecure gRPC server without 'grpc.Creds()' or options with credentials. This allows for a connection without encryption to this server. A malicious attacker could tamper with the gRPC message, which could compromise the machine. Include credentials derived from an SSL certificate in order to create a secure gRPC connection. You can create credentials using 'credentials.NewServerTLSFromFile("cert.pem", "cert.key")'.
@@ -0,0 +1,46 @@
syntax = "proto3";
Copy link
Contributor

Choose a reason for hiding this comment

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

Just raising an idea here. We have a subdirectory for node status protobuf file. Would it make sense to move all protobuf files for other capabilities there too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, that was my thinking too. We'll definitely need to do that but was thinking of doing it in a separate PR. I didn't want to introduce too many structural changes yet.

vault/hcp_link/link.go Outdated Show resolved Hide resolved
@hghaf099
Copy link
Contributor

hghaf099 commented Dec 7, 2022

Looks great! Just posted a couple refactoring comments.
Seems like the no-changelog label is added. I guess this is because we don't have an official one for it yet?

@ccapurso
Copy link
Contributor Author

ccapurso commented Dec 7, 2022

Seems like the no-changelog label is added. I guess this is because we don't have an official one for it yet?
Yup, I think we have one now. I'll add one shortly.

Copy link
Contributor

@hghaf099 hghaf099 left a comment

Choose a reason for hiding this comment

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

Looks good! I just have one last question in the change log.

changelog/18228.txt Outdated Show resolved Hide resolved
@ccapurso ccapurso marked this pull request as ready for review December 8, 2022 19:37
@ccapurso ccapurso merged commit 186ee31 into main Dec 8, 2022
@ccapurso ccapurso deleted the vault-8296-link-oss branch December 8, 2022 20:02
AnPucel pushed a commit that referenced this pull request Jan 14, 2023
* add Link config, init, and capabilities

* add node status proto

* bump protoc version to 3.21.9

* make proto

* adding link tests

* remove wrapped link

* add changelog entry

* update changelog entry
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