Re: A new workaround for bug 3816: ssl_crtd crash with OpenSSL v...

On 05/10/2013 09:03 AM, Tsantilas Christos wrote:
> I am attaching a small utility which does all possible checks we can do.
>
>
> On 05/10/2013 12:01 AM, Alex Rousskov wrote:
>> On 05/09/2013 01:10 PM, Tsantilas Christos wrote:
>>> On 05/09/2013 05:50 PM, Alex Rousskov wrote:
>>> I can reporoduce the bug with the following simple program:
>>>
>>> int main(int argc, char *argv[])
>>> {
>>> SSLeay_add_ssl_algorithms();
>>> SSL_CTX *sslContext = SSL_CTX_new(SSLv3_method());
>>> SSL *ssl = SSL_new(sslContext);
>>> X509 * cert = SSL_get_certificate(ssl);
>>> return 0;
>>> }
>>>
>>> This program crashes inside a function called by SSL_get_certificate.
>>> I suppose we can check if the program exited normally or not, to decide
>>> if the openSSL SDK is OK or have the bug.
>>
>> That sounds like a good idea to me. I would extend that test code to
>> also include code to test that our workaround compiles. Here is a sketch:
>>
>> int main() {
>> ...
>> X509 *cert1 = workaroundCode(...);
>> X509 *cert2 = directCode(...); // may crash!
>> return cert1 == cert2 ? 0 : 1;
>> }
>>
>> I assume that both cert1 and cert2 will be nil in your test case when
>> everything works correctly, but that does not matter.
>
> Well checking with null certificates maybe is not enough good to detect
> problems. The SSL and SSL_CTX structures used here mostly uninitialized
> so it is possible to get a wrong field which is just zeroed.
>
> But maybe we can add in the check program a hard coded certificate. I am
> using it in the check toll I am attaching.
> Then to check if the workaround code works required the following:
> const char *certTxt = "-----BEGIN CERTIFICATE----\n" .....;
>
> SSLeay_add_ssl_algorithms();
> BIO *certBio = BIO_new(BIO_s_mem());
> BIO_puts(certBio, certTxt);
> X509 * cert = NULL;
> PEM_read_bio_X509(certBio, &cert, 0, 0));
> SSL_CTX *sslContext = SSL_CTX_new(SSLv3_method());
> SSL_CTX_use_certificate(sslContext, cert));
> X509 ***pCert = (X509 ***)sslContext->cert;
> X509 *sslCtxCert = pCert && *pCert ? **pCert : NULL;
> return (sslCtxCert != cert ? 1 : 0);
>
>
>>
>> Then, we can use one of the following two strategies:
>>
>> Conservative (work around the bug if possible and clearly necessary):
>> If the test compiles but crashes, enable workaround.
>> Otherwise, disable workaround.
>>
>> Midway (work around the bug if necessary or if work around works):
>> If the test compiles but crashes, enable workaround.
>> If the test compiles and returns 0, enable workaround.
>> Otherwise, disable workaround.
>>
>> Aggressive (work around the bug if possible):
>> If the test compiles, enable workaround.
>> Otherwise, disable workaround.
>>
>>
>> I think we should use either the Midway or the Aggressive approach to
>> accommodate more cases, but I am not sure. What do you think?
>
> I think the best is the aggressive with some additions:
>
> - Check if the workaround work, using a code similar to the above.
> - If not work try to use SSL object and SSL_get_certificate function. If
> this one crashes, disable SSL bumping (only here needed).

I am worried that we are over-engineering this, which will result in
test failures for reasons other than OpenSSL bugs. However, if you
prefer to add real certificate check, I am not objecting to it. If we do
run into problems with this more complex check, we can backtrack and
simplify it. Just do whatever you think is best.

Thank you,

Alex.
Received on Fri May 10 2013 - 15:18:40 MDT