Skip to content

Project status request (2020-10-13) #27

Description

@joehorsnell

Hi,

Can you please advise if this project is still active and being maintained? Ping @AnkurGel @raghuhit @francisf (looking at "recent" committers to this repo).

I was reviewing and clearing up my PRs and had forgotten about #19, which I opened 2.5 years ago. I personally no longer need it, but believe it should be merged to prevent MITM attacks for any users of this code. However, if this repo is abandoned I will close the PR so I don't see it in my list.

Thanks,

Joe.

Activity

  1. abhi291096 commented on Oct 15, 2020

    @abhi291096

    Hey @joehorsnell, allow me to share the details:

    This repo is indeed active. We have reviewed your PR and understand your core concern about a MITM attack. However, after internal discussions, the team wants to solve that problem with a more intricate solution since there are a few cases where simply turning on the verification blocks our execution. We are working towards creating a solution which doesn’t alienate such cases, while also not making any security compromises. Therefore while we have decided to not merge your PR, we want to thank you for bringing this to our attention and request you to watch this space for when we make our updates (expected timeline: November 2020).

    Regards,
    Abhishek

  2. joehorsnell commented on Oct 19, 2020

    @joehorsnell
    Author

    Hey @joehorsnell, allow me to share the details:

    This repo is indeed active. We have reviewed your PR and understand your core concern about a MITM attack. However, after internal discussions, the team wants to solve that problem with a more intricate solution since there are a few cases where simply turning on the verification blocks our execution. We are working towards creating a solution which doesn’t alienate such cases, while also not making any security compromises. Therefore while we have decided to not merge your PR, we want to thank you for bringing this to our attention and request you to watch this space for when we make our updates (expected timeline: November 2020).

    Regards,
    Abhishek

    Hi @abhi291096 - thanks for getting back to me.

    This is probably more a comment for #19, but I'm responding here since this is where your comment is. As I mentioned, I no longer actually need this PR, I was just reviewing my open ones.

    I would strongly suggest that enabling certificate validation must be the default. BrowserStack are a well known company and should set an example for other engineers who may see this code and potentially learn extremely bad habits from, like disabling certificate verification. This approach should only ever be used in very rare circumstances and with a full understanding of the risks. Not as the default, I suggest.

    How about instead I modify the PR to allow disabling certificate verification through an environment variable, but not as the default? That way you can still opt-in and disable it for your use case, but the default is to verify certificates?

  3. abhishekchoudhary commented on Oct 21, 2020

    @abhishekchoudhary

    Hi Joe, I'm the Product Manager for BrowserStack Automate. We would absolutely love to collaborate with you on the PR. Broadly, we'd like to do exactly what you've mentioned - have validation on by default, but have an option to turn it off if we hit an edge case where validation causes execution failure. Please go ahead and implement what you have in mind, and I can have an engineer from my team work with you to review and push it through. Thanks for taking the time!

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions