Re-instantiate providing location information to LoadFile hooks.

#1835 subtly changed the semantics of the `LoadFile` plugin hook to no
longer have the current script location available for signature files
being loaded through `@load-sigs`. This was undocumented behavior, so
it's technically not a regression, but since at least one external
plugin is depending on it, this change restores the old behavior.
This commit is contained in:
Robin Sommer 2022-04-14 09:54:45 +02:00
parent d29160e9de
commit fccb9ccab0
No known key found for this signature in database
GPG key ID: 6BEDA4DA6B8B23E3
8 changed files with 66 additions and 11 deletions

View file

@ -38,6 +38,7 @@ public:
#define YYLTYPE zeek::detail::yyltype
using yyltype = Location;
YYLTYPE GetCurrentLocation();
void SetCurrentLocation(YYLTYPE currloc);
// Used to mean "no location associated with this object".
inline constexpr Location no_location("<no location>", 0, 0, 0, 0);

View file

@ -266,6 +266,13 @@ bool RuleMatcher::ReadFiles(const std::vector<SignatureFile>& files)
if ( ! f.full_path )
f.full_path = util::find_file(f.file, util::zeek_path(), ".sig");
// We mimic previous Zeek versions by temporarily setting the current
// script location to the place where the loading happened. This
// behavior was never documented, but seems worth not breaking as some
// plugins ended up relying on it.
Location orig_location = detail::GetCurrentLocation();
detail::SetCurrentLocation(f.load_location);
std::pair<int, std::optional<std::string>> rc = {-1, std::nullopt};
rc.first = PLUGIN_HOOK_WITH_RESULT(
HOOK_LOAD_FILE, HookLoadFile(zeek::plugin::Plugin::SIGNATURES, f.file, *f.full_path),
@ -277,6 +284,9 @@ bool RuleMatcher::ReadFiles(const std::vector<SignatureFile>& files)
HookLoadFileExtended(zeek::plugin::Plugin::SIGNATURES, f.file, *f.full_path),
std::make_pair(-1, std::nullopt));
// Restore original location information.
detail::SetCurrentLocation(orig_location);
switch ( rc.first )
{
case -1:

View file

@ -49,8 +49,9 @@ bool ScannedFile::AlreadyScanned() const
SignatureFile::SignatureFile(std::string file) : file(std::move(file)) { }
SignatureFile::SignatureFile(std::string file, std::string full_path)
: file(std::move(file)), full_path(std::move(full_path))
SignatureFile::SignatureFile(std::string file, std::string full_path, Location load_location)
: file(std::move(file)), full_path(std::move(full_path)),
load_location(std::move(load_location))
{
}

View file

@ -2,6 +2,7 @@
#pragma once
#include <zeek/Obj.h>
#include <list>
#include <optional>
#include <string>
@ -40,9 +41,10 @@ struct SignatureFile
{
std::string file;
std::optional<std::string> full_path;
Location load_location;
SignatureFile(std::string file);
SignatureFile(std::string file, std::string full_path);
SignatureFile(std::string file, std::string full_path, Location load_location);
};
extern std::vector<SignatureFile> sig_files;

View file

@ -374,7 +374,7 @@ when return TOK_WHEN;
@load-sigs{WS}{FILE} {
const char* file = zeek::util::skip_whitespace(yytext + 10);
std::string path = find_relative_file(file, ".sig");
sig_files.emplace_back(file, path);
sig_files.emplace_back(file, path, GetCurrentLocation());
}
@load-plugin{WS}{ID} {
@ -586,6 +586,11 @@ YYLTYPE zeek::detail::GetCurrentLocation()
return currloc;
}
void zeek::detail::SetCurrentLocation(YYLTYPE currloc) {
::filename = currloc.filename;
line_number = currloc.first_line;
}
static int load_files(const char* orig_file)
{
std::string file_path = find_relative_script_file(orig_file);

View file

@ -1,7 +1,9 @@
### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
HookLoadExtended/script: file=|xxx| resolved=|./xxx.zeek|
HookLoadExtended/script: file=|yyy| resolved=||
HookLoadExtended/signature: file=|abc.sig| resolved=|./abc.sig|
HookLoadExtended/script: file=|xxx| resolved=|./xxx.zeek| srcloc=|n/a|
HookLoadExtended/script: file=|xxx3| resolved=|./xxx3.zeek| srcloc=|./xxx2.zeek|
HookLoadExtended/script: file=|yyy| resolved=|| srcloc=|n/a|
HookLoadExtended/signature: file=|abc.sig| resolved=|./abc.sig| srcloc=|n/a|
HookLoadExtended/signature: file=|def.sig| resolved=|./def.sig| srcloc=|./xxx2.zeek|
new zeek_init(): script has been replaced
new zeek_init(): script has been added
signature works!

View file

@ -1,7 +1,7 @@
# @TEST-EXEC: ${DIST}/auxil/zeek-aux/plugin-support/init-plugin -u . Testing LoadFileExtended
# @TEST-EXEC: cp -r %DIR/plugin-load-file-extended/* .
# @TEST-EXEC: ./configure --zeek-dist=${DIST} && make
# @TEST-EXEC: ZEEK_PLUGIN_PATH=$(pwd) zeek -r $TRACES/wikipedia.trace -b Testing::LoadFileExtended xxx yyy -s abc.sig >> output
# @TEST-EXEC: ZEEK_PLUGIN_PATH=$(pwd) zeek -r $TRACES/wikipedia.trace -b Testing::LoadFileExtended xxx xxx2 yyy -s abc.sig >> output
# @TEST-EXEC: btest-diff output
# @TEST-START-FILE xxx.zeek
@ -12,6 +12,20 @@ event zeek_init() {
# @TEST-END-FILE
# @TEST-START-FILE xxx2.zeek
# Test loading from script land.
@load xxx3
@load-sigs def.sig
# @TEST-END-FILE
# @TEST-START-FILE xxx3.zeek
# empty
# @TEST-END-FILE
# @TEST-START-FILE abc.sig
# empty
# @TEST-END-FILE
# @TEST-START-FILE def.sig
# empty
# @TEST-END-FILE

View file

@ -26,9 +26,15 @@ std::pair<int, std::optional<std::string>> Plugin::HookLoadFileExtended(const Lo
const std::string& file,
const std::string& resolved)
{
// Zeek implicitly provides the location where the current '@load'
// originated. If no location is available, filename will be a nullptr.
auto src = zeek::detail::GetCurrentLocation().filename;
if ( ! src )
src = "n/a";
if ( type == LoadType::SCRIPT && file == "xxx" )
{
printf("HookLoadExtended/script: file=|%s| resolved=|%s|\n", file.c_str(), resolved.c_str());
printf("HookLoadExtended/script: file=|%s| resolved=|%s| srcloc=|%s|\n", file.c_str(), resolved.c_str(), src);
return std::make_pair(1, R"(
event zeek_init() {
@ -41,9 +47,16 @@ std::pair<int, std::optional<std::string>> Plugin::HookLoadFileExtended(const Lo
)");
}
if ( type == LoadType::SCRIPT && file == "xxx3" )
{
printf("HookLoadExtended/script: file=|%s| resolved=|%s| srcloc=|%s|\n", file.c_str(), resolved.c_str(), src);
// We don't replace this one.
return std::make_pair(-1, std::nullopt);
}
if ( type == LoadType::SCRIPT && file == "yyy" )
{
printf("HookLoadExtended/script: file=|%s| resolved=|%s|\n", file.c_str(), resolved.c_str());
printf("HookLoadExtended/script: file=|%s| resolved=|%s| srcloc=|%s|\n", file.c_str(), resolved.c_str(), src);
return std::make_pair(1, R"(
event zeek_init() {
@ -54,7 +67,7 @@ std::pair<int, std::optional<std::string>> Plugin::HookLoadFileExtended(const Lo
if ( type == LoadType::SIGNATURES && file == "abc.sig" )
{
printf("HookLoadExtended/signature: file=|%s| resolved=|%s|\n", file.c_str(), resolved.c_str());
printf("HookLoadExtended/signature: file=|%s| resolved=|%s| srcloc=|%s|\n", file.c_str(), resolved.c_str(), src);
return std::make_pair(1, R"(
signature my-sig {
@ -65,6 +78,13 @@ std::pair<int, std::optional<std::string>> Plugin::HookLoadFileExtended(const Lo
)");
}
if ( type == LoadType::SIGNATURES && file == "def.sig" )
{
printf("HookLoadExtended/signature: file=|%s| resolved=|%s| srcloc=|%s|\n", file.c_str(), resolved.c_str(), src);
// We don't replace this one.
return std::make_pair(-1, std::nullopt);
}
return std::make_pair(-1, std::nullopt);
}