Repository navigation
HBASE-28952: Add coprocessor hook to authorize user based on client SSL certificate chain - #6447
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Maybe we could introduce something like a RpcObserver? So we do not need to do the instance of MasterObserver and RegionServerObserver. |
Makes sense to me. Let me give it a try. |
|
@Apache9 Does that need the entire new co-proc infra with: RpcCoprocessor, RpcCoprocessorEnvironment, RpcObserver, RpcCoprocessorHost, etc.? |
|
I guess we need to introduce all these things. You can have a try first, if we think introducing so many new classes does not make sense, we could discuss again. Thanks. |
368411f to
81b70ae
Compare
|
@Apache9 Check it out. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| CoprocessorHost.RPC_COPROCESSOR_CONF_KEY) | ||
| ) { | ||
| LOG.info("Update the RPC coprocessor(s) because the configuration has changed"); | ||
| initializeCoprocessorHost(newConf); |
There was a problem hiding this comment.
Is a Configuration object enough for your usage? I see the RpcCoprossesorEnvironment is also empty?
There was a problem hiding this comment.
Strictly for this particular hook we don't need anything else. Configuration object is needed to instantiate Observers, the rest is passed in method arguments.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| } | ||
| this.user = this.rpcServer.userProvider.create(this.ugi); | ||
| this.rpcServer.getRpcCoprocessorHost().postAuthorizeConnection( | ||
| this.user != null ? this.user.getName() : null, this.clientCertificateChain); |
There was a problem hiding this comment.
should we send the user in directly and let the implementer decide if they want the name or something else?
There was a problem hiding this comment.
That makes sense too to me. I'll make this change too.
But:
Isn't that a problem to introduce too many dependencies required to pull in by coprocessors?
User is in hbase-common, ConnectionHeader is in hbase-protocol-shaded, the implementor has to reference all these to compile the co-proc, but since they're generic hbase projects, it might not be a problem at all. I'm not sure.
There was a problem hiding this comment.
In MasterObserver only the username is passed as String to coprocessor.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@Apache9 what do you think of the current state? see anything outstanding? |
|
ping @Apache9 @bbeaudreault |
5d4f959 to
634047b
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
634047b to
fd7d07e
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
bbeaudreault
left a comment
bbeaudreault
left a comment
There was a problem hiding this comment.
I'm not an expert in the coprocessor interface, but I don't have any issues here. Looks pretty straightforward.
In terms of user vs username, is there any way for someone to get the user statically if they wanted? If this were a downstream coprocessor, they could use RpcServer.getCurrentCall() but not sure if that's available at this point.
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
Thanks @bbeaudreault . Build is now green. I'm waiting for your approval @Apache9 . |
|
Ping @apurtell |
…L certificate chain (#6447) Signed-off-by: Bryan Beaudreault <[email protected]> (cherry picked from commit d477bf1)
Added new coproc hook to
ServerRpcConnectionright after authorization is done. Support both Master and RS client connections. Coprocs are allowed to throwAccessDeniedExceptionif connection should not be established. Targeted primarly for X509 cert based authentication.Example co-proc usage PR is available to see it in action: anmolnar#2
cc @bbeaudreault @apurtell