--disable-ssl-check option

[ivanprado]

Some users are experiencing problems with the certificates on Windows. While I think the problem is that Windows certs are not up to date, anyway this is causing troubles to the users. As a scape hatch, this PR introduces the option `--disable-ssl-check`` to skip the problem by disabling SSL verification.

An alternative would have been to install our own certificates using certifi. But aiohttp guys decided not to do so and neither should we: we should rely on OS certs instead of trying to pack the certificates ourselves.

[codecov-commenter]

Codecov Report

Merging #37 (147eac8) into master (0447846) will decrease coverage by 0.39%.
The diff coverage is 41.66%.

❗ Current head 147eac8 differs from pull request most recent head ed0f9e3. Consider uploading reports for the commit ed0f9e3 to get more accurate results

@@            Coverage Diff             @@
##           master      #37      +/-   ##
==========================================
- Coverage   66.66%   66.26%   -0.40%     
==========================================
  Files          14       14              
  Lines         501      504       +3     
  Branches       65       65              
==========================================
  Hits          334      334              
- Misses        163      166       +3     
  Partials        4        4              

Impacted Files	Coverage Δ
autoextract/__main__.py	22.66% <0.00%> (-0.31%)	⬇️
autoextract/aio/client.py	47.14% <50.00%> (ø)
autoextract/aio/retry.py	51.61% <50.00%> (-1.73%)	⬇️

[lopuhin]

Thanks @ivanprado , is the typing issue caused by changes in this PR, or happens on master as well?

[lopuhin]

Thanks for the PR @ivanprado , c3cf87c looks good to me, I didn't check the other commit yet - asked about it above.

To me the main question is if we should allow it or not (as disabling certificate checks could be dangerous), but looking around I see that almost all tools do allow it as it's often not in the power of the user to fix certificate errors, so +1 to allow it for us.

[kmike]

The feature looks good to me @ivanprado; the only thing is naming, as usual :)

I know the parameter is named ssl in aiohttp, but SSL is deprecated a long time ago, and not used much by websites now (<4%); it's TLS all the way. Would it be possible to have another name for this option - tls, or something about certificates, or just something about being insecure?

[ivanprado]

TIL TSL vs SSL. What do you think about 015e5b1?

[ivanprado]

is the typing issue caused by changes in this PR, or happens on master as well?

@lopuhin It will likely happen in master as well. Tenacity introduced new type annotations (https://github.com/jd/tenacity/pull/304/files) and now mypy is able to check if our code matches.

as disabling certificate checks could be dangerous

I've been thinking a bit about it. The major issue I can see is somebody trying to do a man in the middle attack. But even with the checks on, this attack is still possible. You just need to create your own certificates (using letsencrypt for example). They will be valid.

Note that here we are only disabling certificates validation. The communication will still be secure over TLS in any case.

[kmike]

Looks good, thanks @ivanprado!