Added New Sections Related To Memory Management, Communication Security and Process Management #91
Conversation
pypalkar23
commented
Dec 11, 2022
pypalkar23
commented
- In the Memory Management section, added a new subsection related to memory leakage scenarios.
- In Communication Security, a new subsection related to gRPC(Google Remote Procedure Call) has been added. To provide greater clarity on the topic code snippet has been added to a subdirectory under the Communication Security subdirectory.
- An entirely new section of Process Management has been added to the book. The section deals with scenarios that may result in zombie/dangling Goroutines.
There was a problem hiding this comment.
@pypalkar23, thanks for bringing up this topics!
That's my first time reviewing PRs in this repo, I hope it's fine with the maintainers.
Left few comments to the gRPC related sections.
| import ( | ||
| "context" | ||
| "crypto/tls" | ||
| "crypto/x509" | ||
| "flag" | ||
| "fmt" | ||
| "io/ioutil" | ||
| "log" | ||
| pb "pentest/grpc/samplebuff" | ||
| "time" | ||
|
|
||
| "google.golang.org/grpc" | ||
| "google.golang.org/grpc/credentials" | ||
| ) |
There was a problem hiding this comment.
nit: fmt
| import ( | |
| "context" | |
| "crypto/tls" | |
| "crypto/x509" | |
| "flag" | |
| "fmt" | |
| "io/ioutil" | |
| "log" | |
| pb "pentest/grpc/samplebuff" | |
| "time" | |
| "google.golang.org/grpc" | |
| "google.golang.org/grpc/credentials" | |
| ) | |
| import ( | |
| "context" | |
| "crypto/tls" | |
| "crypto/x509" | |
| "flag" | |
| "fmt" | |
| "io/ioutil" | |
| "log" | |
| "time" | |
| pb "pentest/grpc/samplebuff" | |
| "google.golang.org/grpc" | |
| "google.golang.org/grpc/credentials" | |
| ) |
| defer cancel() | ||
| r, err := c.Greet(ctx, &pb.SendMsg{Name: *name}) |
There was a problem hiding this comment.
nit: fmt
| defer cancel() | |
| r, err := c.Greet(ctx, &pb.SendMsg{Name: *name}) | |
| defer cancel() | |
| r, err := c.Greet(ctx, &pb.SendMsg{Name: *name}) |
|
|
||
| func main() { | ||
| flag.Parse() | ||
| b, _ := ioutil.ReadFile("../cert/ca.cert") |
There was a problem hiding this comment.
nit: ioutil package is deprecated since go1.16. In this case we should use os.ReadFile instead.
There was a problem hiding this comment.
nit: let's promote best practice of not skipping errors
| b, _ := ioutil.ReadFile("../cert/ca.cert") | |
| b, err := ioutil.ReadFile("../cert/ca.cert") | |
| if err != nil { | |
| log.Fatalf("Could read ca.cert: %v", err) | |
| } |
There was a problem hiding this comment.
@anatolym Shouldn't the error message be "Couldn't read ca.cert: %v", instead?
| //Configuring the certificates | ||
| creds, err := credentials.NewServerTLSFromFile("../cert/service.pem", "../cert/service.key") | ||
|
|
||
| if err != nil { | ||
| log.Fatalf("TLS setup failed: %v", err) | ||
| } |
There was a problem hiding this comment.
nit: fmt
| //Configuring the certificates | |
| creds, err := credentials.NewServerTLSFromFile("../cert/service.pem", "../cert/service.key") | |
| if err != nil { | |
| log.Fatalf("TLS setup failed: %v", err) | |
| } | |
| // Configuring the certificates. | |
| creds, err := credentials.NewServerTLSFromFile("../cert/service.pem", "../cert/service.key") | |
| if err != nil { | |
| log.Fatalf("TLS setup failed: %v", err) | |
| } |
| @@ -0,0 +1,18 @@ | |||
| syntax = "proto3"; | |||
|
|
|||
| option go_package = "github.com/pypalkar23/go-rpc-cis5209/sample"; | |||
There was a problem hiding this comment.
Perhaps, this name should be changed to something else like pentest/grpc/samplebuff?
| // server is used to implement sample.GreeterServer. | ||
| type server struct { | ||
| pb.UnimplementedSampleServiceServer | ||
| } |
There was a problem hiding this comment.
nit: I would suggest not embedding gRPC service interface to a structure like this. The issue is that this implicitly says the server struct implements the interface, and the program compiles with no issues. Now imaging we remove server.Greet method. The program should still compile, but now it panics in runtime when Greet is called. (it will panic because of nil receiver)
Instead, we can enforce interface implementation check on the compile time like this.
| // server is used to implement sample.GreeterServer. | |
| type server struct { | |
| pb.UnimplementedSampleServiceServer | |
| } | |
| // Verify interface implementation on compile. | |
| var _ pb.UnimplementedSampleServiceServer = (*server)(nil) | |
| // server implements sample.GreeterServer interface. | |
| type server struct {} |
Note, the interface implementation check will be implicitly performed by compiler by the pb.RegisterSampleServiceServer(s, &server{}) call below. This is ok but explicit check with var _ ... is more obvious for a reader.
|
|
||
| ### Secure gRPC Server: | ||
| The complete code can be found [here][2]. | ||
| ```go |
There was a problem hiding this comment.
nit: reminder, we should update the examples if the above comments are accepted
|
|
||
| Let's look at how we can implement secure gRPC calls using TLS Encryption on both client and server side. | ||
|
|
||
| ### Secure gRPC Server: |
There was a problem hiding this comment.
nit: fmt here and in other MD files, in general titles and subtitles shouldn't end with punctuation marks (incl :)
| ### Secure gRPC Server: | |
| ### Secure gRPC Server |
|
@pypalkar23 and @anatolym thank you both for the great contributions and sorry for the late reply. @pypalkar23 can you please consider @anatolym suggestions? @anatolym did you have the chance to review the other suggestions changes/additions? Cheers, |
