Introduce channel Attributes; Use them to pass the binder target user to the NameResolver

[jdcormie]

In multi-user Android, the target user for an Intent is unfortunately not part of the intent itself. Instead, it's passed around as a separate argument wherever that Intent is needed. Following suit, the binder grpc transport accepts the target Android user as a parameter to BinderChannelBuilder -- unfortunately it's not in the target URI or the direct SocketAddress, despite being part of the server's location.

But downstream plugins such as NameResolvers need access to this UserHandle too. This change introduces a new Channel Attributes mechanism and uses it to expose the Channel's target user (for use in an upcoming NameResolverProvider).

[ejona86]

We're trying to remove most usages of Attributes. In general, we are replacing them with the same pattern of having a reference-equality Key with a generic for typing.

In this specific case, I'm still favoring exposing ChannelCredentials to the NameResolver, and adding the android context to the ChannelCredentials.

[jdcormie]

We're trying to remove most usages of Attributes. In general, we are replacing them with the same pattern of having a reference-equality Key with a generic for typing.

Can you point me to an example of this I can emulate?

In this specific case, I'm still favoring exposing ChannelCredentials to the NameResolver, and adding the android context to the ChannelCredentials.

But this PR isn't about the Android Context at all, it's about the target UserHandle which definitely doesn't belong in ChannelCredentials.

[ejona86]

We're trying to remove most usages of Attributes. In general, we are replacing them with the same pattern of having a reference-equality Key with a generic for typing.

Can you point me to an example of this I can emulate?

All the "Key"s in the API. CallOptions, Attributes, Context, CreateSubchannelArgs. Where they look different, there is probably an open issue to change one of them.

But this PR isn't about the Android Context at all, it's about the target UserHandle which definitely doesn't belong in ChannelCredentials.

This is the destination identity? And this is for cross-user binder calls? I guess the system might do some of that. UserHandle can be created from a uid, but once created there's no way to learn anything about it; you can only pass it to the system?

How commonly would this be used?

[jdcormie]

This is the destination identity? And this is for cross-user binder calls? I guess the system might do some of that.

Correct, it's the destination Android user for cross-user binder Channels (issue #10173, dd: go/cross-user-ods). And yes it is mostly used by Android system apps because ordinary apps don't normally have permission to reach outside the per-user sandboxing. Our team's is using this in production today. I presume original author wwtbuaa01 is using it too. Not sure who else.

The missing piece, though, is that the NameResolver needs a way to know the target user if we're going to have a single instance of cs/symbol:AndroidIntentNameResolverProvider as you requested. Because that information is per-Channel.

UserHandle can be created from a uid, but once created there's no way to learn anything about it; you can only pass it to the system?

Yeah UserHandle mostly is just a handle that you pass to other Android APIs. In particular, cs/symbol:AndroidIntentNameResolverProvider needs it to call hidden system API PackageManager.queryIntentServicesAsUser.

[jdcormie]

We're trying to remove most usages of Attributes. In general, we are replacing them with the same pattern of having a reference-equality Key with a generic for typing.

Can you point me to an example of this I can emulate?

All the "Key"s in the API. CallOptions, Attributes, Context, CreateSubchannelArgs. Where they look different, there is probably an open issue to change one of them.

OK happy to do this if you are on board with the essence of this change.

[ejona86]

I spoke to the TLs of the other languages on Friday. The options seem to be channel options (similar to discussed here) and constructing the NameResolverProvider with the UserHandle to pass to channel construction (as in https://github.com/grpc/grpc-java/issues/11055#issuecomment-2115872442).

Since this is only for system apps, I'd favor not creating new infrastructure to support it, so I'm preferential to "pass the UserHandle when manually creating the NR provider."

[jdcormie]

I've thought about it and favor the approach in this PR because it:

1. fits with the existing/intended architecture of a single global NRR with one shared NRP of each type. You've convinced me that creating an NRR/NRP per Channel is pretty strange since the target URI is fixed and so there's no need for a registry or scheme lookup. Sharing NRPs is actually helpful for intent:/// resolution because all NRs are interested in the same Android ACTION_PACKAGE_CHANGED event and so they could all very naturally share a single BroadcastReceiver.
2. lets the Channel creator specify the target UserHandle just once. Your proposal requires specifying it redundantly: once to BinderChannelBuilder#bindAsUser() and again in the NRP constructor. If the creator forgets one or the other, things fail mysteriously, or worse, appear to work sometimes but fail in the case where different Android users have different servers enabled/disabled (less common).
3. lets my NRP be @Internal, just like all the others, giving me more freedom to change things later. Your proposal forces me to make the new NRP public so that users can construct it.
4. could be refactored to look more like grpc-core's channel options, as you point out, although I'd like to hear more about how you think this would look.

Can you say more about your objection to adding the minimal Channel -> NRP infrastructure that I need here?

[jdcormie]

Ping?

[jdcormie]

Replaced by https://github.com/grpc/grpc-java/pull/11669