Encryption support in Go client #552

GPrabhudas · 2021-06-24T10:54:17Z

Motivation

Added Go client changes to encrypt messages published to pulsar. Once encrypted, it only allows the consumer with the right set of keys to be able to decrypt the original message published by the producer. Only RSA encryption of data key is supported.

Modifications

Changes are implemented as per PIP-4

Followed Java client Encryption support changes

Addresses issue #333

- add interface to support encryption - provide default encryption/decryption support - add ability to add custom encryption/decryption of data key

- make initial changes in producer for encryption[needs refactoring] - make changes for decrypting in consumer

- add test case to produce encrypted message and receive decrypted message

- adding encryption & decryption needs to have access to MessageMetadata - so making the pulsar proto package public from internal

cckellogg

Thanks for your contribution. I took a quick look and left a few comments. A few suggestions:

In go file names are generally snake case so please try to follow that to keep things consistent. For example this crypto_key_reader.go instead of CryptoKeyReader.go

Can we break this up into multiple pull requests? Can we start with the crypto package that adds the interfaces and base implementations. Then some followup pull requests can be added for the consumer and producer implementation. This will make it easier on people to review the code.

cckellogg · 2021-06-24T16:53:30Z

pulsar/consumer_impl.go

 	"github.com/apache/pulsar-client-go/pulsar/log"
+	pb "github.com/apache/pulsar-client-go/pulsar/pulsar_proto"


We want to avoid exposing the proto as a public api. We should move this back to internal

Client need access to MessagaMetada if they do not want to use default implementation of crypto. And it is not possible unless we make it public or create a wrapper around it.

cckellogg · 2021-06-24T16:54:34Z

pulsar/consumer_impl.go

+			if c.options.KeyReader != nil && c.options.MessageCrypto == nil {
+				logCtx := fmt.Sprintf("[%v] [%v]", pt, c.options.SubscriptionName)
+				messageCrypto, err := crypto.NewDefaultMessageCrypto(logCtx, false, c.log)
+				if err != nil {


why is an error ok here

Good catch. Let me fix it

cckellogg · 2021-06-24T16:54:55Z

pulsar/consumer_impl.go

+
+			// use default message crypto if not already created
+			if c.options.KeyReader != nil && c.options.MessageCrypto == nil {
+				logCtx := fmt.Sprintf("[%v] [%v]", pt, c.options.SubscriptionName)


what is log context and why is it needed?

It is just a log message that default crypto implementation uses while logging.

cckellogg · 2021-06-24T17:01:55Z

pulsar/consumer_partition.go

+		errMsg := fmt.Errorf(errDecryptingPayload, err.Error())
+		switch pc.options.consumerCryptoFailureAcrion {
+		case crypto.FailConsume:
+			pc.log.Warnf(


This info about the consumer will already be logged

pc.log = client.log.SubLogger(log.Fields{ "name": pc.name, "topic": options.topic, "subscription": options.subscription, "consumerID": pc.consumerID, })

Ok. I'll remove the redundant info

cckellogg · 2021-06-24T17:02:27Z

pulsar/crypto/DefaultCryptoKeyReader.go

+import "io/ioutil"
+
+// DefaultKeyReader default implementation of KeyReader
+type DefaultKeyReader struct {


FileKeyReader?

May be yeah that looks good. I'll rename it.

GPrabhudas · 2021-06-25T07:45:58Z

Thanks for your contribution. I took a quick look and left a few comments. A few suggestions:

In go file names are generally snake case so please try to follow that to keep things consistent. For example this crypto_key_reader.go instead of CryptoKeyReader.go

Can we break this up into multiple pull requests? Can we start with the crypto package that adds the interfaces and base implementations. Then some followup pull requests can be added for the consumer and producer implementation. This will make it easier on people to review the code.

Thanks for the suggestions. Actually I've gone too far to put all the functionality in the single PR. If it is absolutely necessary to breakdown into multiple pull requests I'll go ahead to do it.

- make proto internal

cckellogg · 2021-06-28T03:29:41Z

Yes, please break this down into multiple pull requests. The crypto package would be a good one to start with.

GPrabhudas · 2021-06-28T05:21:23Z

Motivation

Added Go client changes to encrypt messages published to pulsar. Once encrypted, it only allows the consumer with the right set of keys to be able to decrypt the original message published by the producer. Only RSA encryption of data key is supported.

Modifications

Changes are implemented as per PIP-4

Followed Java client Encryption support changes

Addresses issue #333

As per suggestions breaking down this into multiple pull requests.

#555 base interface and default implementation
Producer side changes
Consumer side changes

@cckellogg

Breakdown of PR [#552 ](#552) This PR includes base interface and default implementation.

GPrabhudas · 2021-07-08T09:58:14Z

Motivation

Added Go client changes to encrypt messages published to pulsar. Once encrypted, it only allows the consumer with the right set of keys to be able to decrypt the original message published by the producer. Only RSA encryption of data key is supported.

Modifications

Changes are implemented as per PIP-4
Followed Java client Encryption support changes
Addresses issue #333

As per suggestions breaking down this into multiple pull requests.

#555 base interface and default implementation

#560 Producer side changes

Consumer side changes

@cckellogg

wolfstudy · 2021-07-19T08:37:42Z

@cckellogg @GPrabhudas Any update for this? We are ready to initiate a new version of the release, hoping to include this new change in the new version (because some of the code has been merged in), can we consider advancing the progress here.

GPrabhudas · 2021-07-19T08:56:44Z

@cckellogg @GPrabhudas Any update for this? We are ready to initiate a new version of the release, hoping to include this new change in the new version (because some of the code has been merged in), can we consider advancing the progress here.

I'll work on it and address the review suggestions.

GPrabhudas · 2021-09-07T07:41:48Z

Motivation

Added Go client changes to encrypt messages published to pulsar. Once encrypted, it only allows the consumer with the right set of keys to be able to decrypt the original message published by the producer. Only RSA encryption of data key is supported.

Modifications

Changes are implemented as per PIP-4
Followed Java client Encryption support changes
Addresses issue #333

As per suggestions breaking down this into multiple pull requests.

#555 base interface and default implementation

#560 Producer side changes

#612 Consumer side changes

@cckellogg

wolfstudy · 2021-10-09T09:05:09Z

ping @cckellogg @GPrabhudas What is the latest status here?

GPrabhudas · 2021-10-09T09:11:08Z

ping @cckellogg @GPrabhudas What is the latest status here?

@wolfstudy We can close this PR as all of its sub PRs are merged into master.
#555 base interface and default implementation
#560 Producer side changes
#612 Consumer side changes

we can add tag 0.7.0 to above PRs and close this one.

wolfstudy · 2021-10-13T06:40:54Z

ping @cckellogg @GPrabhudas What is the latest status here?

@wolfstudy We can close this PR as all of its sub PRs are merged into master. #555 base interface and default implementation #560 Producer side changes #612 Consumer side changes

we can add tag 0.7.0 to above PRs and close this one.

Cool, i will close this pull request.

PGarule added 24 commits June 7, 2021 18:28

init changes encryption/decryption

532c009

- add interface to support encryption - provide default encryption/decryption support - add ability to add custom encryption/decryption of data key

encrypt & decrypt changes for producer/consumer

86aa9e2

- make initial changes in producer for encryption[needs refactoring] - make changes for decrypting in consumer

fix issue with compression

f087b73

refactor producer code

b17ccac

refactor consumer

65ff8c8

test cases

9b07f7b

- add test case to produce encrypted message and receive decrypted message

add more test cases

39a5ead

synchronize methods

328c3d5

schedule data key update

3f79b5e

fix enc key duplicate issue

0f88480

fix typo in comment

60f6424

remove from internal

51a9a9c

add comments

ffcb0b6

add rsa encryption

1b5958e

add ConsumerCryptoFailure action

3d3dafe

add ProducerCryptoFailure action

ed1f0af

rename and fixes crypto failure actions

6cca738

typo

b84775f

fix consumer issue with decryption

c4bd204

make pulsar proto public

4743be2

- adding encryption & decryption needs to have access to MessageMetadata - so making the pulsar proto package public from internal

fix lint warnings

adcd9fb

add more test cases

546279a

add fixes and test cases

f3af14d

add producer fail test case

4aac581

cckellogg reviewed Jun 24, 2021

View reviewed changes

PGarule added 2 commits June 25, 2021 18:57

address review feedback

bd384f7

- make proto internal

remove comments

b53c5c3

GPrabhudas mentioned this pull request Jun 28, 2021

add base package for encryption #555

Merged

wolfstudy assigned GPrabhudas Jun 29, 2021

wolfstudy requested review from merlimat, wolfstudy and cckellogg June 29, 2021 03:33

wolfstudy added the type/feature label Jun 29, 2021

wolfstudy added this to the 0.6.0 milestone Jun 29, 2021

wolfstudy pushed a commit that referenced this pull request Jul 5, 2021

add base package for encryption (#555)

7a60a7e

Breakdown of PR [#552 ](#552) This PR includes base interface and default implementation.

GPrabhudas mentioned this pull request Jul 8, 2021

Encryption support producer #560

Merged

wolfstudy modified the milestones: 0.6.0, 0.7.0 Jul 21, 2021

GPrabhudas mentioned this pull request Sep 6, 2021

Encryption support ext consumer #612

Merged

wolfstudy closed this Oct 13, 2021

		"github.com/apache/pulsar-client-go/pulsar/log"
		pb "github.com/apache/pulsar-client-go/pulsar/pulsar_proto"

Encryption support in Go client #552

Encryption support in Go client #552

Uh oh!

Conversation

GPrabhudas commented Jun 24, 2021

Motivation

Modifications

Uh oh!

cckellogg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GPrabhudas Jun 25, 2021 • 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!

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!

GPrabhudas Jun 25, 2021 • 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!

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!

GPrabhudas commented Jun 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cckellogg commented Jun 28, 2021

Uh oh!

GPrabhudas commented Jun 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Modifications

Uh oh!

GPrabhudas commented Jul 8, 2021

Motivation

Modifications

Uh oh!

wolfstudy commented Jul 19, 2021

Uh oh!

GPrabhudas commented Jul 19, 2021

Uh oh!

GPrabhudas commented Sep 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Modifications

Uh oh!

wolfstudy commented Oct 9, 2021

Uh oh!

GPrabhudas commented Oct 9, 2021

Uh oh!

wolfstudy commented Oct 13, 2021

Uh oh!

Uh oh!

GPrabhudas Jun 25, 2021 •

edited

Loading

GPrabhudas Jun 25, 2021 •

edited

Loading

GPrabhudas commented Jun 25, 2021 •

edited

Loading

GPrabhudas commented Jun 28, 2021 •

edited

Loading

GPrabhudas commented Sep 7, 2021 •

edited

Loading