Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

custom root ca support. #24

Closed
wants to merge 2 commits into from
Closed

custom root ca support. #24

wants to merge 2 commits into from

Conversation

siennathesane
Copy link

Pivotal has a lot of enterprise customers who have proxies which perform SSL re-encryption, which is like an authorized man-in-the-middle (MITM). Most companies do this to ensure their employees aren't downloading viruses, browsing illegal content, etc., so it's a common use case.

This was an easily solvable problem when the CLI had support for --skip-ssl-validation, but then created a new security risk. This PR adds transport support for a new flag in pivotal-cf/pivnet-cli, --root-ca, which allows for a customer to pass in their company's root CA, so the SSL connection is validated.

Signed-off-by: Mike Lloyd mike@reboot3times.org

Signed-off-by: Mike Lloyd <mike@reboot3times.org>
@xtreme-debbie-chen
Copy link
Contributor

This is a great idea! But unfortunately, it's missing tests. Can you add tests for this feature?

@siennathesane
Copy link
Author

@xtreme-debbie-chen can you point me to the core client tests? I wasn't able to find them.

@pnikonowicz
Copy link
Member

Is this what you're looking for? https://github.com/pivotal-cf/go-pivnet/blob/master/pivnet_test.go

Signed-off-by: Mike Lloyd <mike@reboot3times.org>
@pnikonowicz
Copy link
Member

pnikonowicz commented Feb 6, 2019

Thanks for doing the work on this. It looks like it has lot of potential.

We were not able to get the tests to pass on our Darwin OS.
Can you offer some assistance?
Maybe the operating system you were running these on is a factor?

2019/02/06 12:21:55 http: TLS handshake error from 127.0.0.1:62979: remote error: tls: bad certificate

@siennathesane
Copy link
Author

@pnikonowicz thanks for taking a look at it, I'm not quite done with the testing yet.

Based on my understanding around SSL reencryption, if you reencrypt the traffic at the proxy and tell the client that about the proxy CA cert, it shouldn't throw an error about bad certificates. I'm really not sure why it's throwing that error and I've been doing a bit of research into it.

As far as why I'm not using Ginkgo/Gomega, I can't get easily extend the test framework to do MITM, so I have to stand up the tests manually since there is a lot of setup for a local web server. I totally understand it's not ideal, but I'm not sure how else to go about it.

@pnikonowicz
Copy link
Member

No problem. I'm going to close the PR request for now and you can reopen when you are ready for us taking a look at it again.

Don't worry about ginkgo, we can and are more than willing to preform that refactor when needed.

I looked into the problem but I time boxed the effort and couldn't figure it out in time. Sorry I can't offer any more assistance, but the general idea looks good from a top level perspective.

@pnikonowicz
Copy link
Member

I'm going to reopen as I might be able to work on adding tests for this.
We are also considering adding it as a feature but will need to investigate how popular this feature would be.

If in the meantime, we were able to add tests, then this PR could be merged.

@siennathesane
Copy link
Author

@pnikonowicz I haven't made any progress on this since it was closed since I'm not entirely sure why the tests are failing for the reasons I mentioned above. I'm more than happy to work on it if you can help me figure out what is being done incorrectly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants