Skip to content

Commit

Permalink
[Security] Fix bug for layered composite creds (grpc#34860)
Browse files Browse the repository at this point in the history
Address grpc#12554

The API for `duplicate_without_call_credentials` says 
```
// Creates a version of the channel credentials without any attached call
// credentials. This can be used in order to open a channel to a non-trusted
// gRPC load balancer.
```

As the impl stands right now, because of that description, in the case
of layered composite creds, I think the right behavior would be to call
down until you get the base cred with no call cred.

In discussing with the team, we do wonder if the use-case of layered
composite creds is really something that should be a feature, or if we
should be checking during the creation of composite creds to make sure
we aren't layering composite creds? @markdroth can you give your
thoughts?
  • Loading branch information
gtcooke94 authored Nov 3, 2023
1 parent edfb038 commit 0b1e381
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class grpc_composite_channel_credentials : public grpc_channel_credentials {

grpc_core::RefCountedPtr<grpc_channel_credentials>
duplicate_without_call_credentials() override {
return inner_creds_;
return inner_creds_->duplicate_without_call_credentials();
}

grpc_core::RefCountedPtr<grpc_channel_security_connector>
Expand Down
16 changes: 16 additions & 0 deletions test/core/security/credentials_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3878,6 +3878,22 @@ TEST(CredentialsTest, TestCompositeChannelCredsCompareSuccess) {
grpc_channel_credentials_release(composite_creds_2);
}

TEST(CredentialsTest, RecursiveCompositeCredsDuplicateWithoutCallCreds) {
auto* insecure_creds = grpc_insecure_credentials_create();
auto inner_fake_creds = MakeRefCounted<fake_call_creds>();
auto outer_fake_creds = MakeRefCounted<fake_call_creds>();
auto* inner_composite_creds = grpc_composite_channel_credentials_create(
insecure_creds, inner_fake_creds.get(), nullptr);
auto* outer_composite_creds = grpc_composite_channel_credentials_create(
inner_composite_creds, outer_fake_creds.get(), nullptr);
auto duplicate_without_call_creds =
outer_composite_creds->duplicate_without_call_credentials();
EXPECT_EQ(duplicate_without_call_creds.get(), insecure_creds);
grpc_channel_credentials_release(insecure_creds);
grpc_channel_credentials_release(inner_composite_creds);
grpc_channel_credentials_release(outer_composite_creds);
}

TEST(CredentialsTest,
TestCompositeChannelCredsCompareFailureDifferentChannelCreds) {
auto* insecure_creds = grpc_insecure_credentials_create();
Expand Down

0 comments on commit 0b1e381

Please sign in to comment.