Agent service utility to acquire trusted ConnectionInfo from connecting client node

[tegefaulkes]

Specification

In some AgentService handlers we need to be able to verify the connecting node information such as the NodeId, certificates and connection information. Since the ForwardProxy<->ReverseProxy connection uses mTLS to ensure a secure connection we can obtain this trusted information from the ReverseProxy.

We need to copy and adapt the authenticator implementation from the client/utils. it should be called getTrustedConnectionInfo. This needs to fetch the connection info from the ReverseProxy. The ReverseProxy provides the ReverseProxy.getConnectionInfoByProxy() method for fetching this information. The information will take this form.

type ConnectionInfo = {  
 nodeId: NodeId;  
 certificates: Array<Certificate>;  
 egressHost: Host;  
 egressPort: Port;  
 ingressHost: Host;  
 ingressPort: Port;  
};

Within the handler we can obtain the the host and port of the incoming address by using call.getPeer() This will return the address information in the form of host:port. This can be used to look up the information from the ReverseProxy.

Usage

The fix for this introduced a connectionInformationGetter to the agent service container. This allows you to access it within the service handlers by doing ...

function echo({
  connectionInfoGetter,
}: {
  connectionInfoGetter: ConnectionInfoGetter;
}) {
  return async (
    call: grpc.ServerUnaryCall<utilsPB.EchoMessage, utilsPB.EchoMessage>,
    callback: grpc.sendUnaryData<utilsPB.EchoMessage>,
  ): Promise<void> => {
    const connectionInfo = connectionInfoGetter(call);
    if ( connectionInfo == null) throw new agentErrors.ErrorConnectionInfoMissing();
   // ...
  };
}

If it can't find the respective connectionInfo connectionInfoGetter will return undefined but during normal operation when connecting through the proxies this should never happen. You can't obtain any information when connecting directly bypassing proxies. this only happens in testing so far.

connectionInfoGetter takes the call as a parameter. It uses call.getPeer() to get the connection info. This should be parsed as a URL to handle it more robustly. The format of call.getPeer() is usually Host:Port but it can be protocol://Host:Port.

When using connectionInfoGetter we need an error for when we fail to obtain the ConnectionInfo. ErrorConnectionInfoMissing error needs to be created for this. Likely use the sysexits.UNAVAILABLE exit code in this case.

Additional context

• Relevant discussion found from comments Redesign usage of CDSS with VaultInternal #305 (comment) to Redesign usage of CDSS with VaultInternal #305 (comment).
• Session Bin Test Fixes #296 (comment)
• Related Introducing vault sharing and the restrictions of pulling and cloning with vault permissions #266
• Blocking Redesign usage of CDSS with VaultInternal #305

Tasks

• 1. create a getTrustedConnectionInfo utility that mimics the use of authenticator in client/utils.
  • It needs to take in the call directly as a parameter.
  • It needs to handle the peer information as a URL
  • when used if the connectionInfo is required by the handler. It should throw an ErrorConnectionInfoMissing error.
  • It needs to fetch and return the connections trusted ConnectionInfo from ReverseProxy.getConnectionInfoByProxy()
  • It needs to be provided to the service handlers via the createService(container) parameter similar to how the client authenticator works
• 2. testing
  • We can obtain the connection information
  • Add test for multiple connections at a time to check if the proxy information is different for each.

[CMCDragonkai]

@tegefaulkes as explained the server interceptor is not available. So you have to do something like the authenticator utility that is DIed into the agent services.

[tegefaulkes]

Just a note, we need to use revProxy.getConnectionInfoByProxy().

[CMCDragonkai]

The ReverseProxy supports both getConnectionInfoByProxy and getConnectionByEgress.

The GRPC handler doesn't know the egress host and port information, it only knows the reverse proxy's host and port information.

Assuming that the reverse proxy creates new ephemeral ports https://en.wikipedia.org/wiki/Ephemeral_port for every connection it establishes with the GRPC server, this would mean the GRPC handlers can use this as the index to request the connection info of the client node.

On the GRPC handler, you need to use: call.getPeer() which works for unary calls and streams.

The getPeer though returns one of these 2:

dns:127.0.0.1:4444
127.0.0.1:3455

The reason is that prior to resolution, it may have the dns prefix. After resolution it i sjust IP.

The address is "resolved" after the call is awaited. However this is primarily from the client side. So on the server side, it may always be dns. Not sure.

Either way the parser to should strip it before extracting the IP and host.

I'm not sure what it looks like from IPv6 perspective, perhaps we need to use a URL parser in this case too?

[CMCDragonkai]

We need to test if this works by verifying that ReverseProxy assigns ephemeral ports to any connections it proxies to the GRPCServer.

[tegefaulkes]

@CMCDragonkai Since this issue is related to issues in other PRs. Is this getting addressed here by me or fixed along with the other issues discussed before?

[CMCDragonkai]

It's best you take over this issue. Remember to spec out the tasks first. You may want to create a separate PR first.

[tegefaulkes]

Old spec

Copying the old spec here since it has some useful context information.

We need to be able to check the MTLS of a Node the see if it is trusted when handling Agent GRPC requests. Currently this is required by vault clone/pull to verify the permissions of the Node making the request. This information can be obtained from the ReverseProxy. However the services have no easy way to obtain this information.

refer to this diagram https://excalidraw.com/#room=57543403694c95eba2e9,5qJFVQlpWc0vegUgEAQIpA which is regarding the session token usage and https://github.com/MatrixAI/js-polykey/wiki/network. It is implemented through the sessionInterceptor.

We need to add a new sessionInterceptor called connectionInfoInterceptor. It should be calling ReverseProxy.getConnectionInfoByEgress() automatically and augmenting the request message leading metadata. The new metadata should use Forwarded-For or related headers as it's basically the same idea as in HTTP1.1 reverse proxies.

GRPC upstream has discussion about adding server interceptors: grpc/grpc-node#419. This is why the authenticator function exists. It plugs a function into the handlers for the handlers to authenticate easily. Then the solution is to adapt the authenticator in client/utils to agent/utils instead. Because what you want here is a function that will give you the connection information.

[tegefaulkes]

While prototyping and exploring the problem I found that the agent/GRPCClientAgent.test.ts tests were not testing the connection through the proxies. I don't know if that was intentional but to test the changes in this issue we need to do the connection through the proxies.

[tegefaulkes]

ForwardProxy egressHost defaults to 0.0.0.0. egressHost = '0.0.0.0' as Host,. Is this right? I'd expect it to be 127.0.0.1 Like the others. But looking at the default config egress and ingress default to 0.0.0.0. Why is this?

[CMCDragonkai]

The egressHost is a bit of a strange thing. It's binding on all interfaces so it can send out on all interfaces. I think that's why it's 0.0.0.0. If it only bound on localhost it would only be able to send out packets on localhost, and that's obviously wrong.

We would actually default on :: when our network domain supports IPv6 but our utp-native library seems to cough on that.

[CMCDragonkai]

And no we don't default on 127.0.0.1 for ingress host because we want it to default to listening on all interfaces.

[CMCDragonkai]

While prototyping and exploring the problem I found that the agent/GRPCClientAgent.test.ts tests were not testing the connection through the proxies. I don't know if that was intentional but to test the changes in this issue we need to do the connection through the proxies.

You want to keep those tests that only test non network usage. Unit tests are supposed to be less integrationy.

You can also additional integration tests, but don't remove the tests that aren't using the network domain.

[tegefaulkes]

Right now I've updated agent/GRPCClientAgent.test.ts to use the proxies for all of the tests. Should I change that back?
I do need at least one test testing a call through the proxies to test connectionInfoGetter. Should the extra proxy setup only be done for that one test? Should that test be part of GRPCClientAgent.test.ts or somewhere else?

[CMCDragonkai]

I would say you should create a separate set of test functions to test with proxies. This is where a nested describe could be used. But make sure it's at the very end.

Also isn't GRPCClientAgent.test.ts being modified by @emmacasolin in #311? Becareful about that.

[emmacasolin]

Also isn't GRPCClientAgent.test.ts being modified by @emmacasolin in #311? Becareful about that.

Yes, but only fixing the imports and removing the nodesCrossSignClaim tests into their own file.

[tegefaulkes]

Two things that need to be expanded on here.

• Add test for multiple connections at a time to check if the proxy information is different for each.
• the output of call.getPeer() can be a URL so we need to handle that.
• We need to handle getting the NodeId and cert information if a connection is made directly bypassing the proxies.

[CMCDragonkai]

This utility function's input type should be a string representing a "possible" URL provided by call.getPeer(). And the output type should be:

{ nodeId: NodeId; certificates: Array<Certificate> } | undefined

For the input, because it may be a URL because call.getPeer() may return dns://127.0.0.1:55555 and I'm not sure but IPv6 may be a URL too, or may just return 127.0.0.1:55555, you would need to do a prefix match using a regex like ^.+:\/\/, and if it doesn't match you would prefix it with pk:// and just pass it into a new URL so it can do the parsing for you. Then you can get the host and port.

Subsequently because the GRPCServer itself may be the one that is secured, you would want to do a little condition like this:

if (grpcServer.secured) {
  const certs = grpcServer.getClientCertificates(session);
  // process the certs
} else {
  const connInfo = revProxy.getConnectionInfoByEgress(host, port);
  // process the connInfo
}

Note that in both cases, you may get undefined.

The type of GRPCServer.getClientCertificate and GRPCServer.getClientCertificates is wrong because if the client doesn't supply a certificate, that is one of the modes of TLS where it's only server side TLS, but not client side TLS which means it's not mutual. In this case, simply no certificates are available for the client. Therefore those 2 methods should have | undefined as an additional type. See the comment // Client connected without providing certs. (Remove the . @tegefaulkes).

Finally if no certs/nodeid is provided, then undefined must be returned.

The GRPC service handlers must then decide what to do in that case. 2 additional exceptions should be created in agent/errors.ts:

1. ErrorAgentAuthDenied - used when provided, but the wrong one
2. ErrorAgentAuthMissing - used when undefined but required...

In the case of vaultsScan, none of those should exist because it can just return an empty array. Or an empty stream.

There is an additional problem, where do you get the session for the grpcServer.getClientCertificates call?

This is a hack provided by grpcUtils.getClientSession, but this only works on the client side. I'm pretty sure I worked this out a long time ago but forgot how to extract the session now.

[tegefaulkes]

The URL aspect needs to be fixed. but is getting the information from the GRPCServer strictly needed right now? It's an expansion on this issue and I don't think it fits into the scope of the #266 PR work. I suggest we can move it into a new issue for later.

[CMCDragonkai]

Yea it can moved till later. But you should make the necessary transformation of the output type. Atm I can't even find how to access the client certificate on the server side, because I can't get access to the session object. I'm looking through the past.

[CMCDragonkai]

I think it's call.stream.session.

So:

// @ts-ignore
grpcServer.getClientCertificates(call.stream.session);

Should be sufficient to get you the client certificates.

I think adding getServerSession utility util similar to the getClientSession util would be suitable to remember how to do it.

[CMCDragonkai]

Try this:

/**
 * Acqure the HTTP2 session for a GRPC connection from the server side
 * This relies on monkey patching the gRPC library internals
 * The `ServerSurfaceCall` is expected to be an instance of `Http2ServerCallStream`
 * It will contain `stream` property, which will contain the `session` property
 */
function getServerSession(
  call: ServerSurfaceCall
): Http2Session {
  // @ts-ignore
  return call.stream.session;
}

I'll keep it in my branch in the mean time, but when you get around to it see if it works.

Then I think your utility function can be simplified to and it will use the call property internally to do everything:

authenticateTLS(call)

Note the diff between authenticator and authenticate. We can do the same here with authenticatorTLS and authenticateTLS.

I just checked the authenticate call itself, and it does throw new clientErrors.ErrorClientAuth....

The main difference here is that agent service handlers have to be more flexible since you may deal return different results depending on if you know who the contacting agent is.

[tegefaulkes]

Reminder for me to create a new issue for expanding the functionality of connectionInfoGetter.

Also, do we really want to call it authenticatorTLS and authenticateTLS? It's not technically authenticating anything. It's just retrieving the information. Maybe connectionInfoGetter/connectionInfoGet?

[tegefaulkes]

This has been updated with the URL changes. the rest of the changes has been moved into issue #355

[CMCDragonkai]

It's sort of propagating authentication information via TLS. So it is form of authentication with respect to the handlers.

[CMCDragonkai]

Try this:

/**
 * Acqure the HTTP2 session for a GRPC connection from the server side
 * This relies on monkey patching the gRPC library internals
 * The `ServerSurfaceCall` is expected to be an instance of `Http2ServerCallStream`
 * It will contain `stream` property, which will contain the `session` property
 */
function getServerSession(
  call: ServerSurfaceCall
): Http2Session {
  // @ts-ignore
  return call.stream.session;
}

I'll keep it in my branch in the mean time, but when you get around to it see if it works.

Then I think your utility function can be simplified to and it will use the call property internally to do everything:

authenticateTLS(call)

Note the diff between authenticator and authenticate. We can do the same here with authenticatorTLS and authenticateTLS.

I just checked the authenticate call itself, and it does throw new clientErrors.ErrorClientAuth....

The main difference here is that agent service handlers have to be more flexible since you may deal return different results depending on if you know who the contacting agent is.

I actually cherry picked this to master.

[tegefaulkes]

Added the test with multiple connections. Looks like it's working fine. each connection has it's own NodeId and egressPort.