[Bro] Bro and OpenSSL 1.1

[Johanna Amann]

Hello Hilko,

> as Debian is transitioning to using OpenSSL 1.1 in the upcoming release
> (9.x "stretch"), we are forced to deal with widespread API breakage
> because many data structures that had previously been considered part of
> the API have been made opaque. Many of these changes are fairly easy to
> implement by using getter/setter functions instead. (The main time-sink
> for me was locating those functions in the OpenSSL sources.)

Thanks a lot for doing this; I was aware that we will have to do that at
some point of time, but I have not really looked into this myself.

Just to make sure - the OpenSSL 1.1 API is incompatible to the older API?
(If the answer is yes - as I assume - this will mean quite a few
ifdefs...)

> For the bro package, some work-in-progress patches can be found in our
> bug tracking system[1].

Thanks. Can we just use the patches as a starting point when we add
support to Bro itself?

> One missing piece (apart from running tests with real packet trace data)
> is that some OCSP details cannot yet be accessed through OpenSSL 1.1's
> current set of API functions. Specifically, the function
> 
>     X509* x509_get_ocsp_signer(STACK_OF(X509) *certs, OCSP_RESPID *rid)

Heh. Yes, that was actually one of the things that I already worried about
when writing that specific code - it required messing around way too deep
the internal data structures of OpenSSL; it makes validating an OCSP reply
at an arbitrary time really hard.

> from src/file_analysis/analyzer/x509/functions.bif cannot currently be
> ported. There's ongoing work to fix that[2] in upstream OpenSSL, but we
> don't know yet whether this change will be ready in time for the freeze
> leading to the next Debian release. So, we are thinking that we may have
> to disable the x509_ocsp_verify function and anything that uses it.
> 
> Does anyone have any advice on what to look for when disabling that
> functionality? Or is there maybe a less intrusive alternative that we
> haven't discovered yet?

After thinkina about this for a bit - I think the best way in that case is
to disable this functionality; when looking at the code just now, I found
a small bug in it and I think solving it will actually increase the
reliance on x509_get_ocsp_signer.

In theory, one could also choose to just patch out everything in the
current code that uses signer (it is not that much), and rely completely
on the validation logic inside of OpenSSL -- however, that will fail on
traces, especially once they are older (you can't pass a validation time
to the ocsp validation function).

So - yes - disabling this seems like the better way to me at the moment.

I think all you need to do to disable this is remove the function and also
remove policy/protocols/ssl/validate-ocsp.bro. Alternatively, you could
make the function always return X509_V_ERR_APPLICATION_VERIFICATION with
an error string describing that the functionality is disabled in your
build. This might actually be the better solution since it won't break
custom scripts that rely on this functionality.

Thanks again for looking into this :)
 Johanna