Skip to content
This repository has been archived by the owner on Apr 14, 2022. It is now read-only.

Disabling cert verification: can we improve security without punishing users? #142

Open
njsmith opened this issue Nov 18, 2019 · 7 comments

Comments

@njsmith
Copy link
Member

njsmith commented Nov 18, 2019

@sethmlarson thinks that the classic verify=False makes it a bit too easy to disable cert checking, so it encourages folks to do it without fully understanding the consequences. Of course we don't want to make it punitively difficult either. Often the folks who have to disable cert checking hate that as much as we do, and are just stuck in an impossible situation. But some ideas for things we could potentially do:

  • Make the kwarg more verbose, so it's more obvious what it does and that enabling it is insecure, e.g. disable_secure_certificate_validation=True
  • Issue a warning whenever it's used... folks who want to disable the warning could still do so, but it makes the issue visible withotu actually stopping anyone from getting their work done
  • Make it very easy to trust a specific certificate, to make it easy to do TOFU-style trust for self-signed certificates. Ideally this should be even easier than disabling cert validation entirely.
@sethmlarson
Copy link
Contributor

I'm really liking the fingerprinting idea, allows for users to trust an otherwise completely incorrect certificate and it's easy to transform an error message (could contain the "mismatched" fingerprint) into a mostly secure setup (use the fingerprint from the error message in my TLS config).

As with everything related to TLS configuration, documentation will go a long way towards users making good decisions with security. Example error messages embedded within the documentation should be the the top of the Google search results. :)

@pquentin
Copy link
Member

I guess trust on first use is only really possible if we implement #141, which is another argument in favor of it! And then a simple kwarg could be enough, and we just have to make it shorter than disable_secure_certificate_validation :)

@njsmith
Copy link
Member Author

njsmith commented Nov 19, 2019

I guess trust on first use is only really possible if we implement #141, which is another argument in favor of it!

Oh hmm, I guess there's actually two different things we could call "trust on first use", so let's be more explicit.

I think the flow @sethmlarson had in mind was something like:

  • there's a kwarg pinned_certs={"hostname": "hash}
  • when writing a script, someone tries to connect without this, and gets a cert validation error. The error message includes the cert hash and a tip about how to use pinned_certs=
  • the user manually copies that hash out of the error message, and pastes it into their code as a string literal.

And it sounds like @pquentin is thinking of something like:

  • there's a kwarg tofu=True
  • when this is set, and we encounter an untrusted cert, we check the session store. If there's already a cert there for this host, we compare them, and error out if they don't match. If there isn't, we silently accept the cert, and store it in the session store for next time.

Both approaches seem better than verify=False. But I think the first one, with the hard-coded strings, might be better overall?

The major problem with using the session store here is that in practice, this is a really ephemeral thing, so users wouldn't actually get much protection – usually when we go to look up the cert, it won't be there. E.g. for a script that runs once and then exits, tofu=True is essentially equivalent to verify=False.

OTOH, trying to make the session store magically persist across invocations is also fraught with challenges. First because as a library, we kind of just... can't really do that without some explicit manual configuration. And second because in this case, I think it could be really confusing to users if https certs are "randomly" accepted or rejected based on some long-lived implicit state. The hard-coded string version has the advantage that it makes it really obvious to the user that they're pinning a cert and what the lifetime of that pin is.

@sethmlarson
Copy link
Contributor

@njsmith Yep, you understood what I was imagining to be the interface. Anything that's in the session store is liable to expire / be deleted / not be feasible to store, and we don't want that to happen for most TOFU use-cases, especially as an alternative to disabling verification. :)

@pquentin
Copy link
Member

Thanks for the great explanation, that makes a lot more sense!

@njsmith
Copy link
Member Author

njsmith commented Nov 20, 2019

So here's an interesting question about an interface like pinned_certs={...}: what we should do if we see a cert that does validate normally, but doesn't match the one in pinned_certs? For "ugh I just want this self-signed site to work" case, if the site later switches to a real cert, that's fine and you want to quietly accept it – basically you want to treat pinned_certs as adding some more (very limited) trust roots. But you can also imagine users who want to explicitly pin a cert because they don't want to trust arbitrary CAs, where it's supposed to effectively limit the trust.

I guess those kinds of users might also want to do more sophisticated things, like pin a specific root cert without pinning a specific cert. Though you can also accomplish some of this via more general TLS trust config, like by setting a trust store that only includes the root you want.

Maybe what I'm saying is just that when it comes time to choose the details for how pinned_certs=... should work, we should look at that in the context of TLS trust configuration more generally.

@sethmlarson
Copy link
Contributor

sethmlarson commented Nov 20, 2019

pinned_certs should be additive by default because your trust store can be configured to be empty the same way you can configure pinned_certs to be empty too.

The "ugh make this site work" user will probably leave the ca_certs parameter to its default meaning that if the site suddenly isn't self-signed anymore, nothing breaks and security isn't degraded. IMO for this use-case it'd actually be more of a security issue if TLS were to suddenly break upon the site changing to a valid cert because a user may be confused and switch the pin to the new valid cert instead of "unpinning" the old cert (!). If the site changes it's self-signed cert to another self-signed cert TLS will start failing, but this would also be the case for a MITM so we have to report an error here. This configuration is as safe as a TOFU config can be.

Maybe we can issue a warning if a site with a pinned cert suddenly starts working via traditional PKI with instructions on how to remove the pin?

I agree we'll have to do a more comprehensive view of the many trust config use-cases.

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

No branches or pull requests

3 participants