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

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).

We can implement it using two scripts/checks as Amos suggest.

>
>
> If it is easy, we could also allow folks to define the corresponding
> decision macro manually, when building Squid, so that they can overwrite
> our automation when needed (e.g., when compiling Squid for use with a
> different OpenSSL version). I do not think we need to make that choice
> an explicit ./configure option though.
>
>
> Thank you,
>
> Alex.
>
>