You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/06/24 10:54:27 UTC
[GitHub] [pulsar-client-go] GPrabhudas opened a new pull request #552: Encryption support in Go client
GPrabhudas opened a new pull request #552:
URL: https://github.com/apache/pulsar-client-go/pull/552
### 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](https://github.com/apache/incubator-pulsar/wiki/PIP-4:-Pulsar-End-to-End-Encryption)
Followed Java client [Encryption support changes](https://github.com/apache/pulsar/pull/731)
Addresses issue [#333](https://github.com/apache/pulsar-client-go/issues/333)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] wolfstudy commented on pull request #552: Encryption support in Go client
Posted by GitBox <gi...@apache.org>.
wolfstudy commented on pull request #552:
URL: https://github.com/apache/pulsar-client-go/pull/552#issuecomment-941971670
> > 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] wolfstudy closed pull request #552: Encryption support in Go client
Posted by GitBox <gi...@apache.org>.
wolfstudy closed pull request #552:
URL: https://github.com/apache/pulsar-client-go/pull/552
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] GPrabhudas commented on a change in pull request #552: Encryption support in Go client
Posted by GitBox <gi...@apache.org>.
GPrabhudas commented on a change in pull request #552:
URL: https://github.com/apache/pulsar-client-go/pull/552#discussion_r658541845
##########
File path: pulsar/crypto/DefaultCryptoKeyReader.go
##########
@@ -0,0 +1,51 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package crypto
+
+import "io/ioutil"
+
+// DefaultKeyReader default implementation of KeyReader
+type DefaultKeyReader struct {
Review comment:
May be yeah that looks good. I'll rename it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] GPrabhudas edited a comment on pull request #552: Encryption support in Go client
Posted by GitBox <gi...@apache.org>.
GPrabhudas edited a comment on pull request #552:
URL: https://github.com/apache/pulsar-client-go/pull/552#issuecomment-868295571
> 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 more people insist to breakdown into multiple pull requests I'll go ahead to do it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] GPrabhudas commented on a change in pull request #552: Encryption support in Go client
Posted by GitBox <gi...@apache.org>.
GPrabhudas commented on a change in pull request #552:
URL: https://github.com/apache/pulsar-client-go/pull/552#discussion_r658542930
##########
File path: pulsar/consumer_impl.go
##########
@@ -304,25 +305,40 @@ func (c *consumer) internalTopicSubscribeToPartitions() error {
} else {
nackRedeliveryDelay = c.options.NackRedeliveryDelay
}
+
+ // 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)
+ messageCrypto, err := crypto.NewDefaultMessageCrypto(logCtx, false, c.log)
+ if err != nil {
Review comment:
Good catch. Let me fix it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] GPrabhudas commented on pull request #552: Encryption support in Go client
Posted by GitBox <gi...@apache.org>.
GPrabhudas commented on pull request #552:
URL: https://github.com/apache/pulsar-client-go/pull/552#issuecomment-882373105
> @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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] GPrabhudas commented on a change in pull request #552: Encryption support in Go client
Posted by GitBox <gi...@apache.org>.
GPrabhudas commented on a change in pull request #552:
URL: https://github.com/apache/pulsar-client-go/pull/552#discussion_r658529680
##########
File path: pulsar/consumer_impl.go
##########
@@ -25,9 +25,10 @@ import (
"sync"
"time"
+ "github.com/apache/pulsar-client-go/pulsar/crypto"
"github.com/apache/pulsar-client-go/pulsar/internal"
- pb "github.com/apache/pulsar-client-go/pulsar/internal/pulsar_proto"
"github.com/apache/pulsar-client-go/pulsar/log"
+ pb "github.com/apache/pulsar-client-go/pulsar/pulsar_proto"
Review comment:
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] wolfstudy commented on pull request #552: Encryption support in Go client
Posted by GitBox <gi...@apache.org>.
wolfstudy commented on pull request #552:
URL: https://github.com/apache/pulsar-client-go/pull/552#issuecomment-941971670
> > 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] GPrabhudas commented on a change in pull request #552: Encryption support in Go client
Posted by GitBox <gi...@apache.org>.
GPrabhudas commented on a change in pull request #552:
URL: https://github.com/apache/pulsar-client-go/pull/552#discussion_r658541350
##########
File path: pulsar/consumer_partition.go
##########
@@ -571,6 +627,114 @@ func (pc *partitionConsumer) MessageReceived(response *pb.CommandMessage, header
return nil
}
+func createEncryptionContext(msgMeta *pb.MessageMetadata) EncryptionContext {
+ encCtx := EncryptionContext{
+ Algorithm: msgMeta.GetEncryptionAlgo(),
+ Param: msgMeta.GetEncryptionParam(),
+ UncompressedSize: int(msgMeta.GetUncompressedSize()),
+ BatchSize: int(msgMeta.GetNumMessagesInBatch()),
+ }
+
+ if msgMeta.Compression != nil {
+ encCtx.CompressionType = CompressionType(*msgMeta.Compression)
+ }
+
+ kMap := map[string]EncryptionKey{}
+ for _, k := range msgMeta.GetEncryptionKeys() {
+ metaMap := map[string]string{}
+ for _, m := range k.GetMetadata() {
+ metaMap[*m.Key] = *m.Value
+ }
+
+ kMap[*k.Key] = EncryptionKey{
+ KeyValue: k.GetValue(),
+ Metadata: metaMap,
+ }
+ }
+
+ encCtx.Keys = kMap
+ return encCtx
+}
+
+func (pc *partitionConsumer) decryptPayLoadIfNeeded(msgID *pb.MessageIdData,
+ msgMeta *pb.MessageMetadata,
+ payload []byte) ([]byte, error) {
+
+ // No encryption keys found in message metadata, do not decrypt payload
+ if len(msgMeta.GetEncryptionKeys()) == 0 {
+ return payload, nil
+ }
+
+ // If KeyReader interface is not implemented, throw an error based on the configuration
+ if pc.options.KeyReader == nil {
+ switch pc.options.consumerCryptoFailureAcrion {
+ case crypto.Consume:
+ pc.log.Warnf("[%v][%v][%v] KeyReader interface is not implemented. Consuming encrypted message.",
+ pc.topic,
+ pc.options.subscription,
+ pc.options.consumerName,
+ )
+ return payload, errKeyReaderNotImplemted
+ case crypto.Discard:
+ pc.log.Warnf(`[%v][%v][%v] Skipping decryption since KeyReader
+ interface is not implemented and config is set to discard`,
+ pc.topic,
+ pc.options.subscription,
+ pc.options.consumerName,
+ )
+ pc.discardCorruptedMessage(msgID, pb.CommandAck_DecryptionError)
+ return nil, errKeyReaderNotImplemted
+ case crypto.FailConsume:
+ pc.log.Errorf(
+ `[%v][%v][%v][%v] Message delivery failed since KeyReader interface
+ is not implemented to consume encrypted message`,
+ pc.topic,
+ pc.options.subscription,
+ pc.options.consumerName,
+ msgID,
+ )
+ return nil, errKeyReaderNotImplemted
+ }
+ }
+
+ decryptedPayload, err := pc.options.messageCrypto.Decrypt(msgMeta, payload, pc.options.KeyReader)
+ if err != nil {
+ errMsg := fmt.Errorf(errDecryptingPayload, err.Error())
+ switch pc.options.consumerCryptoFailureAcrion {
+ case crypto.FailConsume:
+ pc.log.Warnf(
Review comment:
Ok. I'll remove the redundant info
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] GPrabhudas commented on pull request #552: Encryption support in Go client
Posted by GitBox <gi...@apache.org>.
GPrabhudas commented on pull request #552:
URL: https://github.com/apache/pulsar-client-go/pull/552#issuecomment-876303788
> > ### 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](https://github.com/apache/incubator-pulsar/wiki/PIP-4:-Pulsar-End-to-End-Encryption)
> > Followed Java client [Encryption support changes](https://github.com/apache/pulsar/pull/731)
> > Addresses issue [#333](https://github.com/apache/pulsar-client-go/issues/333)
>
> As per suggestions breaking down this into multiple pull requests.
>
> 1. [#555](https://github.com/apache/pulsar-client-go/pull/555) base interface and default implementation
> 2. [#560](https://github.com/apache/pulsar-client-go/pull/560) Producer side changes
> 3. Consumer side changes
@cckellogg
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] GPrabhudas commented on a change in pull request #552: Encryption support in Go client
Posted by GitBox <gi...@apache.org>.
GPrabhudas commented on a change in pull request #552:
URL: https://github.com/apache/pulsar-client-go/pull/552#discussion_r658529680
##########
File path: pulsar/consumer_impl.go
##########
@@ -25,9 +25,10 @@ import (
"sync"
"time"
+ "github.com/apache/pulsar-client-go/pulsar/crypto"
"github.com/apache/pulsar-client-go/pulsar/internal"
- pb "github.com/apache/pulsar-client-go/pulsar/internal/pulsar_proto"
"github.com/apache/pulsar-client-go/pulsar/log"
+ pb "github.com/apache/pulsar-client-go/pulsar/pulsar_proto"
Review comment:
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] wolfstudy commented on pull request #552: Encryption support in Go client
Posted by GitBox <gi...@apache.org>.
wolfstudy commented on pull request #552:
URL: https://github.com/apache/pulsar-client-go/pull/552#issuecomment-882360249
@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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] GPrabhudas commented on pull request #552: Encryption support in Go client
Posted by GitBox <gi...@apache.org>.
GPrabhudas commented on pull request #552:
URL: https://github.com/apache/pulsar-client-go/pull/552#issuecomment-869365766
> ### 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](https://github.com/apache/incubator-pulsar/wiki/PIP-4:-Pulsar-End-to-End-Encryption)
>
> Followed Java client [Encryption support changes](https://github.com/apache/pulsar/pull/731)
>
> Addresses issue [#333](https://github.com/apache/pulsar-client-go/issues/333)
As per suggestions breaking down this into multiple pull requests.
1. [#555](https://github.com/apache/pulsar-client-go/pull/555) base interface and default implementation
2. Producer side changes
3. Consumer side changes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] GPrabhudas commented on pull request #552: Encryption support in Go client
Posted by GitBox <gi...@apache.org>.
GPrabhudas commented on pull request #552:
URL: https://github.com/apache/pulsar-client-go/pull/552#issuecomment-939262293
> 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] wolfstudy closed pull request #552: Encryption support in Go client
Posted by GitBox <gi...@apache.org>.
wolfstudy closed pull request #552:
URL: https://github.com/apache/pulsar-client-go/pull/552
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] GPrabhudas commented on a change in pull request #552: Encryption support in Go client
Posted by GitBox <gi...@apache.org>.
GPrabhudas commented on a change in pull request #552:
URL: https://github.com/apache/pulsar-client-go/pull/552#discussion_r658540626
##########
File path: pulsar/consumer_impl.go
##########
@@ -304,25 +305,40 @@ func (c *consumer) internalTopicSubscribeToPartitions() error {
} else {
nackRedeliveryDelay = c.options.NackRedeliveryDelay
}
+
+ // 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)
Review comment:
It is just a log message that default interface uses logging.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] GPrabhudas commented on a change in pull request #552: Encryption support in Go client
Posted by GitBox <gi...@apache.org>.
GPrabhudas commented on a change in pull request #552:
URL: https://github.com/apache/pulsar-client-go/pull/552#discussion_r658529680
##########
File path: pulsar/consumer_impl.go
##########
@@ -25,9 +25,10 @@ import (
"sync"
"time"
+ "github.com/apache/pulsar-client-go/pulsar/crypto"
"github.com/apache/pulsar-client-go/pulsar/internal"
- pb "github.com/apache/pulsar-client-go/pulsar/internal/pulsar_proto"
"github.com/apache/pulsar-client-go/pulsar/log"
+ pb "github.com/apache/pulsar-client-go/pulsar/pulsar_proto"
Review comment:
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. Let me check if I can create a wrapper on it and expose it instead.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] GPrabhudas commented on pull request #552: Encryption support in Go client
Posted by GitBox <gi...@apache.org>.
GPrabhudas commented on pull request #552:
URL: https://github.com/apache/pulsar-client-go/pull/552#issuecomment-914068978
> > > ### 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](https://github.com/apache/incubator-pulsar/wiki/PIP-4:-Pulsar-End-to-End-Encryption)
> > > Followed Java client [Encryption support changes](https://github.com/apache/pulsar/pull/731)
> > > Addresses issue [#333](https://github.com/apache/pulsar-client-go/issues/333)
> >
> >
> > As per suggestions breaking down this into multiple pull requests.
> >
> > 1. [#555](https://github.com/apache/pulsar-client-go/pull/555) base interface and default implementation
> > 2. [#560](https://github.com/apache/pulsar-client-go/pull/560) Producer side changes
> > 3. [#612](https://github.com/apache/pulsar-client-go/pull/612)Consumer side changes
@cckellogg
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] wolfstudy commented on pull request #552: Encryption support in Go client
Posted by GitBox <gi...@apache.org>.
wolfstudy commented on pull request #552:
URL: https://github.com/apache/pulsar-client-go/pull/552#issuecomment-939261353
ping @cckellogg @GPrabhudas What is the latest status here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] cckellogg commented on pull request #552: Encryption support in Go client
Posted by GitBox <gi...@apache.org>.
cckellogg commented on pull request #552:
URL: https://github.com/apache/pulsar-client-go/pull/552#issuecomment-869323409
Yes, please break this down into multiple pull requests. The crypto package would be a good one to start with.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] GPrabhudas edited a comment on pull request #552: Encryption support in Go client
Posted by GitBox <gi...@apache.org>.
GPrabhudas edited a comment on pull request #552:
URL: https://github.com/apache/pulsar-client-go/pull/552#issuecomment-914068978
> > > ### 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](https://github.com/apache/incubator-pulsar/wiki/PIP-4:-Pulsar-End-to-End-Encryption)
> > > Followed Java client [Encryption support changes](https://github.com/apache/pulsar/pull/731)
> > > Addresses issue [#333](https://github.com/apache/pulsar-client-go/issues/333)
> >
> >
> > As per suggestions breaking down this into multiple pull requests.
> >
> > 1. [#555](https://github.com/apache/pulsar-client-go/pull/555) base interface and default implementation
> > 2. [#560](https://github.com/apache/pulsar-client-go/pull/560) Producer side changes
> > 3. [#612](https://github.com/apache/pulsar-client-go/pull/612) Consumer side changes
@cckellogg
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] cckellogg commented on a change in pull request #552: Encryption support in Go client
Posted by GitBox <gi...@apache.org>.
cckellogg commented on a change in pull request #552:
URL: https://github.com/apache/pulsar-client-go/pull/552#discussion_r658123901
##########
File path: pulsar/consumer_impl.go
##########
@@ -304,25 +305,40 @@ func (c *consumer) internalTopicSubscribeToPartitions() error {
} else {
nackRedeliveryDelay = c.options.NackRedeliveryDelay
}
+
+ // 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)
+ messageCrypto, err := crypto.NewDefaultMessageCrypto(logCtx, false, c.log)
+ if err != nil {
Review comment:
why is an error ok here
##########
File path: pulsar/consumer_impl.go
##########
@@ -25,9 +25,10 @@ import (
"sync"
"time"
+ "github.com/apache/pulsar-client-go/pulsar/crypto"
"github.com/apache/pulsar-client-go/pulsar/internal"
- pb "github.com/apache/pulsar-client-go/pulsar/internal/pulsar_proto"
"github.com/apache/pulsar-client-go/pulsar/log"
+ pb "github.com/apache/pulsar-client-go/pulsar/pulsar_proto"
Review comment:
We want to avoid exposing the proto as a public api. We should move this back to internal
##########
File path: pulsar/crypto/DefaultCryptoKeyReader.go
##########
@@ -0,0 +1,51 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package crypto
+
+import "io/ioutil"
+
+// DefaultKeyReader default implementation of KeyReader
+type DefaultKeyReader struct {
Review comment:
FileKeyReader?
##########
File path: pulsar/consumer_partition.go
##########
@@ -571,6 +627,114 @@ func (pc *partitionConsumer) MessageReceived(response *pb.CommandMessage, header
return nil
}
+func createEncryptionContext(msgMeta *pb.MessageMetadata) EncryptionContext {
+ encCtx := EncryptionContext{
+ Algorithm: msgMeta.GetEncryptionAlgo(),
+ Param: msgMeta.GetEncryptionParam(),
+ UncompressedSize: int(msgMeta.GetUncompressedSize()),
+ BatchSize: int(msgMeta.GetNumMessagesInBatch()),
+ }
+
+ if msgMeta.Compression != nil {
+ encCtx.CompressionType = CompressionType(*msgMeta.Compression)
+ }
+
+ kMap := map[string]EncryptionKey{}
+ for _, k := range msgMeta.GetEncryptionKeys() {
+ metaMap := map[string]string{}
+ for _, m := range k.GetMetadata() {
+ metaMap[*m.Key] = *m.Value
+ }
+
+ kMap[*k.Key] = EncryptionKey{
+ KeyValue: k.GetValue(),
+ Metadata: metaMap,
+ }
+ }
+
+ encCtx.Keys = kMap
+ return encCtx
+}
+
+func (pc *partitionConsumer) decryptPayLoadIfNeeded(msgID *pb.MessageIdData,
+ msgMeta *pb.MessageMetadata,
+ payload []byte) ([]byte, error) {
+
+ // No encryption keys found in message metadata, do not decrypt payload
+ if len(msgMeta.GetEncryptionKeys()) == 0 {
+ return payload, nil
+ }
+
+ // If KeyReader interface is not implemented, throw an error based on the configuration
+ if pc.options.KeyReader == nil {
+ switch pc.options.consumerCryptoFailureAcrion {
+ case crypto.Consume:
+ pc.log.Warnf("[%v][%v][%v] KeyReader interface is not implemented. Consuming encrypted message.",
+ pc.topic,
+ pc.options.subscription,
+ pc.options.consumerName,
+ )
+ return payload, errKeyReaderNotImplemted
+ case crypto.Discard:
+ pc.log.Warnf(`[%v][%v][%v] Skipping decryption since KeyReader
+ interface is not implemented and config is set to discard`,
+ pc.topic,
+ pc.options.subscription,
+ pc.options.consumerName,
+ )
+ pc.discardCorruptedMessage(msgID, pb.CommandAck_DecryptionError)
+ return nil, errKeyReaderNotImplemted
+ case crypto.FailConsume:
+ pc.log.Errorf(
+ `[%v][%v][%v][%v] Message delivery failed since KeyReader interface
+ is not implemented to consume encrypted message`,
+ pc.topic,
+ pc.options.subscription,
+ pc.options.consumerName,
+ msgID,
+ )
+ return nil, errKeyReaderNotImplemted
+ }
+ }
+
+ decryptedPayload, err := pc.options.messageCrypto.Decrypt(msgMeta, payload, pc.options.KeyReader)
+ if err != nil {
+ errMsg := fmt.Errorf(errDecryptingPayload, err.Error())
+ switch pc.options.consumerCryptoFailureAcrion {
+ case crypto.FailConsume:
+ pc.log.Warnf(
Review comment:
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,
})
```
##########
File path: pulsar/consumer_impl.go
##########
@@ -304,25 +305,40 @@ func (c *consumer) internalTopicSubscribeToPartitions() error {
} else {
nackRedeliveryDelay = c.options.NackRedeliveryDelay
}
+
+ // 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)
Review comment:
what is log context and why is it needed?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] GPrabhudas commented on pull request #552: Encryption support in Go client
Posted by GitBox <gi...@apache.org>.
GPrabhudas commented on pull request #552:
URL: https://github.com/apache/pulsar-client-go/pull/552#issuecomment-868295571
> 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. Let me break the code into multiple PRs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] GPrabhudas commented on a change in pull request #552: Encryption support in Go client
Posted by GitBox <gi...@apache.org>.
GPrabhudas commented on a change in pull request #552:
URL: https://github.com/apache/pulsar-client-go/pull/552#discussion_r658540626
##########
File path: pulsar/consumer_impl.go
##########
@@ -304,25 +305,40 @@ func (c *consumer) internalTopicSubscribeToPartitions() error {
} else {
nackRedeliveryDelay = c.options.NackRedeliveryDelay
}
+
+ // 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)
Review comment:
It is just a log message that default crypto implementaion uses while logging.
##########
File path: pulsar/consumer_impl.go
##########
@@ -304,25 +305,40 @@ func (c *consumer) internalTopicSubscribeToPartitions() error {
} else {
nackRedeliveryDelay = c.options.NackRedeliveryDelay
}
+
+ // 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)
Review comment:
It is just a log message that default crypto implementation uses while logging.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] GPrabhudas edited a comment on pull request #552: Encryption support in Go client
Posted by GitBox <gi...@apache.org>.
GPrabhudas edited a comment on pull request #552:
URL: https://github.com/apache/pulsar-client-go/pull/552#issuecomment-869365766
> ### 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](https://github.com/apache/incubator-pulsar/wiki/PIP-4:-Pulsar-End-to-End-Encryption)
>
> Followed Java client [Encryption support changes](https://github.com/apache/pulsar/pull/731)
>
> Addresses issue [#333](https://github.com/apache/pulsar-client-go/issues/333)
As per suggestions breaking down this into multiple pull requests.
1. [#555](https://github.com/apache/pulsar-client-go/pull/555) base interface and default implementation
2. Producer side changes
3. Consumer side changes
@cckellogg
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] GPrabhudas commented on pull request #552: Encryption support in Go client
Posted by GitBox <gi...@apache.org>.
GPrabhudas commented on pull request #552:
URL: https://github.com/apache/pulsar-client-go/pull/552#issuecomment-882373105
> @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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] GPrabhudas edited a comment on pull request #552: Encryption support in Go client
Posted by GitBox <gi...@apache.org>.
GPrabhudas edited a comment on pull request #552:
URL: https://github.com/apache/pulsar-client-go/pull/552#issuecomment-868295571
> 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] wolfstudy commented on pull request #552: Encryption support in Go client
Posted by GitBox <gi...@apache.org>.
wolfstudy commented on pull request #552:
URL: https://github.com/apache/pulsar-client-go/pull/552#issuecomment-882360249
@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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org