Skip to content

Conversation

@danielocallaghan
Copy link

Fix MethodDescriptor lookup in ManualGrpcSecurityMetadataSource for gRPC 1.75.0+

Problem

With gRPC 1.75.0+, MethodDescriptor instances from ProtoReflectionService don't match instances from ServiceDescriptor.getMethods() when used as HashMap keys. This causes security configuration lookups to fail, resulting in ServerReflection and other services requiring authentication even when configured with AccessPredicate.permitAll().

Symptoms

  • ServerReflection unexpectedly requires authentication despite source.set(ServerReflectionGrpc.getServiceDescriptor(), AccessPredicate.permitAll())
  • Method-level security configurations are ignored
  • Debug logs show "No security config found for method, using default"

Root Cause

gRPC 1.75.0 changed MethodDescriptor equality/identity behavior. When ProtoReflectionService creates MethodDescriptor instances, they are not the same object instances returned by ServerReflectionGrpc.getServiceDescriptor(), causing HashMap lookups by object identity to fail.

The issue occurs in this lookup chain:

  1. GrpcSecurityConfiguration calls source.set(ServerReflectionGrpc.getServiceDescriptor(), AccessPredicate.permitAll())
  2. This stores configuration using MethodDescriptor instances from ServiceDescriptor.getMethods()
  3. At runtime, ProtoReflectionService uses different MethodDescriptor instances
  4. HashMap lookup by object identity fails
  5. Security falls back to defaultAttributes (deny all)

Solution

Use method.getFullMethodName() (String) as the HashMap key instead of relying on MethodDescriptor object identity. This provides stable, reliable lookups across all MethodDescriptor instances for the same method.

Changes Made

  1. Added name-based lookup map:

    private final Map<String, Collection<ConfigAttribute>> accessMapByName = new HashMap<>();
  2. Updated getAttributes() method:

    • Try name-based lookup first (more reliable with gRPC 1.75.0+)
    • Fallback to object-based lookup for backward compatibility
    • Added debug logging for troubleshooting
  3. Updated all mutation methods:

    • set(ServiceDescriptor, AccessPredicate) - populates both maps
    • set(MethodDescriptor, AccessPredicate) - populates both maps
    • remove(ServiceDescriptor) - removes from both maps
    • remove(MethodDescriptor) - removes from both maps
  4. Updated getAllConfigAttributes():

    • Combines both maps to ensure all configurations are returned

Backward Compatibility

Fully backward compatible - The fix maintains both lookup mechanisms (name-based and object-based) to ensure existing code continues to work. The name-based lookup is attempted first as it's more reliable with gRPC 1.75.0+, with fallback to object-based lookup.

Testing

Verified with gRPC 1.75.0 that:

  • ✅ ServerReflection works without authentication when configured with permitAll()
  • ✅ Health checks continue to work as expected
  • ✅ Authenticated services still require authentication
  • ✅ Both lookup mechanisms function correctly
  • ✅ Debug logging helps identify lookup path taken

Test Case

@Test
fun `calling the server reflection works without authentication`() {
    val responseObserver: StreamObserver<ServerReflectionResponse> = mock()
    val requestObserver = serverReflectionStub.serverReflectionInfo(responseObserver)

    requestObserver.onNext(
        ServerReflectionRequest.newBuilder()
            .setListServices("")
            .build(),
    )
    requestObserver.onCompleted()

    verify(responseObserver, timeout(10000)).onNext(
        argThat { response -> response.hasListServicesResponse() }
    )
    verify(responseObserver, timeout(10000)).onCompleted()
}

Impact

Who is affected?

  • Projects using gRPC 1.75.0+ with grpc-spring-boot-starter 3.1.0.RELEASE
  • Any code using ManualGrpcSecurityMetadataSource with method-level security configuration

Benefits

  • Fixes ServerReflection authentication issue with gRPC 1.75.0+
  • Makes security configuration more robust across gRPC versions
  • Better debugging through added logging

Risks

  • Very low - maintains full backward compatibility with dual lookup mechanism

Related Issues

This issue is related to the gRPC 1.75.0 release and ProtoReflectionService deprecation:

  • gRPC 1.75.0 deprecated ProtoReflectionService in favor of ProtoReflectionServiceV1
  • MethodDescriptor identity behavior changed between versions
  • Similar issues may affect other security configurations relying on MethodDescriptor identity

Checklist

  • Code compiles without errors
  • Code follows project style (spotless applied)
  • Change is backward compatible
  • Tests pass with gRPC 1.75.0
  • Debug logging added for troubleshooting
  • Maintainer review

Screenshots/Logs

Before fix - ServerReflection requires authentication:

2025-10-23 DEBUG ManualGrpcSecurityMetadataSource - No security config found for method grpc.reflection.v1alpha.ServerReflection/ServerReflectionInfo, using default

After fix - ServerReflection works without authentication:

2025-10-23 DEBUG ManualGrpcSecurityMetadataSource - Found security config for method by name: grpc.reflection.v1alpha.ServerReflection/ServerReflectionInfo

…RPC 1.75.0+

## Problem
With gRPC 1.75.0+, MethodDescriptor instances from ProtoReflectionService
don't match instances from ServiceDescriptor.getMethods() when used as
HashMap keys. This causes security configuration lookups to fail, resulting
in ServerReflection and other services requiring authentication even when
configured with AccessPredicate.permitAll().

## Root Cause
gRPC 1.75.0 changed MethodDescriptor equality/identity behavior. When
ProtoReflectionService creates MethodDescriptor instances, they are not
the same object instances returned by ServerReflectionGrpc.getServiceDescriptor(),
causing HashMap lookups by object identity to fail.

## Solution
Use method.getFullMethodName() (String) as the HashMap key instead of
relying on MethodDescriptor object identity. This provides stable,
reliable lookups across all MethodDescriptor instances for the same method.

Changes:
- Added accessMapByName HashMap using String (fullMethodName) as key
- Updated getAttributes() to try name-based lookup first
- Maintained backward compatibility with existing object-based lookup
- Updated all set() and remove() methods to populate both maps
- Added debug logging for troubleshooting

## Backward Compatibility
The fix maintains both lookup mechanisms (name-based and object-based)
to ensure existing code continues to work. The name-based lookup is
attempted first as it's more reliable with gRPC 1.75.0+.

## Testing
Verified with gRPC 1.75.0 that:
- ServerReflection works without authentication when configured with permitAll()
- Health checks continue to work as expected
- Authenticated services still require authentication
- Both lookup mechanisms function correctly
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants