Added New Sections Related To Memory Management, Communication Security and Process Management by pypalkar23 · Pull Request #91 · OWASP/Go-SCP

pypalkar23 · 2022-12-11T16:05:46Z

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.

anatolym

@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.

anatolym · 2022-12-12T20:14:25Z

src/communication-security/grpc-code/grpc_client_secured/client.go

+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"
+)


nit: fmt

Suggested change

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"

)

anatolym · 2022-12-12T20:14:51Z

src/communication-security/grpc-code/grpc_client_secured/client.go

+	defer cancel()
+	r, err := c.Greet(ctx, &pb.SendMsg{Name: *name})


nit: fmt

Suggested change

defer cancel()

r, err := c.Greet(ctx, &pb.SendMsg{Name: *name})

defer cancel()

r, err := c.Greet(ctx, &pb.SendMsg{Name: *name})

anatolym · 2022-12-12T20:16:44Z

src/communication-security/grpc-code/grpc_client_secured/client.go

+
+func main() {
+	flag.Parse()
+	b, _ := ioutil.ReadFile("../cert/ca.cert")


nit: ioutil package is deprecated since go1.16. In this case we should use os.ReadFile instead.

nit: let's promote best practice of not skipping errors

Suggested change

b, _ := ioutil.ReadFile("../cert/ca.cert")

b, err := ioutil.ReadFile("../cert/ca.cert")

if err != nil {

log.Fatalf("Could read ca.cert: %v", err)

}

@anatolym Shouldn't the error message be "Couldn't read ca.cert: %v", instead?

anatolym · 2022-12-12T20:20:02Z

src/communication-security/grpc-code/grpc_server_secured/server.go

+	//Configuring the certificates
+	creds, err := credentials.NewServerTLSFromFile("../cert/service.pem", "../cert/service.key")
+
+	if err != nil {
+		log.Fatalf("TLS setup failed: %v", err)
+	}


nit: fmt

Suggested change

//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)

}

anatolym · 2022-12-12T20:21:55Z

src/communication-security/grpc-code/samplebuff/sample.proto

@@ -0,0 +1,18 @@
+syntax = "proto3";
+
+option go_package = "github.com/pypalkar23/go-rpc-cis5209/sample";


Perhaps, this name should be changed to something else like pentest/grpc/samplebuff?

anatolym · 2022-12-12T20:30:03Z

src/communication-security/grpc-code/grpc_server_secured/server.go

+// server is used to implement sample.GreeterServer.
+type server struct {
+	pb.UnimplementedSampleServiceServer
+}


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.

Suggested change

// 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.

anatolym · 2022-12-12T20:31:27Z

src/communication-security/grpc.md

+
+### Secure gRPC Server:
+The complete code can be found [here][2].
+```go


nit: reminder, we should update the examples if the above comments are accepted

anatolym · 2022-12-12T20:32:25Z

src/communication-security/grpc.md

+
+Let's look at how we can implement secure gRPC calls using TLS Encryption on both client and server side.
+
+### Secure gRPC Server:


nit: fmt here and in other MD files, in general titles and subtitles shouldn't end with punctuation marks (incl :)

Suggested change

### Secure gRPC Server:

### Secure gRPC Server

PauloASilva · 2022-12-27T09:11:31Z

@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,
Paulo A. Silva

pypalkar23 and others added 7 commits December 7, 2022 11:42

Added new snippets in memory management

16e6a66

Added article related to gRPC

e7f4998

Adding Chapter for Process Management

a8b6ebc

Updating Summary

8171ed4

Update grpc.md

4a5742a

Fixing markdown in Memory management section

34d5df4

Correcting Typos

598bb1b

anatolym reviewed Dec 12, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added New Sections Related To Memory Management, Communication Security and Process Management#91

Added New Sections Related To Memory Management, Communication Security and Process Management#91
pypalkar23 wants to merge 7 commits intoOWASP:masterfrom
pypalkar23:master

pypalkar23 commented Dec 11, 2022

Uh oh!

anatolym left a comment

Uh oh!

anatolym Dec 12, 2022

Uh oh!

anatolym Dec 12, 2022

Uh oh!

anatolym Dec 12, 2022

Uh oh!

anatolym Dec 12, 2022

Uh oh!

PauloASilva Dec 27, 2022

Uh oh!

anatolym Dec 12, 2022

Uh oh!

anatolym Dec 12, 2022

Uh oh!

anatolym Dec 12, 2022

Uh oh!

anatolym Dec 12, 2022

Uh oh!

anatolym Dec 12, 2022 •

edited

Loading

Uh oh!

PauloASilva commented Dec 27, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		defer cancel()
		r, err := c.Greet(ctx, &pb.SendMsg{Name: *name})

		@@ -0,0 +1,18 @@
		syntax = "proto3";

		option go_package = "github.com/pypalkar23/go-rpc-cis5209/sample";

-// 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 {}


		Let's look at how we can implement secure gRPC calls using TLS Encryption on both client and server side.

		### Secure gRPC Server:

Conversation

pypalkar23 commented Dec 11, 2022

Uh oh!

anatolym left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anatolym Dec 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PauloASilva commented Dec 27, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

anatolym Dec 12, 2022 •

edited

Loading