diff --git a/CHANGES b/CHANGES index 174cc1463e..006f827db8 100644 --- a/CHANGES +++ b/CHANGES @@ -1,4 +1,15 @@ +2.0-beta-12 | 2011-11-03 15:21:08 -0700 + + * No longer write to the PacketFilter::LOG stream if not reading + traffic. (Seth Hall) + +2.0-beta-10 | 2011-11-03 15:17:08 -0700 + + * Notice framework documentation update. (Seth Hall) + + * Fixing compiler warnings (addresses #388) (Jon Siwek) + 2.0-beta | 2011-10-27 17:46:28 -0700 * Preliminary fix for SSH login detection: we need a counted measure diff --git a/VERSION b/VERSION index 83a60b28b1..827f665253 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -2.0-beta +2.0-beta-12 diff --git a/aux/binpac b/aux/binpac index c379cefad9..29143bd0d3 160000 --- a/aux/binpac +++ b/aux/binpac @@ -1 +1 @@ -Subproject commit c379cefad994004e6ca5f6ba7de038ab6da3a5f5 +Subproject commit 29143bd0d3f15d4e8903acd0e60fd9280a31e6f7 diff --git a/aux/bro-aux b/aux/bro-aux index 6c5c999d9f..17b69fd96b 160000 --- a/aux/bro-aux +++ b/aux/bro-aux @@ -1 +1 @@ -Subproject commit 6c5c999d9fe05f0e08cc55af478b6e3e47d15c53 +Subproject commit 17b69fd96b13a63a7ac66812b360a93e2ce0695d diff --git a/aux/broccoli b/aux/broccoli index 7ad1a04b3e..c951bda3c8 160000 --- a/aux/broccoli +++ b/aux/broccoli @@ -1 +1 @@ -Subproject commit 7ad1a04b3e9e63e164de00e62bd0db0f2e4597fc +Subproject commit c951bda3c8ee7719a129f8e15639479b3c80657c diff --git a/aux/broctl b/aux/broctl index 6d284656e4..a152042822 160000 --- a/aux/broctl +++ b/aux/broctl @@ -1 +1 @@ -Subproject commit 6d284656e4c5317a83d8ac58fe51ac8157269284 +Subproject commit a152042822adfce7cdf7291262645b03ac5f8199 diff --git a/cmake b/cmake index bbf129bd7b..019ea8f3e0 160000 --- a/cmake +++ b/cmake @@ -1 +1 @@ -Subproject commit bbf129bd7bd33dfb5641ff0d9242f4b3ebba8e82 +Subproject commit 019ea8f3e0416144c461d55b41bb935a550d9dfd diff --git a/doc/cluster.rst b/doc/cluster.rst index bf6f00b625..afd5f2fe35 100644 --- a/doc/cluster.rst +++ b/doc/cluster.rst @@ -13,7 +13,7 @@ The figure below illustrates the main components of a Bro cluster. .. {{git_pull('bro:doc/deployment.png')}} -.. image:: deployment.png +.. image:: deployment.bro.png Tap *** diff --git a/doc/index.rst b/doc/index.rst index 5cbe82eec6..c7c85bd21f 100644 --- a/doc/index.rst +++ b/doc/index.rst @@ -19,7 +19,7 @@ Bro Documentation `FAQ <{{docroot}}/documentation/faq.html>`_ A list with frequently asked questions. -`How to Report a Problem <{{docroot}}/reporting-problems.html>`_ +`How to Report a Problem <{{docroot}}/documentation/reporting-problems.html>`_ Some advice for when you see Bro doing something you believe it shouldn't. @@ -29,8 +29,8 @@ Frameworks Bro comes with a number of frameworks, some of which are described in more detail here: -`Reporting <{{git('bro:doc/notice.rst')}}>`_ - The notice/alarm framework. +`Notice <{{git('bro:doc/notice.rst')}}>`_ + The notice framework. `Logging <{{git('bro:doc/logging.rst')}}>`_ Customizing and extensing Bro's logging. diff --git a/doc/notice.rst b/doc/notice.rst index 81c65b1411..1322d44585 100644 --- a/doc/notice.rst +++ b/doc/notice.rst @@ -9,7 +9,7 @@ Notice Framework situations, and the notice policy tells which of them the user wants to be acted upon in some manner. In particular, the notice policy can specify actions to be taken, such as sending an email or compiling regular - alarm emails. This page gives an introduction into writing such a notice + alarm emails. This page gives an introduction into writing such a notice policy. .. contents:: @@ -17,7 +17,7 @@ Notice Framework Overview -------- -Let us start with a little bit of background on Bro's philosophy on reporting +Let's start with a little bit of background on Bro's philosophy on reporting things. Bro ships with a large number of policy scripts which perform a wide variety of analyses. Most of these scripts monitor for activity which might be of interest for the user. However, none of these scripts determines the @@ -25,28 +25,21 @@ importance of what it finds itself. Instead, the scripts only flags situations as *potentially* interesting, leaving it to the local configuration to define which of them are in fact actionable. This decoupling of detection and reporting allows Bro to address the different needs that sites have: -definitions of what constitutes an attack differ quite a bit between -environments, and activity deemed malicious at one site might be fully -acceptable at another. +definitions of what constitutes an attack or even a compromise differ quite a +bit between environments, and activity deemed malicious at one site might be +fully acceptable at another. Whenever one of Bro's analysis scripts sees something potentially interesting -it flags the situation by calling the NOTICE function and giving it -a single Notice::Info record. A Notice has a -Notice::Type, which reflects the kind of activity that has been -seen, and it is usually also augmented with further context about the -situation. For example, whenever the base SSH analysis scripts sees an SSH -session where it is heuristically guessed to be a successful login, it raises -a Notice of the type SSH::Login. The code in the base SSH analysis script -looks like this: +it flags the situation by calling the ``NOTICE`` function and giving it a +single ``Notice::Info`` record. A Notice has a ``Notice::Type``, which +reflects the kind of activity that has been seen, and it is usually also +augmented with further context about the situation. -.. code:: bro +More information about raising notices can be found in the `Raising Notices`_ +section. - NOTICE([$note=SSH::Login, - $msg="Heuristically detected successful SSH login.", - $conn=c]); - -Once a notice is raised, it can have any number of actions attached to it by -the Notice::policy which is described in the `Notice Policy`_ +Once a notice is raised, it can have any number of actions applied to it by +the ``Notice::policy`` set which is described in the `Notice Policy`_ section below. Such actions can be to send a mail to the configured address(es) or to simply ignore the notice. Currently, the following actions are defined: @@ -59,49 +52,45 @@ are defined: - Description * - Notice::ACTION_LOG - - Write the notice to the Notice::LOG logging stream. + - Write the notice to the ``Notice::LOG`` logging stream. * - Notice::ACTION_ALARM - - Log into the Notice::ALARM_LOG stream which will rotate + - Log into the ``Notice::ALARM_LOG`` stream which will rotate hourly and email the contents to the email address or addresses - defined in the Notice::mail_dest variable. + defined in the ``Notice::mail_dest`` variable. * - Notice::ACTION_EMAIL - Send the notice in an email to the email address or addresses given in - the Notice::mail_dest variable. + the ``Notice::mail_dest`` variable. * - Notice::ACTION_PAGE - Send an email to the email address or addresses given in the - Notice::mail_page_dest variable. + ``Notice::mail_page_dest`` variable. * - Notice::ACTION_NO_SUPPRESS - This action will disable the built in notice suppression for the - notice. Keep in mind that this action will need to be attached to + notice. Keep in mind that this action will need to be applied to every notice that shouldn't be suppressed including each of the future notices that would have normally been suppressed. How these notice actions are applied to notices is discussed in the `Notice Policy`_ and `Notice Policy Shortcuts`_ sections. +Processing Notices +------------------ + Notice Policy -------------- +************* -The predefined set Notice::policy provides the mechanism for -applying actions and other behavior modifications to notices. Each entry of -Notice::policy defines a combination of several things, a condition -to be matched against all raised notices, an action to be taken if the -condition matches, and/or a interval to suppress that distinct notice with the -``$suppress_for`` field. The notice policy is defined by adding any number of -Notice::Info records to the Notice::policy set. +The predefined set ``Notice::policy`` provides the mechanism for applying +actions and other behavior modifications to notices. Each entry of +``Notice::policy`` is a record of the type ``Notice::PolicyItem`` which +defines a condition to be matched against all raised notices and one or more +of a variety of behavior modifiers. The notice policy is defined by adding any +number of ``Notice::PolicyItem`` records to the ``Notice::policy`` set. -Here's a simple example which tells Bro to send an email for all Notices of -type SSH::Login if the server is 10.0.0.1: - -.. note:: - - Keep in mind that the semantics of the SSH::Login notice are - such that it is only raised when Bro heuristically detects a successful - login. No apparently failed logins will raise this notice. +Here's a simple example which tells Bro to send an email for all notices of +type ``SSH::Login`` if the server is 10.0.0.1: .. code:: bro @@ -112,25 +101,73 @@ type SSH::Login if the server is 10.0.0.1: $action = Notice::ACTION_EMAIL] }; -While the syntax might look a bit convoluted at first, it provides a lot of -flexibility due to having access to Bro's full programming language. ``$pred`` -defines the entry's condition in the form of a predicate written as a Bro -function. The function is passed the Notice::Info record and it -returns a boolean indicating whether the entry applies. If the predicate -evaluates to true (``T``), Bro applies any values found in both the -``$action`` and ``$suppress_for`` fields. The lack of a predicate in a -Notice::PolicyItem is implicitly true since an implicit false -(``F``) value would never be used. +.. note:: + + Keep in mind that the semantics of the SSH::Login notice are + such that it is only raised when Bro heuristically detects a successful + login. No apparently failed logins will raise this notice. + +While the syntax might look a bit convoluted at first, it provides a lot of +flexibility due to having access to Bro's full programming language. + +Predicate Field +^^^^^^^^^^^^^^^ + +The ``Notice::PolicyItem`` record type has a field name ``$pred`` which +defines the entry's condition in the form of a predicate written as a Bro +function. The function is passed the notice as a ``Notice::Info`` record and +it returns a boolean value indicating if the entry is applicable to that +particular notice. + +.. note:: + + The lack of a predicate in a ``Notice::PolicyItem`` is implicitly true + (``T``) since an implicit false (``F``) value would never be used. -The Notice::policy set can hold an arbitrary number of such entries. Bro evaluates the predicates of each entry in the order defined by the -``$priority`` field. If multiple predicates evaluate to true, it is undefined -which of the matching results is taken. One can however associate a *priority* -with an entry by adding a field ``$priority=`` to its definition; see -``policy/notice-policy.bro`` for examples. In the case of multiple matches -with different priorities, Bro picks the one with the highest. If -``$priority`` is omitted, as it is in the example above, the default priority -is 1. +``$priority`` field in ``Notice::PolicyItem`` records. The valid values are +0-10 with 10 being earliest evaluated. If ``$priority`` is omitted, the +default priority is 5. + +Behavior Modification Fields +^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +There are a set of fields in the ``Notice::PolicyItem`` record type that +indicate ways that either the notice or notice processing should be modified +if the predicate field (``$pred``) evaluated to true (``T``). Those fields are +explained in more detail in the following table. + +.. list-table:: + :widths: 20 30 20 + :header-rows: 1 + + * - Field + - Description + - Example + + * - ``$action=`` + - Each Notice::PolicyItem can have a single action applied to the notice + with this field. + - ``$action = Notice::ACTION_EMAIL`` + + * - ``$suppress_for=`` + - This field makes it possible for a user to modify the behavior of the + notice framework's automated suppression of intrinsically similar + notices. More information about the notice framework's automated + suppression can be found in the `Automated Suppression`_ section of + this document. + - ``$suppress_for = 10mins`` + + * - ``$halt=`` + - This field can be used for modification of the notice policy + evaluation. To stop processing of notice policy items before + evaluating all of them, set this field to ``T`` and make the ``$pred`` + field return ``T``. ``Notice::PolicyItem`` records defined at a higher + priority as defined by the ``$priority`` field will still be evaluated + but those at a lower priority won't. + - ``$halt = T`` + + .. code:: bro @@ -144,16 +181,16 @@ is 1. Notice Policy Shortcuts ------------------------ +*********************** Although the notice framework provides a great deal of flexibility and configurability there are many times that the full expressiveness isn't needed and actually becomes a hindrance to achieving results. The framework provides -a default Notice::policy suite as a way of giving users the +a default ``Notice::policy`` suite as a way of giving users the shortcuts to easily apply many common actions to notices. These are implemented as sets and tables indexed with a -Notice::Type enum value. The following table shows and describes +``Notice::Type`` enum value. The following table shows and describes all of the variables available for shortcut configuration of the notice framework. @@ -165,35 +202,185 @@ framework. - Description * - Notice::ignored_types - - Adding a Notice::Type to this set results in the notice + - Adding a ``Notice::Type`` to this set results in the notice being ignored. It won't have any other action applied to it, not even - Notice::ACTION_LOG. + ``Notice::ACTION_LOG``. * - Notice::emailed_types - - Adding a Notice::Type to this set results in - Notice::ACTION_EMAIL being applied to the notices of that - type. + - Adding a ``Notice::Type`` to this set results in + ``Notice::ACTION_EMAIL`` being applied to the notices of that type. * - Notice::alarmed_types - Adding a Notice::Type to this set results in - Notice::ACTION_ALARM being applied to the notices of that - type. + ``Notice::ACTION_ALARM`` being applied to the notices of that type. * - Notice::not_suppressed_types - - Adding a Notice::Type to this set results in that notice - no longer undergoing the normal notice suppression that would take - place. Be careful when using this in production it could result in a - dramatic increase in the number of notices being processed. + - Adding a ``Notice::Type`` to this set results in that notice no longer + undergoing the normal notice suppression that would take place. Be + careful when using this in production it could result in a dramatic + increase in the number of notices being processed. + + * - Notice::type_suppression_intervals + - This is a table indexed on ``Notice::Type`` and yielding an interval. + It can be used as an easy way to extend the default suppression + interval for an entire ``Notice::Type`` without having to create a + whole ``Notice::policy`` entry and setting the ``$suppress_for`` + field. + +Raising Notices +--------------- + +A script should raise a notice for any occurrence that a user may want to be +notified about or take action on. For example, whenever the base SSH analysis +scripts sees an SSH session where it is heuristically guessed to be a +successful login, it raises a Notice of the type ``SSH::Login``. The code in +the base SSH analysis script looks like this: + +.. code:: bro + + NOTICE([$note=SSH::Login, + $msg="Heuristically detected successful SSH login.", + $conn=c]); + +``NOTICE`` is a normal function in the global namespace which wraps a function +within the ``Notice`` namespace. It takes a single argument of the +``Notice::Info`` record type. The most common fields used when raising notices +are described in the following table: + +.. list-table:: + :widths: 32 40 + :header-rows: 1 + + * - Field name + - Description + + * - ``$note`` + - This field is required and is an enum value which represents the + notice type. + + * - ``$msg`` + - This is a human readable message which is meant to provide more + information about this particular instance of the notice type. + + * - ``$sub`` + - This is a sub-message which meant for human readability but will + frequently also be used to contain data meant to be matched with the + ``Notice::policy``. + + * - ``$conn`` + - If a connection record is available when the notice is being raised + and the notice represents some attribute of the connection the + connection record can be given here. Other fields such as ``$id`` and + ``$src`` will automatically be populated from this value. + + * - ``$id`` + - If a conn_id record is available when the notice is being raised and + the notice represents some attribute of the connection, the connection + be given here. Other fields such as ``$src`` will automatically be + populated from this value. + + * - ``$src`` + - If the notice represents an attribute of a single host then it's + possible that only this field should be filled out to represent the + host that is being "noticed". + + * - ``$n`` + - This normally represents a number if the notice has to do with some + number. It's most frequently used for numeric tests in the + ``Notice::policy`` for making policy decisions. + + * - ``$identifier`` + - This represents a unique identifier for this notice. This field is + described in more detail in the `Automated Suppression`_ section. + + * - ``$suppress_for`` + - This field can be set if there is a natural suppression interval for + the notice that may be different than the default value. The value set + to this field can also be modified by a user's ``Notice::policy`` so + the value is not set permanently and unchangeably. + +When writing Bro scripts which raise notices, some thought should be given to +what the notice represents and what data should be provided to give a consumer +of the notice the best information about the notice. If the notice is +representative of many connections and is an attribute of a host (e.g. a +scanning host) it probably makes most sense to fill out the ``$src`` field and +not give a connection or conn_id. If a notice is representative of a +connection attribute (e.g. an apparent SSH login) the it makes sense to fill +out either ``$conn`` or ``$id`` based on the data that is available when the +notice is raised. Using care when inserting data into a notice will make later +analysis easier when only the data to fully represent the occurrence that +raised the notice is available. If complete connection information is +available when an SSL server certificate is expiring, the logs will be very +confusing because the connection that the certificate was detected on is a +side topic to the fact that an expired certificate was detected. It's possible +in many cases that two or more separate notices may need to be generated. As +an example, one could be for the detection of the expired SSL certificate and +another could be for if the client decided to go ahead with the connection +neglecting the expired certificate. + +Automated Suppression +--------------------- + +The notice framework supports suppression for notices if the author of the +script that is generating the notice has indicated to the notice framework how +to identify notices that are intrinsically the same. Identification of these +"intrinsically duplicate" notices is implemented with an optional field in +``Notice::Info`` records named ``$identifier`` which is a simple string. +If the ``$identifier`` and ``$type`` fields are the same for two notices, the +notice framework actually considers them to be the same thing and can use that +information to suppress duplicates for a configurable period of time. + +.. note:: + + If the ``$identifier`` is left out of a notice, no notice suppression + takes place due to the framework's inability to identify duplicates. This + could be completely legitimate usage if no notices could ever be + considered to be duplicates. + +The ``$identifier`` field is typically comprised of several pieces of data +related to the notice that when combined represent a unique instance of that +notice. Here is an example of the script +``policy/protocols/ssl/validate-certs.bro`` raising a notice for session +negotiations where the certificate or certificate chain did not validate +successfully against the available certificate authority certificates. + +.. code:: bro + + NOTICE([$note=SSL::Invalid_Server_Cert, + $msg=fmt("SSL certificate validation failed with (%s)", c$ssl$validation_status), + $sub=c$ssl$subject, + $conn=c, + $identifier=cat(c$id$resp_h,c$id$resp_p,c$ssl$validation_status,c$ssl$cert_hash)]); + +In the above example you can see that the ``$identifier`` field contains a +string that is built from the responder IP address and port, the validation +status message, and the MD5 sum of the server certificate. Those fields in +particular are chosen because different SSL certificates could be seen on any +port of a host, certificates could fail validation for different reasons, and +multiple server certificates could be used on that combination of IP address +and port with the ``server_name`` SSL extension (explaining the addition of +the MD5 sum of the certificate). The result is that if a certificate fails +validation and all four pieces of data match (IP address, port, validation +status, and certificate hash) that particular notice won't be raised again for +the default suppression period. + +Setting the ``$identifier`` field is left to those raising notices because +it's assumed that the script author who is raising the notice understands the +full problem set and edge cases of the notice which may not be readily +apparent to users. If users don't want the suppression to take place or simply +want a different interval, they can always modify it with the +``Notice::policy``. -Automated Deduplication ------------------------ +Extending Notice Framework +-------------------------- -Extending Notice Actions ------------------------- +Adding Custom Notice Actions +**************************** -Extending Emails ----------------- +Extending Notice Emails +*********************** Cluster Considerations ----------------------- \ No newline at end of file +---------------------- + diff --git a/scripts/base/frameworks/packet-filter/main.bro b/scripts/base/frameworks/packet-filter/main.bro index 784a7725ed..1097315172 100644 --- a/scripts/base/frameworks/packet-filter/main.bro +++ b/scripts/base/frameworks/packet-filter/main.bro @@ -144,7 +144,8 @@ function install() $sub=default_filter]); } - Log::write(PacketFilter::LOG, info); + if ( reading_live_traffic() || reading_traces() ) + Log::write(PacketFilter::LOG, info); } event bro_init() &priority=10 diff --git a/src/Base64.h b/src/Base64.h index 0fe7e04910..7f351a2c2b 100644 --- a/src/Base64.h +++ b/src/Base64.h @@ -44,7 +44,7 @@ public: if ( analyzer ) analyzer->Weird("base64_illegal_encoding", msg); else - reporter->Error(msg); + reporter->Error("%s", msg); } protected: diff --git a/src/CompHash.cc b/src/CompHash.cc index 6893cafd8f..148c8384ee 100644 --- a/src/CompHash.cc +++ b/src/CompHash.cc @@ -522,7 +522,7 @@ ListVal* CompositeHash::RecoverVals(const HashKey* k) const } if ( kp != k_end ) - reporter->InternalError("under-ran key in CompositeHash::DescribeKey %ld", k_end - kp); + reporter->InternalError("under-ran key in CompositeHash::DescribeKey %zd", k_end - kp); return l; } diff --git a/src/Debug.cc b/src/Debug.cc index 3aa805d5bf..1b849fff7e 100644 --- a/src/Debug.cc +++ b/src/Debug.cc @@ -797,7 +797,7 @@ int dbg_handle_debug_input() input_line = (char*) safe_malloc(1024); input_line[1023] = 0; // ### Maybe it's not always stdin. - fgets(input_line, 1023, stdin); + input_line = fgets(input_line, 1023, stdin); #endif // ### Maybe not stdin; maybe do better cleanup. diff --git a/src/Desc.cc b/src/Desc.cc index 7121253087..c70878de34 100644 --- a/src/Desc.cc +++ b/src/Desc.cc @@ -296,7 +296,7 @@ void ODesc::AddBytesRaw(const void* bytes, unsigned int n) if ( ! write_failed ) // Most likely it's a "disk full" so report // subsequent failures only once. - reporter->Error(fmt("error writing to %s: %s", f->Name(), strerror(errno))); + reporter->Error("error writing to %s: %s", f->Name(), strerror(errno)); write_failed = true; return; diff --git a/src/File.cc b/src/File.cc index 37ce7c0b4b..080923ad37 100644 --- a/src/File.cc +++ b/src/File.cc @@ -149,7 +149,7 @@ BroFile::BroFile(const char* arg_name, const char* arg_access, BroType* arg_t) t = arg_t ? arg_t : base_type(TYPE_STRING); if ( ! Open() ) { - reporter->Error(fmt("cannot open %s: %s", name, strerror(errno))); + reporter->Error("cannot open %s: %s", name, strerror(errno)); is_open = 0; okay_to_manage = 0; } @@ -641,7 +641,7 @@ void BroFile::InitEncrypt(const char* keyfile) if ( ! key ) { - reporter->Error(fmt("can't open key file %s: %s", keyfile, strerror(errno))); + reporter->Error("can't open key file %s: %s", keyfile, strerror(errno)); Close(); return; } @@ -649,8 +649,8 @@ void BroFile::InitEncrypt(const char* keyfile) pub_key = PEM_read_PUBKEY(key, 0, 0, 0); if ( ! pub_key ) { - reporter->Error(fmt("can't read key from %s: %s", keyfile, - ERR_error_string(ERR_get_error(), 0))); + reporter->Error("can't read key from %s: %s", keyfile, + ERR_error_string(ERR_get_error(), 0)); Close(); return; } @@ -671,8 +671,8 @@ void BroFile::InitEncrypt(const char* keyfile) if ( ! EVP_SealInit(cipher_ctx, cipher_type, &psecret, (int*) &secret_len, iv, &pub_key, 1) ) { - reporter->Error(fmt("can't init cipher context for %s: %s", keyfile, - ERR_error_string(ERR_get_error(), 0))); + reporter->Error("can't init cipher context for %s: %s", keyfile, + ERR_error_string(ERR_get_error(), 0)); Close(); return; } @@ -684,8 +684,8 @@ void BroFile::InitEncrypt(const char* keyfile) fwrite(secret, ntohl(secret_len), 1, f) && fwrite(iv, iv_len, 1, f)) ) { - reporter->Error(fmt("can't write header to log file %s: %s", - name, strerror(errno))); + reporter->Error("can't write header to log file %s: %s", + name, strerror(errno)); Close(); return; } @@ -709,8 +709,8 @@ void BroFile::FinishEncrypt() if ( outl && ! fwrite(cipher_buffer, outl, 1, f) ) { - reporter->Error(fmt("write error for %s: %s", - name, strerror(errno))); + reporter->Error("write error for %s: %s", + name, strerror(errno)); return; } @@ -741,17 +741,17 @@ int BroFile::Write(const char* data, int len) if ( ! EVP_SealUpdate(cipher_ctx, cipher_buffer, &outl, (unsigned char*)data, inl) ) { - reporter->Error(fmt("encryption error for %s: %s", + reporter->Error("encryption error for %s: %s", name, - ERR_error_string(ERR_get_error(), 0))); + ERR_error_string(ERR_get_error(), 0)); Close(); return 0; } if ( outl && ! fwrite(cipher_buffer, outl, 1, f) ) { - reporter->Error(fmt("write error for %s: %s", - name, strerror(errno))); + reporter->Error("write error for %s: %s", + name, strerror(errno)); Close(); return 0; } @@ -798,7 +798,7 @@ void BroFile::UpdateFileSize() struct stat s; if ( fstat(fileno(f), &s) < 0 ) { - reporter->Error(fmt("can't stat fd for %s: %s", name, strerror(errno))); + reporter->Error("can't stat fd for %s: %s", name, strerror(errno)); current_size = 0; return; } diff --git a/src/FileAnalyzer.cc b/src/FileAnalyzer.cc index 1abe88caec..672d1e1e09 100644 --- a/src/FileAnalyzer.cc +++ b/src/FileAnalyzer.cc @@ -74,11 +74,11 @@ void File_Analyzer::InitMagic(magic_t* magic, int flags) *magic = magic_open(flags); if ( ! *magic ) - reporter->Error(fmt("can't init libmagic: %s", magic_error(*magic))); + reporter->Error("can't init libmagic: %s", magic_error(*magic)); else if ( magic_load(*magic, 0) < 0 ) { - reporter->Error(fmt("can't load magic file: %s", magic_error(*magic))); + reporter->Error("can't load magic file: %s", magic_error(*magic)); magic_close(*magic); *magic = 0; } diff --git a/src/LogMgr.cc b/src/LogMgr.cc index 9e320f8810..f78a2a19e0 100644 --- a/src/LogMgr.cc +++ b/src/LogMgr.cc @@ -1453,8 +1453,8 @@ bool LogMgr::Flush(EnumVal* id) void LogMgr::Error(LogWriter* writer, const char* msg) { - reporter->Error(fmt("error with writer for %s: %s", - writer->Path().c_str(), msg)); + reporter->Error("error with writer for %s: %s", + writer->Path().c_str(), msg); } // Timer which on dispatching rotates the filter. diff --git a/src/OSFinger.cc b/src/OSFinger.cc index 70504e8422..f8032d60ee 100644 --- a/src/OSFinger.cc +++ b/src/OSFinger.cc @@ -85,8 +85,8 @@ void OSFingerprint::collide(uint32 id) if (sig[id].ttl % 32 && sig[id].ttl != 255 && sig[id].ttl % 30) { problems=1; - reporter->Warning(fmt("OS fingerprinting: [!] Unusual TTL (%d) for signature '%s %s' (line %d).", - sig[id].ttl,sig[id].os,sig[id].desc,sig[id].line)); + reporter->Warning("OS fingerprinting: [!] Unusual TTL (%d) for signature '%s %s' (line %d).", + sig[id].ttl,sig[id].os,sig[id].desc,sig[id].line); } for (i=0;iWarning(fmt("OS fingerprinting: [!] Duplicate signature name: '%s %s' (line %d and %d).", - sig[i].os,sig[i].desc,sig[i].line,sig[id].line)); + reporter->Warning("OS fingerprinting: [!] Duplicate signature name: '%s %s' (line %d and %d).", + sig[i].os,sig[i].desc,sig[i].line,sig[id].line); } /* If TTLs are sufficiently away from each other, the risk of @@ -277,10 +277,10 @@ do_const: if (sig[id].opt[j] ^ sig[i].opt[j]) goto reloop; problems=1; - reporter->Warning(fmt("OS fingerprinting: [!] Signature '%s %s' (line %d)\n" + reporter->Warning("OS fingerprinting: [!] Signature '%s %s' (line %d)\n" " is already covered by '%s %s' (line %d).", sig[id].os,sig[id].desc,sig[id].line,sig[i].os,sig[i].desc, - sig[i].line)); + sig[i].line); reloop: ; diff --git a/src/PolicyFile.cc b/src/PolicyFile.cc index 53b115048a..9f67ee88a1 100644 --- a/src/PolicyFile.cc +++ b/src/PolicyFile.cc @@ -88,7 +88,8 @@ bool LoadPolicyFileText(const char* policy_filename) // ### This code is not necessarily Unicode safe! // (probably fine with UTF-8) pf->filedata = new char[size+1]; - fread(pf->filedata, size, 1, f); + if ( fread(pf->filedata, size, 1, f) != 1 ) + reporter->InternalError("Failed to fread() file data"); pf->filedata[size] = 0; fclose(f); diff --git a/src/RemoteSerializer.cc b/src/RemoteSerializer.cc index 03588429de..324da2c92b 100644 --- a/src/RemoteSerializer.cc +++ b/src/RemoteSerializer.cc @@ -392,7 +392,7 @@ static bool sendToIO(ChunkedIO* io, ChunkedIO::Chunk* c) { if ( ! io->Write(c) ) { - reporter->Warning(fmt("can't send chunk: %s", io->Error())); + reporter->Warning("can't send chunk: %s", io->Error()); return false; } @@ -404,7 +404,7 @@ static bool sendToIO(ChunkedIO* io, char msg_type, RemoteSerializer::PeerID id, { if ( ! sendCMsg(io, msg_type, id) ) { - reporter->Warning(fmt("can't send message of type %d: %s", msg_type, io->Error())); + reporter->Warning("can't send message of type %d: %s", msg_type, io->Error()); return false; } @@ -419,7 +419,7 @@ static bool sendToIO(ChunkedIO* io, char msg_type, RemoteSerializer::PeerID id, { if ( ! sendCMsg(io, msg_type, id) ) { - reporter->Warning(fmt("can't send message of type %d: %s", msg_type, io->Error())); + reporter->Warning("can't send message of type %d: %s", msg_type, io->Error()); return false; } @@ -715,7 +715,7 @@ bool RemoteSerializer::CloseConnection(PeerID id) Peer* peer = LookupPeer(id, true); if ( ! peer ) { - reporter->Error(fmt("unknown peer id %d for closing connection", int(id))); + reporter->Error("unknown peer id %d for closing connection", int(id)); return false; } @@ -750,14 +750,14 @@ bool RemoteSerializer::RequestSync(PeerID id, bool auth) Peer* peer = LookupPeer(id, true); if ( ! peer ) { - reporter->Error(fmt("unknown peer id %d for request sync", int(id))); + reporter->Error("unknown peer id %d for request sync", int(id)); return false; } if ( peer->phase != Peer::HANDSHAKE ) { - reporter->Error(fmt("can't request sync from peer; wrong phase %d", - peer->phase)); + reporter->Error("can't request sync from peer; wrong phase %d", + peer->phase); return false; } @@ -777,14 +777,14 @@ bool RemoteSerializer::RequestLogs(PeerID id) Peer* peer = LookupPeer(id, true); if ( ! peer ) { - reporter->Error(fmt("unknown peer id %d for request logs", int(id))); + reporter->Error("unknown peer id %d for request logs", int(id)); return false; } if ( peer->phase != Peer::HANDSHAKE ) { - reporter->Error(fmt("can't request logs from peer; wrong phase %d", - peer->phase)); + reporter->Error("can't request logs from peer; wrong phase %d", + peer->phase); return false; } @@ -802,14 +802,14 @@ bool RemoteSerializer::RequestEvents(PeerID id, RE_Matcher* pattern) Peer* peer = LookupPeer(id, true); if ( ! peer ) { - reporter->Error(fmt("unknown peer id %d for request sync", int(id))); + reporter->Error("unknown peer id %d for request sync", int(id)); return false; } if ( peer->phase != Peer::HANDSHAKE ) { - reporter->Error(fmt("can't request events from peer; wrong phase %d", - peer->phase)); + reporter->Error("can't request events from peer; wrong phase %d", + peer->phase); return false; } @@ -869,8 +869,8 @@ bool RemoteSerializer::CompleteHandshake(PeerID id) if ( p->phase != Peer::HANDSHAKE ) { - reporter->Error(fmt("can't complete handshake; wrong phase %d", - p->phase)); + reporter->Error("can't complete handshake; wrong phase %d", + p->phase); return false; } @@ -1138,7 +1138,7 @@ bool RemoteSerializer::SendCaptureFilter(PeerID id, const char* filter) if ( peer->phase != Peer::HANDSHAKE ) { - reporter->Error(fmt("can't sent capture filter to peer; wrong phase %d", peer->phase)); + reporter->Error("can't sent capture filter to peer; wrong phase %d", peer->phase); return false; } @@ -1215,8 +1215,8 @@ bool RemoteSerializer::SendCapabilities(Peer* peer) { if ( peer->phase != Peer::HANDSHAKE ) { - reporter->Error(fmt("can't sent capabilties to peer; wrong phase %d", - peer->phase)); + reporter->Error("can't sent capabilties to peer; wrong phase %d", + peer->phase); return false; } @@ -3011,8 +3011,8 @@ bool RemoteSerializer::SendCMsgToChild(char msg_type, Peer* peer) { if ( ! sendCMsg(io, msg_type, peer ? peer->id : PEER_NONE) ) { - reporter->Warning(fmt("can't send message of type %d: %s", - msg_type, io->Error())); + reporter->Warning("can't send message of type %d: %s", + msg_type, io->Error()); return false; } return true; diff --git a/src/SerialObj.cc b/src/SerialObj.cc index 6921115c56..a8ab969f5e 100644 --- a/src/SerialObj.cc +++ b/src/SerialObj.cc @@ -19,7 +19,7 @@ SerialObj* SerialObj::Instantiate(SerialType type) return o; } - reporter->Error(fmt("Unknown object type 0x%08x", type)); + reporter->Error("Unknown object type 0x%08x", type); return 0; } @@ -29,7 +29,7 @@ const char* SerialObj::ClassName(SerialType type) if ( f != names->end() ) return f->second; - reporter->Error(fmt("Unknown object type 0x%08x", type)); + reporter->Error("Unknown object type 0x%08x", type); return ""; } diff --git a/src/StateAccess.cc b/src/StateAccess.cc index e62904469c..136a006c52 100644 --- a/src/StateAccess.cc +++ b/src/StateAccess.cc @@ -350,7 +350,7 @@ void StateAccess::Replay() v->AsRecordVal()->Assign(idx, op2 ? op2->Ref() : 0); } else - reporter->Error(fmt("access replay: unknown record field %s for assign", field)); + reporter->Error("access replay: unknown record field %s for assign", field); } else if ( t == TYPE_VECTOR ) @@ -411,7 +411,7 @@ void StateAccess::Replay() v->AsRecordVal()->Assign(idx, new_val, OP_INCR); } else - reporter->Error(fmt("access replay: unknown record field %s for assign", field)); + reporter->Error("access replay: unknown record field %s for assign", field); } else if ( t == TYPE_VECTOR ) diff --git a/src/bro.bif b/src/bro.bif index 0fd20397f0..3350de2867 100644 --- a/src/bro.bif +++ b/src/bro.bif @@ -2181,7 +2181,7 @@ function send_id%(p: event_peer, id: string%) : bool ID* i = global_scope()->Lookup(id->CheckString()); if ( ! i ) { - reporter->Error(fmt("send_id: no global id %s", id->CheckString())); + reporter->Error("send_id: no global id %s", id->CheckString()); return new Val(0, TYPE_BOOL); } @@ -2250,7 +2250,7 @@ function get_event_peer%(%) : event_peer Val* v = remote_serializer->GetPeerVal(src); if ( ! v ) { - reporter->Error(fmt("peer %d does not exist anymore", int(src))); + reporter->Error("peer %d does not exist anymore", int(src)); RecordVal* p = mgr.GetLocalPeerVal(); Ref(p); return p; @@ -3295,13 +3295,13 @@ function identify_data%(data: string, return_mime: bool%): string if ( ! *magic ) { - reporter->Error(fmt("can't init libmagic: %s", magic_error(*magic))); + reporter->Error("can't init libmagic: %s", magic_error(*magic)); return new StringVal(""); } if ( magic_load(*magic, 0) < 0 ) { - reporter->Error(fmt("can't load magic file: %s", magic_error(*magic))); + reporter->Error("can't load magic file: %s", magic_error(*magic)); magic_close(*magic); *magic = 0; return new StringVal(""); diff --git a/src/scan.l b/src/scan.l index 88d4e9e5e0..7ebd7894e1 100644 --- a/src/scan.l +++ b/src/scan.l @@ -1030,7 +1030,7 @@ void clear_reST_doc_comments() if ( ! reST_doc_comments ) return; - fprintf(stderr, "Warning: %lu unconsumed reST comments:\n", + fprintf(stderr, "Warning: %zu unconsumed reST comments:\n", reST_doc_comments->size()); print_current_reST_doc_comments(); diff --git a/src/util.cc b/src/util.cc index 4ef83bb4ad..f81eff8f22 100644 --- a/src/util.cc +++ b/src/util.cc @@ -478,22 +478,22 @@ bool ensure_dir(const char *dirname) { if ( errno != ENOENT ) { - reporter->Warning(fmt("can't stat directory %s: %s", - dirname, strerror(errno))); + reporter->Warning("can't stat directory %s: %s", + dirname, strerror(errno)); return false; } if ( mkdir(dirname, 0700) < 0 ) { - reporter->Warning(fmt("can't create directory %s: %s", - dirname, strerror(errno))); + reporter->Warning("can't create directory %s: %s", + dirname, strerror(errno)); return false; } } else if ( ! S_ISDIR(st.st_mode) ) { - reporter->Warning(fmt("%s exists but is not a directory", dirname)); + reporter->Warning("%s exists but is not a directory", dirname); return false; } @@ -506,7 +506,7 @@ bool is_dir(const char* path) if ( stat(path, &st) < 0 ) { if ( errno != ENOENT ) - reporter->Warning(fmt("can't stat %s: %s", path, strerror(errno))); + reporter->Warning("can't stat %s: %s", path, strerror(errno)); return false; } @@ -556,15 +556,15 @@ static bool read_random_seeds(const char* read_file, uint32* seed, if ( stat(read_file, &st) < 0 ) { - reporter->Warning(fmt("Seed file '%s' does not exist: %s", - read_file, strerror(errno))); + reporter->Warning("Seed file '%s' does not exist: %s", + read_file, strerror(errno)); return false; } if ( ! (f = fopen(read_file, "r")) ) { - reporter->Warning(fmt("Could not open seed file '%s': %s", - read_file, strerror(errno))); + reporter->Warning("Could not open seed file '%s': %s", + read_file, strerror(errno)); return false; } @@ -599,8 +599,8 @@ static bool write_random_seeds(const char* write_file, uint32 seed, if ( ! (f = fopen(write_file, "w+")) ) { - reporter->Warning(fmt("Could not create seed file '%s': %s", - write_file, strerror(errno))); + reporter->Warning("Could not create seed file '%s': %s", + write_file, strerror(errno)); return false; } @@ -1024,7 +1024,7 @@ FILE* rotate_file(const char* name, RecordVal* rotate_info) FILE* newf = fopen(tmpname, "w"); if ( ! newf ) { - reporter->Error(fmt("rotate_file: can't open %s: %s", tmpname, strerror(errno))); + reporter->Error("rotate_file: can't open %s: %s", tmpname, strerror(errno)); return 0; } @@ -1033,7 +1033,7 @@ FILE* rotate_file(const char* name, RecordVal* rotate_info) struct stat dummy; if ( link(name, newname) < 0 || stat(newname, &dummy) < 0 ) { - reporter->Error(fmt("rotate_file: can't move %s to %s: %s", name, newname, strerror(errno))); + reporter->Error("rotate_file: can't move %s to %s: %s", name, newname, strerror(errno)); fclose(newf); unlink(newname); unlink(tmpname); @@ -1043,7 +1043,7 @@ FILE* rotate_file(const char* name, RecordVal* rotate_info) // Close current file, and move the tmp to its place. if ( unlink(name) < 0 || link(tmpname, name) < 0 || unlink(tmpname) < 0 ) { - reporter->Error(fmt("rotate_file: can't move %s to %s: %s", tmpname, name, strerror(errno))); + reporter->Error("rotate_file: can't move %s to %s: %s", tmpname, name, strerror(errno)); exit(1); // hard to fix, but shouldn't happen anyway... }