Mirror/zeek
1
0
Fork 0
mirror of https://github.com/zeek/zeek.git synced 2025-10-08 09:38:19 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Merge remote-tracking branch 'origin/topic/johanna/remove-serializer'

Browse source 
* origin/topic/johanna/remove-serializer:
  Fix memory leak introduced by removing opaque of ocsp_resp.
  Change return value of OpaqueVal::DoSerialize.
  Add missing ShallowClone implementation for SetType
  Remove opaque of ocsp_resp.
  Remove remnants of event serializer.
  Fix cardinalitycounter deserialization.
  Smaller compile fixes for the new opaque serialization.
  Reimplement serialization infrastructure for OpaqueVals.
  Couple of compile fixes.
  Remove const from ShallowClone.
  Remove test-case for removed functionality
  Implement a Shallow Clone operation for types.
  Remove value serialization.

Various changes I made:

- Fix memory leak in type-checker for opaque vals wrapped in broker::data

- Noticed the two "copy-all" leak tests weren't actually checking for
  memory leaks because the heap checker isn't active until after zeek_init()
  is evaluated.

- Change OpaqueVal::DoClone to use the clone caching mechanism

- Improve copy elision for broker::expected return types in the various
  OpaqueVal serialize methods

  - Not all compilers end up properly treating the return of
    local/automatic variable as an rvalue that can be moved, and ends up
    copying it instead.

  - Particularly, until GCC 8, this pattern ends up copying instead of
    moving, and we still support platforms whose default compiler
    pre-dates that version.

  - Generally seems it's something that wasn't addressed until C++14.
    See http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1579

- Change OpaqueVal::SerializeType to return broker::expected

- Change probabilistic DoSerialize methods to return broker::expected
This commit is contained in:
Jon Siwek 2019-06-20 13:23:22 -07:00
commit 399496efa8
102 changed files with 1574 additions and 9140 deletions
Download patch file Download diff file Expand all files Collapse all files

15
src/file_analysis/FileReassembler.cc
View file

@ -110,19 +110,4 @@ void FileReassembler::Overlap(const u_char* b1, const u_char* b2, uint64 n)
{
// Not doing anything here yet.
}
IMPLEMENT_SERIAL(FileReassembler, SER_FILE_REASSEMBLER);
bool FileReassembler::DoSerialize(SerialInfo* info) const
{
reporter->InternalError("FileReassembler::DoSerialize not implemented");
return false; // Cannot be reached.
}
bool FileReassembler::DoUnserialize(UnserialInfo* info)
{
reporter->InternalError("FileReassembler::DoUnserialize not implemented");
return false; // Cannot be reached.
}
} // end file_analysis

2
src/file_analysis/FileReassembler.h
View file

@ -50,8 +50,6 @@ public:
protected:
FileReassembler();
DECLARE_SERIAL(FileReassembler);
void Undelivered(uint64 up_to_seq) override;
void BlockInserted(DataBlock* b) override;
void Overlap(const u_char* b1, const u_char* b2, uint64 n) override;

60
src/file_analysis/analyzer/x509/OCSP.cc
View file

@ -28,8 +28,6 @@ X509* helper_sk_X509_value(const STACK_OF(X509)* certs, int i)
using namespace file_analysis;
IMPLEMENT_SERIAL(OCSP_RESPVal, SER_OCSP_RESP_VAL);
#define OCSP_STRING_BUF_SIZE 2048
static Val* get_ocsp_type(RecordVal* args, const char* name)
@ -177,9 +175,8 @@ bool file_analysis::OCSP::EndOfFile()
return false;
}
OCSP_RESPVal* resp_val = new OCSP_RESPVal(resp); // resp_val takes ownership
ParseResponse(resp_val);
Unref(resp_val);
ParseResponse(resp);
OCSP_RESPONSE_free(resp);
}
return true;
@ -451,9 +448,8 @@ void file_analysis::OCSP::ParseRequest(OCSP_REQUEST* req)
BIO_free(bio);
}
void file_analysis::OCSP::ParseResponse(OCSP_RESPVal *resp_val)
void file_analysis::OCSP::ParseResponse(OCSP_RESPONSE *resp)
{
OCSP_RESPONSE *resp = resp_val->GetResp();
//OCSP_RESPBYTES *resp_bytes = resp->responseBytes;
OCSP_BASICRESP *basic_resp = nullptr;
OCSP_RESPDATA *resp_data = nullptr;
@ -508,7 +504,6 @@ void file_analysis::OCSP::ParseResponse(OCSP_RESPVal *resp_val)
#endif
vl.append(GetFile()->GetVal()->Ref());
vl.append(resp_val->Ref());
vl.append(status_val);
#if ( OPENSSL_VERSION_NUMBER < 0x10100000L ) || defined(LIBRESSL_VERSION_NUMBER)
@ -692,52 +687,3 @@ void file_analysis::OCSP::ParseExtensionsSpecific(X509_EXTENSION* ex, bool globa
ParseSignedCertificateTimestamps(ex);
}
OCSP_RESPVal::OCSP_RESPVal(OCSP_RESPONSE* arg_ocsp_resp) : OpaqueVal(ocsp_resp_opaque_type)
{
ocsp_resp = arg_ocsp_resp;
}
OCSP_RESPVal::OCSP_RESPVal() : OpaqueVal(ocsp_resp_opaque_type)
{
ocsp_resp = nullptr;
}
OCSP_RESPVal::~OCSP_RESPVal()
{
if (ocsp_resp)
OCSP_RESPONSE_free(ocsp_resp);
}
OCSP_RESPONSE* OCSP_RESPVal::GetResp() const
{
return ocsp_resp;
}
bool OCSP_RESPVal::DoSerialize(SerialInfo* info) const
{
DO_SERIALIZE(SER_OCSP_RESP_VAL, OpaqueVal);
unsigned char *buf = nullptr;
int length = i2d_OCSP_RESPONSE(ocsp_resp, &buf);
if ( length < 0 )
return false;
bool res = SERIALIZE_STR(reinterpret_cast<const char*>(buf), length);
OPENSSL_free(buf);
return res;
}
bool OCSP_RESPVal::DoUnserialize(UnserialInfo* info)
{
DO_UNSERIALIZE(OpaqueVal)
int length;
unsigned char *ocsp_resp_buf, *opensslbuf;
if ( ! UNSERIALIZE_STR(reinterpret_cast<char **>(&ocsp_resp_buf), &length) )
return false;
opensslbuf = ocsp_resp_buf; // OpenSSL likes to shift pointers around. really.
ocsp_resp = d2i_OCSP_RESPONSE(nullptr, const_cast<const unsigned char**>(&opensslbuf), length);
delete [] ocsp_resp_buf;
if ( ! ocsp_resp )
return false;
return true;
}

17
src/file_analysis/analyzer/x509/OCSP.h
View file

@ -5,7 +5,6 @@
#include <string>
#include "Val.h"
#include "../File.h"
#include "Analyzer.h"
#include "X509Common.h"
@ -14,8 +13,6 @@
namespace file_analysis {
class OCSP_RESPVal;
class OCSP : public file_analysis::X509Common {
public:
bool DeliverStream(const u_char* data, uint64 len) override;
@ -29,7 +26,7 @@ protected:
OCSP(RecordVal* args, File* file, bool request);
private:
void ParseResponse(OCSP_RESPVal*);
void ParseResponse(OCSP_RESPONSE*);
void ParseRequest(OCSP_REQUEST*);
void ParseExtensionsSpecific(X509_EXTENSION* ex, bool, ASN1_OBJECT*, const char*) override;
@ -37,18 +34,6 @@ private:
bool request = false; // true if ocsp request, false if reply
};
class OCSP_RESPVal: public OpaqueVal {
public:
explicit OCSP_RESPVal(OCSP_RESPONSE *);
~OCSP_RESPVal() override;
OCSP_RESPONSE *GetResp() const;
protected:
OCSP_RESPVal();
private:
OCSP_RESPONSE *ocsp_resp;
DECLARE_SERIAL(OCSP_RESPVal);
};
}
#endif

38
src/file_analysis/analyzer/x509/X509.cc
View file

@ -18,8 +18,6 @@
using namespace file_analysis;
IMPLEMENT_SERIAL(X509Val, SER_X509_VAL);
file_analysis::X509::X509(RecordVal* args, file_analysis::File* file)
: file_analysis::X509Common::X509Common(file_mgr->GetComponentTag("X509"), args, file)
{
@ -491,39 +489,29 @@ Val* X509Val::DoClone(CloneState* state)
return certificate;
}
bool X509Val::DoSerialize(SerialInfo* info) const
IMPLEMENT_OPAQUE_VALUE(X509Val)
broker::expected<broker::data> X509Val::DoSerialize() const
{
DO_SERIALIZE(SER_X509_VAL, OpaqueVal);
unsigned char *buf = NULL;
int length = i2d_X509(certificate, &buf);
if ( length < 0 )
return false;
bool res = SERIALIZE_STR(reinterpret_cast<const char*>(buf), length);
return broker::ec::invalid_data;
auto d = std::string(reinterpret_cast<const char*>(buf), length);
OPENSSL_free(buf);
return res;
return {std::move(d)};
}
bool X509Val::DoUnserialize(UnserialInfo* info)
bool X509Val::DoUnserialize(const broker::data& data)
{
DO_UNSERIALIZE(OpaqueVal)
int length;
unsigned char *certbuf, *opensslbuf;
if ( ! UNSERIALIZE_STR(reinterpret_cast<char **>(&certbuf), &length) )
auto s = caf::get_if<std::string>(&data);
if ( ! s )
return false;
opensslbuf = certbuf; // OpenSSL likes to shift pointers around. really.
certificate = d2i_X509(NULL, const_cast<const unsigned char**>(&opensslbuf), length);
delete[] certbuf;
if ( !certificate )
return false;
return true;
auto opensslbuf = reinterpret_cast<const unsigned char*>(s->data());
certificate = d2i_X509(NULL, &opensslbuf, s->size());
return (certificate != nullptr);
}

5
src/file_analysis/analyzer/x509/X509.h
View file

@ -5,7 +5,7 @@
#include <string>
#include "Val.h"
#include "OpaqueVal.h"
#include "X509Common.h"
#if ( OPENSSL_VERSION_NUMBER < 0x10002000L ) || defined(LIBRESSL_VERSION_NUMBER)
@ -151,10 +151,9 @@ protected:
*/
X509Val();
DECLARE_OPAQUE_VALUE(X509Val)
private:
::X509* certificate; // the wrapped certificate
DECLARE_SERIAL(X509Val);
};
}

5
src/file_analysis/analyzer/x509/ocsp_events.bif
View file

@ -52,9 +52,6 @@ event ocsp_response_status%(f: fa_file, status: string%);
##
## f: The file.
##
## req_ref: An opaque pointer to the underlying OpenSSL data structure of the
## OCSP response.
##
## status: The status of the OCSP response (e.g. succesful, malformedRequest, tryLater).
##
## version: Version of the OCSP response (typically - for version 1).
@ -71,7 +68,7 @@ event ocsp_response_status%(f: fa_file, status: string%);
## .. zeek:see:: ocsp_request ocsp_request_certificate ocsp_response_status
## ocsp_response_certificate ocsp_extension
## x509_ocsp_ext_signed_certificate_timestamp
event ocsp_response_bytes%(f: fa_file, resp_ref: opaque of ocsp_resp, status: string, version: count, responderId: string, producedAt: time, signatureAlgorithm: string, certs: x509_opaque_vector%);
event ocsp_response_bytes%(f: fa_file, status: string, version: count, responderId: string, producedAt: time, signatureAlgorithm: string, certs: x509_opaque_vector%);
## This event is raised for each SingleResponse contained in an OCSP response.
## See :rfc:`6960` for more details on OCSP.