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

Send a PKI from the client to the server #2694

Closed
justin-bits opened this issue Oct 31, 2018 · 56 comments · Fixed by #15179
Closed

Send a PKI from the client to the server #2694

justin-bits opened this issue Oct 31, 2018 · 56 comments · Fixed by #15179
Assignees
Labels
type: feature New feature that does not currently exist

Comments

@justin-bits
Copy link

Need a way to send a PKI from the client to the server when it is requested by the server on login (two way SSL).

On connecting to our application, the server requests a PKI / personal certificate from the client. This PKI is unique to the user; it is installed in the browser so that they can authenticate to websites with it.

The browser prompts the user to “Select a certificate to authenticate yourself”, where the user can select OK to send it.

This prompt/dialog isn’t appearing in Test Runner. Instead, our custom error message indicating that the user did not send a PKI is displayed.

I attempted to use a group policy to force all instances of Chrome to auto select a certificate, but the PKI still isn’t sent from the client to the server. The group policy does work for all other instances of Chrome that aren’t created by Cypress.

@jennifer-shehane jennifer-shehane added stage: needs investigating Someone from Cypress needs to look at this priority: medium 🔸 labels Oct 31, 2018
@jsalankey
Copy link

This is a critical piece of functionality for our ability to use Cypress as our testing framework.

@sforner405
Copy link

We also need to add a specific certificate to communicate with the server.
This feature is necessary if we want to use cypress.

@jennifer-shehane jennifer-shehane added stage: needs review The PR code is done & tested, needs review and removed stage: needs investigating Someone from Cypress needs to look at this labels Nov 8, 2018
@brian-mann
Copy link
Member

brian-mann commented Nov 8, 2018

I've reviewed the PR here #2740 and understand the implications of this and have some feedback.

I see three areas that need to be addressed:

  1. Would attaching the client certs only on cy.visit() actually fix the issue? Wouldn't the browser need to send the certs on all requests, not just the first?

  2. We'd also need to ensure this works on cy.request() too since that is made outside of the browser.

  3. I need to review how browsers normally accept the PKI certificates as command line arguments or through group policy to understand the various ways in which they can be configured. These would at the least need to be configurable at the cypress.json level, and likely need some way to control which domain(s) they are sent to - enabling you to have multiple certificates for multiple domains. We may also need to abstract this away and enable it to be configured at the CLI level too, and automatically make it work for multiple browser agents. Alternatively, if its configurable via command line flags, then right now you could just modify the launch arguments (to chrome, etc) and add them in.

Thoughts:
Although we could do everything at the network layer transparently - I believe it likely makes more sense to utilize the browser agent itself to handle automatically attaching these instead of at the proxy layer. That way problems would bubble up to the dev tools in the browser and would be more immediately apparent.

Update:
On second thought... after some brief research it may actually be quite a PITA / difficult to get all of the browsers to play nice with this, and it may actually be easiest to attach this at the network layer outside of the browser. The configuration would likely be easier, and we may be able to also support it dynamically via cy.visit() and cy.request() as well as options.

@sforner405
Copy link

Even in a CI environment it is more difficult to add this to a browser instance.
Had a lot of issues there recently.

@matt-rhys-jones
Copy link

matt-rhys-jones commented Nov 8, 2018

Thanks for reviewing :) Just to provide a bit of background in regards to your first point, our reasoning for focusing on cy.visit() was because it was impossible for us to perform any form of e2e testing on the page under test without some way of utilising certificates.

By allowing certs to be passed through via cy.visit() we could at least load the initial page, then use fixtures for subsequent requests the page made. While this doesn't give us a true e2e picture, it would allow us to have fairly comprehensive front-end tests and significantly enhances our ability to adopt Cypress while further support for PKI was being considered/worked on. We saw it very much as a potential first step, but one that would have immediate benefit to us at the BBC.

In regards to your third point, my understanding of how PKI typically works is that a certificate is unique to a single user/machine, but can be used to authenticate across multiple domains that a corporation manages. With this in mind, certainly for us, the ability to configure just a single certificate in cypress.json, or by another means, would be enough for us to make full use of what Cypress has to offer.

We'd more than happily work with whatever implementation you felt was best, but hopefully this sheds some more light on what we're trying to achieve.

@craigkj
Copy link

craigkj commented Nov 12, 2018

@brian-mann Hi there. I worked with @matt-rhys-jones on this and we would like to keep this PR moving. Did you come to a conclusion on what you thought the best approach was? Thanks for the feedback btw. Cheers.

@brian-mann
Copy link
Member

This PR would need to be updated to reflect / address the issue in a more comprehensive way. I would prefer coming up with a solution that:

  1. works across all browsers
  2. works for cy.visit, cy.request, and all other network traffic the browser makes

Updating just cy.visit() would not really address this issue - and my preference would be either adding command line arguments or cypress.json configuration to address this.

@matt-rhys-jones and @craigkj what would be most helpful is if you can find us examples of using other testing tools to accommodate this need - selenium/webdriver, puppeteer, etc.

When I looked I did not see a way to launch chrome with arguments that would add these certificates so I cannot find a comparable way in which this could be achieved. If there are existing solutions we can piggy-back off of their ideas.

To be clear, without an easy way to configure each browser means that we'd be looking at implementing this at the network layer behind the scenes in a way that the browser doesn't know that it's happening. That'll work across all browsers but may have unintended effects, since I'm not super experienced with PKI in general, although I understand how it works.

Additionally it seems that whatever way we come up with would have to be targeted on a domain basis, and possibly also expose specific sets of configuration.

@matt-rhys-jones
Copy link

Hi @brian-mann - thanks for your thoughts. I agree that updating cy.visit() only partially addresses the issue (and was only intended to do so initially).

In regards to existing solutions, I haven’t come across many in the past, and certainly not any elegant ones. Most seem to involve automating dialogue box clicks with device automation tools. Selenium does allow the web driver to be configured to custom Firefox profiles with the various certificates provided but as you say this isn't possible across all browsers.

These solutions are inherently flakey, you'd know more than me but personally I don’t think it would be easy to come up with a reliable cross browser solution that automated the various dialogue boxes (or configs for headless browsers) to support PKI. This would also require continued maintenance as new versions of browsers emerge, and documentation updates too.

I think that for this to work reliably it would need to be done at the network level.

My understanding is that SSL operates at the Session/Presentation layer of the OSI model rather than Application layer. While the browser can be configured to support PKI in it's entirety, I believe the Operating System would typically be responsible for housing and trusting Root Certificates / Certificate Authorities, and the browser would verify against these. Either way, how the browser ultimately handles the HTTPS response should remain unchanged as that’s at the Application layer.

This is probably an over-simplistic viewpoint and I"m sure the separation between OSI layers in the browser isn't as clear-cut as that, but that's my understanding of how it should work.

Therefore I think implementing this transparently at the network layer of Cypress makes more sense as the browser shouldn't care whether the original request was being made to a PKI domain or not providing it gets a valid HTTPS response from it's original request.

If we were worried about side effects it might be sensible to implement PKI support behind an opt-in feature flag or switch (e.g. cypress run —enablePKI). If the flag is enabled then we could check the configuration for certificate/domain combinations and pass the certificates on with the request to the relevant domains transparently.


I’d imagine the most challenging part would be maintaining the standard of Cypress’s error handling and messages. Errors returned from openssl are not particularly friendly and I’d imagine we’d need to wrap around these so that we can provide clear guidance to users on which part of the PKI authentication went wrong (e.g. badly formatted certs, missing CA, missing certificate/key, unable to validate certificate against CA). I’m not sure how easy that would be, and it would require more understanding of the TLS spec and openssl internals/errors which again would take time.

If you think the above approach is viable, and Cypress would be happy to provide support with PR reviews and some guidance (if the docs fail me) then I would be happy to have a go at implementing this in my spare time.

Alternatively, if you wanted to add PKI support to your roadmap and develop in house then I would be happy to test it out.

Either way, it's definitely a feature we feel would be valuable for us at the BBC, add value to Cypress as a product, and address the original issue that was raised.

Please do let me know how you'd like to proceed! :)

@sforner405
Copy link

Seems that our problem is also not fixed by the PR.
We need to add the certificate to the XHR requests made by the browser.
So the requirement
"works for cy.visit, cy.request, and all other network traffic the browser makes"
is important for us.
Best thing would be to have something like:
{ "certificates": { "baseurl": { "key": "keypath", "cert": "certpath" } } }

@matt-rhys-jones
Copy link

Yes, it should definitely be implemented for all requests. However you can at least use fixtures for your subsequent XHR requests as a middle ground until we have a more rounded solution.

@sforner405
Copy link

I can see it was added to Sprint 11, but never got fixed or added to the subsequent sprint milestones.
Is this still something you are looking into?

@jennifer-shehane jennifer-shehane added difficulty: 4️⃣ stage: proposal 💡 No work has been done of this issue and removed stage: needs review The PR code is done & tested, needs review labels Dec 11, 2018
@jennifer-shehane jennifer-shehane removed this from the Sprint 11 milestone Dec 11, 2018
@jennifer-shehane
Copy link
Member

The scope of this issue has been expanded - so work on this has been postponed until the implementation details are decided first. See #2694 (comment)

@jeff-mccoy
Copy link

jeff-mccoy commented Feb 16, 2019

Realize this is closed, just wanted to drop for future reference how we did it. I'm a US government software engineer and ran into this problem today, decided to just leave Cypress as-is and implement a reverse proxy instead. Basically expose different proxy ports with different scenarios: Valid, invalid pass, revoked, tampered, etc. That way Cypress (or anything else) can just get the scenarios it needs via a port.

We generate the SSL certs dynamically and inject them into the test environment. Also note, this app uses web sockets so I had to add a handler for that, it would be even simpler without.

const httpProxy = require('http-proxy');
const fs = require('fs');
const http = require('http');
const morgan = require('morgan')('dev');

proxyBuilder('Valid cert', 'client-valid.p12', 'test', 25000);
proxyBuilder('Invalid cert password', 'client-valid.p12', 'bad-password', 25001);
proxyBuilder('Untrusted cert', 'client-untrusted.p12', 'test', 25002);
proxyBuilder('Revoked cert', 'client-revoked.p12', 'test', 25003);
proxyBuilder('Expired cert', 'client-expired.p12', 'test', 25004);
proxyBuilder('Tampered cert', 'client-tampered.p12', 'test', 25005);

function proxyBuilder(testTitle, certPath, certPass, targetPort) {

  try {

    const proxy = new httpProxy.createProxyServer({
      target: {
        protocol: 'https:',
        host: 'localhost',
        port: process.env.SERVER_PORT,
        pfx: fs.readFileSync(`${__dirname}/../ssl/${certPath}`),
        passphrase: certPass,
      },
      secure: false,
      changeOrigin: true,
    });

    const proxyServer = http.createServer((req, res) => {
      console.log(testTitle);
      morgan(req, res, () => null);
      proxy.web(req, res);
    });

    proxyServer.on('upgrade', (req, socket, head) => {
      console.log(testTitle);
      morgan(req, socket, () => null);
      proxy.ws(req, socket, head);
    });

    proxyServer.listen(targetPort);

  } catch (e) {
    console.error(`Problem starting ${testTitle}`);
    console.error(e);
  }

}

@jennifer-shehane
Copy link
Member

@clevrtec This issue is not closed. It is part of our roadmap, but has not had any work done on it at this time.

@c32hedge
Copy link

c32hedge commented Mar 15, 2019

In my use case, I would like to use cy.request to make API calls that require certificate-based authentication. Being able to use certificates at all would be a minimum need for me to be able to test our UI when we make changes via the API. However, I'd like to be able to use Cypress to test the API itself, and to do that I would want to test using different certificates, so just setting one in a config file wouldn't be ideal.

@Brett-Holmes
Copy link

We also need this feature to be able to use Cypress at all. Any Updates? Thank you!

@kmaverick
Copy link

kmaverick commented Jan 2, 2021 via email

@jennifer-shehane jennifer-shehane added the type: feature New feature that does not currently exist label Jan 4, 2021
@CMalanaphyStatpro
Copy link

Where I work we now have a patched version of 6.0.1 that, together with a pair of custom commands and some appropriate certificate config entries in cypress.env.json / cypress.json gives us cy.visitWithCerts and cy.requestWithCerts, and removes the need for reverse proxies and so on.... I'm thinking it'd be good to eliminate the custom commands bit and add the 'WithCerts' commands to the cypress code and push up to github as a PR in the latest version of Cypress - but before I did that, is this something that would get support from the Cypress authorities? Would 2 new Cypress commands like this be dismissed as unwanted?

Hi @GCHQDeveloper911, Did you get any feedback on your changes? if not could you send an example of the custom commands and certificate config entries in cypress.env.json

@GCHQDeveloper911
Copy link
Contributor

Hi @CMalanaphyStatpro - Didn't really get any feedback, but I'm working on it anyway - give me a bit to sort a couple of issues!

@jennifer-shehane
Copy link
Member

@GCHQDeveloper911 We would be open to a PR for this. It may help accelerate the work. We would still likely need to do some additional work on top of it. We're not sure exactly how the API would look, whether it would be those commands exactly. We would also want to have full cross browser support before an official release. Feel free to open a PR in 'draft' and let us know if you need any review.

A way to get this out quickly, without the Cypress team's complete buy in, would be to create a plugin that people can use.

@GCHQDeveloper911
Copy link
Contributor

Hi @jennifer-shehane Some colleagues of mine have picked this up, and I think are close to a "pr-worthy" solution. However, I'll ping them a link to this thread, to highlight the plugin suggestion. Thanks!

@jonminter
Copy link

@GCHQDeveloper911 Wondering about the progress your colleagues are making with this. Have a need for cert based authentication with Cypress for the project I am working on. Currently using a reverse proxy but would be interested in a different approach that doesn't require that.

I'd be willing to help with the implementation. The plugin idea sounded promising.

@GCHQDeveloper911
Copy link
Contributor

Hi @jonminter - I just spoke to the guys, they're just doing some work to re-baseline against 6.5.0... Hopefully get a PR in soon...

Cheers

@cypress-bot cypress-bot bot added stage: work in progress and removed stage: proposal 💡 No work has been done of this issue labels Feb 22, 2021
@thirstycoda
Copy link

Hi @GCHQDeveloper911,

We were hoping to use Cypress with certificates to impersonate different users at the same URL to test differing functionality/permissions. It looks like your PR introduces the ability to use the same certificate for all tests that relate to a particular URL. Am I right in thinking Cypress still won't support our requirement after your PR or is there a way to achieve what we need?

Thanks

@jonminter
Copy link

@GCHQDeveloper911 @thirstycoda

I have the same requirements and question after reading over the PR. I think this could be accomplished by adding a method to the Cypress API to set or override the configured client certificate for the request. Something like:

cy.clientPkiCerificate({
  "pfx": "/home/tester/certs/cert.pfx",
  "passphrase": "/home/tester/certs/pfx-passphrase.txt"
});

@GCHQDeveloper911
Copy link
Contributor

Hi @jonminter & @thirstycoda ....

We completely agree with you this would be a very nice further feature. However, for now our focus was a design that did not alter the 'test-facing' APIs so that it could be added to Cypress without breaking Cypress users' existing tests - key to getting the Cypress authorities to accepting the change.

The guys I've been working with that have done the hard work(!) on this have more changes coming / planned, including cert validation on start up. I'm sure they'll also look at your 'cert switching' suggestions too. But for now, the view is "keep it simple" and "get the basics approved".

Hope this helps :)

@flotwig
Copy link
Contributor

flotwig commented Mar 3, 2021

+1 @GCHQDeveloper911, per-test PKI config can be added at a later date. See #2694 (comment) for some additional discussion on API design.

@steverb1
Copy link

@jennifer-shehane Has this been addressed yet? If not, do you have any timeframe for it?
This is something we badly need also, before our company can adopt Cypress for use.

@markslater
Copy link

I'm really happy to see progress on this feature. In common with other commenters, I am working at an organisation that uses client certificates extensively, so to date, our Cypress tests haven't been truly representative of real world usage.

I want to second @thirstycoda's comment: being able to configure Cypress with a single client certificate is a big step forward, but really unlocking all client certificate based testing will require the capability to set certificates per call. In addition to the example @thirstycoda gave, it would be good to test the rainy day cases, like users presenting an expired certificate, or an invalid certificate, or no certificate, etc.

It would be great if this could be given consideration soon after #15179 (I also wonder if #15179 does actually make switching between client certificates possible, by changing the file referred to in the config on the fly).

@steverb1
Copy link

While I agree more capability around certificates will be helpful, let's get this first increment to the finish line (released) before we worry about more. I wouldn't want to see this delayed, by starting to look at more.

@pcj
Copy link

pcj commented May 18, 2021

This is kind of a big deal.

Trying to upgrade our tests from cypress 4.2 to 7.3.0 and hitting this. AFAIK, Cypress does not surface TLS client certificate errors. I got so frustrated by this (I didn't know what the underlying issue was... why SSO authentication no longer worked) I started a sample project using playwright. Sure enough, the first chromium test I ran popped up a dialog asking me what client certificate I'd like to use.

Here's the workaround for playwright: microsoft/playwright#1799

@upsampled
Copy link

@jeff-mccoy thanks for sharing that work around. Do you mind clarifying how you are using it:

  • Are you running that code as a service? or part of some cypress mechanic?
  • Is DNS resolved at the proxy or the cypress client?
  • Does the host: 'localhost' line mean that all requests to the proxy are redirected to that domain name? Does changing it to my-dev.corpA.com mean that I can tell cypress to send requests to 127.0.0.1:25000 and they will then be redirected to my-dev.corpA.com:443?

I see @GCHQDeveloper911 is the hero hammering out the PR with this. Thank you for doing that.

Current Use-case

Just to generally add to the requirements/expectations, here is my setup:

  • mostly isolated test network with its own root DNS server (so we can actually use Internet DNS names)
  • only way to get data to/from this network is a single SOCKv5h proxy. The 'h' just means I force DNS resolution to occur at the proxy so that domain name are resolved on a real nameserver. git, go, curl, wget all handle it without issue.

The testbed allows for a real mtls connection. While this could be done with just having a local nameserver override a record, that is more complex and does not have the overall assurance of a single proxy enforcing internet access.

Right now the main issues with cypress integration are:

  • mtls (covered in this ticket)
  • Use of a proxy prevents the logging of test results, also SOCKSv5 is not supported but I have no problem adding other proxies

I believe that the @jeff-mccoy 's solution can actually handle this, as long as I run cypress on the SOCKv5 machine (real internet setup) and the BASEURL given to cypress is just an IP:Port (hopefully the https proxy will use the isolated DNS server, else I will just have to play games with /etc/hosts).

@cypress-bot cypress-bot bot added stage: needs review The PR code is done & tested, needs review and removed stage: work in progress labels Jun 11, 2021
flotwig added a commit that referenced this issue Jul 12, 2021
Co-authored-by: GCHQDeveloper911 <GCHQDeveloper911@users.noreply.github.com>
Co-authored-by: Jennifer Shehane <jennifer@cypress.io>
Co-authored-by: Zach Bloomquist <github@chary.us>
Co-authored-by: Zach Bloomquist <git@chary.us>
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jul 12, 2021

The code for this is done in cypress-io/cypress#15179, but has yet to be released.
We'll update this issue and reference the changelog when it's released.

@cypress-bot cypress-bot bot removed the stage: needs review The PR code is done & tested, needs review label Jul 12, 2021
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jul 20, 2021

Released in 8.0.0.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v8.0.0, please open a new issue.

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Jul 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: feature New feature that does not currently exist
Projects
None yet
Development

Successfully merging a pull request may close this issue.