Closed SnijderC closed 7 years ago
Merging #5 into master will increase coverage by
0.13%
. The diff coverage is91.17%
.
@@ Coverage Diff @@
## master #5 +/- ##
==========================================
+ Coverage 82.31% 82.44% +0.13%
==========================================
Files 11 12 +1
Lines 1425 1430 +5
==========================================
+ Hits 1173 1179 +6
+ Misses 252 251 -1
Impacted Files | Coverage Δ | |
---|---|---|
certvalidator/ocsp_client.py | 77.77% <100%> (-4.05%) |
:x: |
certvalidator/_urllib.py | 100% <100%> (ø) |
|
certvalidator/crl_client.py | 59.25% <62.5%> (-1.76%) |
:x: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 486c2f8...154af62. Read the comment docs.
To see the issue I'm trying to fix, you can reproduce a request to the Let's Encrypt OCSP server with openssl
:
# Get the certificate chain and extract the issuer and server certificate from the chain
echo "" | \
openssl s_client -connect chrissnijder.nl:443 \
-servername chrissnijder.nl \
-showcerts 2>&1 | \
sed -ne '/-----BEGIN/,/-----END/ p;w chain.pem' | \
sed -ne '1,/-----END/ w server.pem' -e '2,$ !d' -e '/-----BEGIN/,$ w issuer.pem'
# Extract the oscp URL from the server certificate
OCSP_URL=$(openssl x509 -noout -ocsp_uri -in server.pem)
# Send an OCSP request
openssl ocsp -no_nonce -VAfile issuer.pem -issuer issuer.pem -cert server.pem -url $OCSP_URL
The above would result in:
Error querying OCSP responder
140736391005192:error:27076072:OCSP routines:PARSE_HTTP_LINE1:server response error:ocsp_ht.c:314:Code=400,Reason=Bad Request
whereas:
# Extract the hostname from the URL (remove http:// and trailing /)
OCSP_HOST=$(echo $OCSP_URL | sed -e 's/http://' -e 's/\///g')
openssl ocsp -issuer issuer.pem -cert server.pem -url $OCSP_URL -header host $OCSP_HOST
results in:
Response verify OK
server.pem: good
This Update: Feb 26 17:00:00 2017 GMT
Next Update: Mar 5 17:00:00 2017 GMT
Also at: https://www.openssl.org/docs/man1.0.2/apps/ocsp.html on -header
-header name value If sending a request to an OCSP server, then the specified header name and value are added to the HTTP request. Note that the name and value must be specified as two separate parameters, not as a single quoted string, and that the header name does not have the trailing colon. Some OCSP responders require a Host header; use this flag to provide it. [emphasis mine]
Thanks, this is a great catch! Would you mind also adding the host header to the CRL client?
Sure, but...:
_add_header()
and _get_host()
be placed? Now that 2 modules need the same functions.crl_client
not already using _add_header()
?Duplicating code is not nice, so maybe a new file _utils.py
?
Perhaps _urllib.py
, containing all of the Python 2/3 abstraction and then using it in the crl and ocsp clients?
Ha, sorry for the typo in the commit message that made crl_client()
look like a method in instead of a module. :)
Created a wrapper that makes Request functionally equivalent for these modules, I'm not sure this is removing obscurity since we add it right back to another module, but there it is.
Did you get time to check this out? Would be really nice if you could merge this, then I can remove some workarounds in our project.
Sorry, been kind of behind on open source stuff due to other commitments. Hopefully this week.
Thanks for pulling, I don't mean to push you, but any idea when a new version will be released? If it's going to take some time, I need to implement some workaround to fetch the master branch in our project. Thanks!
I was hoping to get #4 merged in before doing a release.
If you are using pip, I think you can point it at a github repo.
Add Host header for OCSP requests, required for some OCSP servers, e.g. Let's Encrypt.