Skip to content

Support custom TLS handshake verify to origin server#4013

Closed
CrendKing wants to merge 1 commit intoapache:masterfrom
CrendKing:master
Closed

Support custom TLS handshake verify to origin server#4013
CrendKing wants to merge 1 commit intoapache:masterfrom
CrendKing:master

Conversation

@CrendKing
Copy link
Copy Markdown

In LinkedIn, we have use case, where we want to verify origin server's identity before sending any data. The verification consists of not only the standard server certificate being issued by trusted CA, but also other extension fields (such as Subject Alternative Name) matching criteria. This is viable because all server certificates are issued by our internal CA with standardized format.

Currently, ATS only verifies the server certificate against CA, specified at proxy.config.ssl.client.CA.cert.path and proxy.config.ssl.client.CA.cert.filename . This PR adds a new API TSVConnVerifyCallbackSet that allows caller to associate a callback when the TLS handshake is initiated. Internally it utilizes OpenSSL SSL_set_verify, same way as how SSLUtils.cc currently uses. Additionally, it allows caller to specify a pointer for user data. Note that TSVConnVerifyCallbackSet only sets the callback if the VConn is SSLNetVConnection, or no-op otherwise.

We are currently using this new API in one of our plugin to achieve the aforementioned requirement. We believe that other ATS users might have similar use cases to do stricter or looser verifications. In such case, we added an autest test case to demonstrate how to match DNS name in Subject Alternative Name.

Any comment or feedback is welcomed. Thank you!

@zizhong
Copy link
Copy Markdown
Member

zizhong commented Jul 26, 2018

[approve ci]

@CrendKing
Copy link
Copy Markdown
Author

Added the missing file header.

@bryancall
Copy link
Copy Markdown
Contributor

[approve ci]

@zizhong
Copy link
Copy Markdown
Member

zizhong commented Jul 26, 2018

[approve ci]

@shinrich
Copy link
Copy Markdown
Member

shinrich commented Jul 26, 2018

@persiaAziz implemented similar logic using the TS_EVENT_SSL_SERVER_VERIFY_HOOK callback which is currently on master. The documentation is a bit thin, but it is in the hook diagram on https://docs.trafficserver.apache.org/en/latest/developer-guide/plugins/hooks-and-transactions/index.en.html?highlight=hooks%20transactions

So you can register an extra continuation on the verify hook which will be called after the core hook. If the hook callback fails, the handshake fails.

The main code was added in commit 00cf3d3

@CrendKing
Copy link
Copy Markdown
Author

CrendKing commented Jul 27, 2018

Hi Susan. Thanks for pointing out the TS_EVENT_SSL_SERVER_VERIFY_HOOK work. I tried both to use example/verify_cert/verify_cert.cc and writing my own plugin to test that function (I already set CONFIG proxy.config.ssl.client.verify.server INT 2), but the callback is never invoked. It seems the current SSLNetVConnection.cc::callHooks() does not have code to handle the relevant event. Is there more work to be done on this feature?

To be frank, adding a new SSL hook to verify origin server cert was my first attempt to solve the problem. There were two problem drove me to change the design. First, all existing SSL hooks at the time were on the server side (browser -> ATS). There was no example on the client side (ATS -> origin server). I have all the freedom to choose the design. Second, the existing SSL hooks (including TS_EVENT_SSL_SERVER_VERIFY_HOOK) required records.config to enable proxy.config.ssl.client.verify.server. This option does not work in LinkedIn, because the SNI verify will always fail (our origin server has different name than the SNI in the request). Worse, the option even controlled if the CA cert is set. We independently implemented #3834 ourselves to decouple the two.

One of the requirements we need is that the verification is per transaction instead of global. We need to verify certain fields in SAN against parameters in corresponding remap rule (@@param). That is why my implementation also have a user-provided data argument (passed via continuation). I'm not sure if TS_EVENT_SSL_SERVER_VERIFY_HOOK can be used in such way.

@shinrich
Copy link
Copy Markdown
Member

I'll get you more details on how to exercise the verify hook when I am back in the office next week. I know that we have at least one group using this feature.

So you want to replace the standard openssl certificate verify instead of augmenting it?

Can you clarify what you mean by "the verification is per transaction instead of global"? Do you mean you want to verify the certificate for every transaction that appears on a TLS connection rather than just once for the start fo the TLS connection?

@CrendKing
Copy link
Copy Markdown
Author

CrendKing commented Jul 27, 2018

Yes, we want to augment it. X509 v3 has large number of extensions, and the standard procedure can't verify all of them in the way every ATS user needs.

Allow me to clarify. Our use case is this (over simplified): we have tool that generates X509 certificate for both services that deploy in staging and production, as well as for developers who need to test their services TLS functionality. Both types of the certificates are signed by the same internal CA, thus the standard OpenSSL verification method will not distinguish. However, one of the X509 v3 extensions differs between the two.

We also have a DNS system that given a service-like domain name, it resolves to an IP that hosts that service. This may sometimes includes a developer's laptop IP.

The goal is that for certain critical services where requests may carry sensitive data, we do not want to connect to developer IPs. With this PR, we create a plugin that specifically compares the X509 v3 extension against a parameter from remap rule. The verification happens if 1) the transaction falls into the remap rule; and 2) it is a new IP that ATS has no established connection to. Of course, once handshake is completed, subsequent transactions to the same IP will not trigger the verify method, until the connection is closed or timed out, saving performance.

With TS_EVENT_SSL_SERVER_VERIFY_HOOK, assuming it works, and the hook is called after TSRemapDoRemap, the approach could be

  1. Write a remap plugin that put the parameter to VConn if TSVConnIsSsl() == true by calling TSVConnArgSet().
  2. Write the hook that inspect the VConn arg. If exists, we want to specially verify this connection.

The performance would be slightly lower, but hook is the more idiomatic way in ATS.

@shinrich
Copy link
Copy Markdown
Member

shinrich commented Aug 1, 2018

Sorry for the delay in getting back to this. I finally had time to dig back into this. @persiaAziz originally did this work, but she has moved onto other things.

The way the TS_SSL_SERVER_VERIFY_HOOK currently works, the registered callback will not be executed if the built in openssl checks did not pass. But we did have feedback from some internal customers desiring to replace the openssl check much like what you want to do. Looking at some old proof of concept internal code, it looks like Persia was proposing two new plugin APIs.

TSSetDefaultVerification(vc, 0); // default verification must be set to 0 if the default verification is not desired
TSCertVerifiedSet(vc, 1); // must be set to 1 if verification passes on the plugin side 

I think the intent with the first code is to tell Traffic Server to ignore the openssl decision. And the second call lets the plugin make its own decision. Barring fights on the naming, does that kind of API make sense to you and would it work for your scenario?

Presumably our internal customers lost interest and wandered off since this work is from at least a year ago and didn't get pushed to completion. Only the plugin verify that gets called if the base openssl verify completes is in the current open source master.

I resurrected Persia's example server verify plugin. I must head off now, but I'll get that ready to put in example to at least show how the exercise the current hook API.

@CrendKing
Copy link
Copy Markdown
Author

Thank you Susan. I could not find either TSSetDefaultVerification or TSCertVerifiedSet in the current code base. Could you please provide the example to use the hook, so that we can evaluate if it works for our use case?

@zwoop zwoop added this to the 9.0.0 milestone Aug 9, 2018
@CrendKing
Copy link
Copy Markdown
Author

Hello. Still waiting for an update on this request @shinrich. Thank you!

@shinrich
Copy link
Copy Markdown
Member

shinrich commented Jan 9, 2019

Conversation continuing via issue #4569

@zwoop zwoop modified the milestones: 9.0.0, 10.0.0 Aug 21, 2019
@bryancall
Copy link
Copy Markdown
Contributor

This PR hasn't been updated for over 2 months. If you would like to work on this please reopen. Make sure it passes CI and doesn't have any merge conflicts.

@bryancall bryancall closed this Oct 17, 2019
@bryancall bryancall removed this from the 10.0.0 milestone Oct 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants