[issue28747] Expose SSL_CTX_set_cert_verify_callback

[David Peall]

Change by David Peall :


--
nosy: +David Peall

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28747] Expose SSL_CTX_set_cert_verify_callback

[Chih-Hsuan Yen]

Change by Chih-Hsuan Yen :


--
nosy:  -yan12125

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28747] Expose SSL_CTX_set_cert_verify_callback

[Marcelo Salhab Brogliato]

Marcelo Salhab Brogliato  added the comment:

Exposing SSL_CTX_set_cert_verify_callback is useful when we want to use a 
Public Key Authentication, like it is done in the SSH Protocol.

Do you know any other way to use Public Key Authentication besides using 
SSL_CTX_set_cert_verify_callback?

--
nosy: +Marcelo Salhab Brogliato

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28747] Expose SSL_CTX_set_cert_verify_callback

[Steve Dower]

Steve Dower  added the comment:

The change to make OpenSSL a separate DLL (on Windows, at least, which is all I 
really care about) means this function is available via ctypes. That's good 
enough for me, so I'll close this.

--
resolution:  -> out of date
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28747] Expose SSL_CTX_set_cert_verify_callback

[Chi Hsuan Yen]

Changes by Chi Hsuan Yen :


--
nosy: +Chi Hsuan Yen

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28747] Expose SSL_CTX_set_cert_verify_callback

[Ronald Oussoren]

Changes by Ronald Oussoren :


--
nosy: +ronaldoussoren

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28747] Expose SSL_CTX_set_cert_verify_callback

[Steve Dower]

Steve Dower added the comment:

Any comment from the SSL experts?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28747] Expose SSL_CTX_set_cert_verify_callback

[Ned Deily]

Ned Deily added the comment:

>From a release manager perspective, I'm OK in principle with adding it to 
>3.6.1 as long as the ssl experts are OK with it.

--
nosy: +alex, dstufft, janssen

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28747] Expose SSL_CTX_set_cert_verify_callback

[Steve Dower]

Steve Dower added the comment:

The current _3.patch builds on default without warning and the tests pass 
(_2.patch is the one Ned tried).

Any objections to committing this into 3.7?

What about 3.6.1? As an additive and easy to detect API, I think it's suitable, 
and I will certainly use it (right now my library's setup.py depends on having 
each libeay.lib from each original CPython build handy to get some of the 
functions out of it - these files are about 50MB each, so it's a little 
painful).

If it helps, I'm happy to add a warning to the docs that setting the callback 
may cause a loss of security if the callback does not properly validate the 
certificate (or some wording to that effect). Personally I think that's fairly 
well implied though, as there isn't any other obvious use for the callback.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28747] Expose SSL_CTX_set_cert_verify_callback

[Steve Dower]

Steve Dower added the comment:

For the sake of review, I fixed the patch and rebased it on default.

--
Added file: http://bugs.python.org/file45594/28747_3.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28747] Expose SSL_CTX_set_cert_verify_callback

[Steve Dower]

Steve Dower added the comment:

Whoops, that's what I get for renaming something. Easily fixed, but I'm happy 
to aim for 3.7.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28747] Expose SSL_CTX_set_cert_verify_callback

[Ned Deily]

Ned Deily added the comment:

We are two weeks from producing the release candidate for 3.6.0.  I don't think 
we should be rushing to add a new security-critical API which, IIUC, won't be 
used in the initial release anyway.  Let's target it for 3.7 after proper 
review and then we can decide whether it makes sense to backport to a 3.6.x 
maint release if the security issues it may solve warrant it.

--
versions:  -Python 3.6

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28747] Expose SSL_CTX_set_cert_verify_callback

[Ned Deily]

Ned Deily added the comment:

And, as it stands, the tests fail (at least on macOS):

==
ERROR: test_set_cert_verify_callback (test.test_ssl.SimpleBackgroundTests)
--
Traceback (most recent call last):
  File 
"/py/dev/36/root/fwd_macports/Library/Frameworks/pytest_10.12.framework/Versions/3.6/lib/python3.6/test/test_ssl.py",
 line 1782, in test_set_cert_verify_callback
ctx._set_cert_verify_callback(callback)
AttributeError: 'SSLContext' object has no attribute '_set_cert_verify_callback'

==
ERROR: test_set_cert_verify_callback_error (test.test_ssl.SimpleBackgroundTests)
--
Traceback (most recent call last):
  File 
"/py/dev/36/root/fwd_macports/Library/Frameworks/pytest_10.12.framework/Versions/3.6/lib/python3.6/test/test_ssl.py",
 line 1805, in test_set_cert_verify_callback_error
ctx._set_cert_verify_callback(raise_error)
AttributeError: 'SSLContext' object has no attribute '_set_cert_verify_callback'

==
ERROR: test_set_cert_verify_callback_suppress_error 
(test.test_ssl.SimpleBackgroundTests)
--
Traceback (most recent call last):
  File 
"/py/dev/36/root/fwd_macports/Library/Frameworks/pytest_10.12.framework/Versions/3.6/lib/python3.6/test/test_ssl.py",
 line 1827, in test_set_cert_verify_callback_suppress_error
ctx._set_cert_verify_callback(raise_error)
AttributeError: 'SSLContext' object has no attribute '_set_cert_verify_callback'

--
Ran 130 tests in 27.416s

FAILED (errors=3, skipped=8)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28747] Expose SSL_CTX_set_cert_verify_callback

[Ned Deily]

Ned Deily added the comment:

With the patch (_2), clang (and gcc 4.2) on macOS warn:

./Modules/_ssl.c:3968:7: warning: assigning to 'unsigned char *' from 'char *'
  converts between pointers to integer types with different sign 
[-Wpointer-sign]
p = PyBytes_AS_STRING(enc_cert);
  ^ ~~~

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28747] Expose SSL_CTX_set_cert_verify_callback

[Steve Dower]

Steve Dower added the comment:

Should have assigned this to me, as I expect I'll be the one to apply it.

Christian - I need to look to you for whether I've exposed the right function 
here and it's not adding security risk (obviously excluding a broken callback 
implementation). I *think* it's okay, but I trust your greater experience here.

3.6.0b4 is being tagged tomorrow and I think this is worth getting in - hence 
why I added Ned. There's no added functionality and no impact if the API isn't 
used. The latest patch removes the '_' prefix but happy to put it back and 
leave it as undocumented.

If neither of you have any concerns, I'll check it in. As I mentioned, at some 
point early in Python 3.6's release I should have a library available that lets 
the OS do certificate validation, but it needs this callback exposed.

--
assignee: christian.heimes -> steve.dower

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28747] Expose SSL_CTX_set_cert_verify_callback

[Steve Dower]

Steve Dower added the comment:

> Basically you want to replace OpenSSL's X509 verification with Windows' cert 
> validation and just leave the handshake and encryption to OpenSSL?

Yep. (See WinVerifyTrust for the Windows API I'm using.)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28747] Expose SSL_CTX_set_cert_verify_callback

[Christian Heimes]

Christian Heimes added the comment:

IMHO SSL CTX set cert verify callback() is the wrong approach. Your are 
completely bypassing cert validation checks of OpenSSL. The callback has to 
build the chain and perform all checks on its own. By all checks I literally 
mean *all*, 
https://wiki.openssl.org/index.php/Manual:SSL_CTX_set_cert_verify_callback(3)#WARNINGS

Basically you want to replace OpenSSL's X509 verification with Windows' cert 
validation and just leave the handshake and encryption to OpenSSL?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28747] Expose SSL_CTX_set_cert_verify_callback

[Steve Dower]

Steve Dower added the comment:

Few patch updates - better tests and some docs.

--
Added file: http://bugs.python.org/file45553/28747_2.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28747] Expose SSL_CTX_set_cert_verify_callback

[Steve Dower]

Steve Dower added the comment:

When I was stepping through, this callback avoided all of those lookups, so I 
don't understand how it's being called too late?

This approach basically takes the entire raw certificate and lets the OS do 
whatever it needs. OpenSSL doesn't ever have to crack it open at all (or at 
least when it does, it can assume the whole chain is trusted).

What am I missing here? I've got no doubt I'm missing something, as OpenSSL is 
possibly the most complicated code I've ever seen :)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28747] Expose SSL_CTX_set_cert_verify_callback

[Christian Heimes]

Christian Heimes added the comment:

Hi Steve,

there is a better approach to fix issue20916. The verify callback is not the 
correct API, because it is called too late. We want to hook into the cert 
resolution mechanism of OpenSSL and get trust anchors and CRLs in before 
OpenSSL builds the verification chain.

Instead of a verify cb we have to implement a X509_LOOKUP_METHOD with a 
get_by_subject(). The function looks up X509_LU_CRL or X509_LU_X509 by 
X509_NAME. The other lookups functions (fingerprint, issuer) aren't used to 
look up root CAs.

Then use some CAPI function like CertFindCertificateInStore() with 
CERT_FIND_SUBJECT_NAME to look up the cert, convert it to OpenSSL X509 object, 
copy the additional trust flags from Windows' cert type to the X509_CERT_AUX 
member of OpenSSL's X509 type.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28747] Expose SSL_CTX_set_cert_verify_callback

[Steve Dower]

Steve Dower added the comment:

Oh, I also made the SSL module chain exceptions properly. That's probably the 
biggest and scariest part of the change, but it can't have been overwriting 
exceptions before anyway (because it calls back into Python code to instantiate 
SSLError), so it's only going to chain in the new case of the callback function 
raising.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28747] Expose SSL_CTX_set_cert_verify_callback

[Steve Dower]

Steve Dower added the comment:

Patch attached with code changes and basic tests. Doc changes to follow.

--
keywords: +patch
Added file: http://bugs.python.org/file45551/28747_1.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28747] Expose SSL_CTX_set_cert_verify_callback

[Steve Dower]

New submission from Steve Dower:

As a prerequisite for fixing issues such as issue20916 (dynamic download/update 
of CAs and CRLs), we really need to be able to plug into the certificate 
verification function for OpenSSL.

This patch adds SSLContext._set_cert_verify_callback, which will allow Python 
code to inject its own verification function.

No other functionality is added, but I have proof-of-concept code that uses 
this patch to delegate all certificate handling to Windows and it works 
beautifully (better than I expected :) ).

If possible, I'd like to get this into Python 3.6. I intend to turn that 
proof-of-concept into an actual released library and would like to be able to 
do it sooner rather than later. Targeting 3.6 is the main reason I named the 
function with an underscore, but I'd be happy to drop it.

--
assignee: christian.heimes
components: SSL
messages: 281230
nosy: christian.heimes, ned.deily, steve.dower
priority: normal
severity: normal
stage: patch review
status: open
title: Expose SSL_CTX_set_cert_verify_callback
type: security
versions: Python 3.6, Python 3.7

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com