Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Core dump on running script that modifies connection record in log_stream_policy #3805

Open
Mezzle opened this issue Jul 2, 2024 · 8 comments
Labels
Type: Bug 🐛 Unexpected behavior or output.

Comments

@Mezzle
Copy link

Mezzle commented Jul 2, 2024

The following script causes zeek to core dump

const session_timeout = 60secs; # Adjust X seconds as needed

global sessions: table[string] of string &default_insert=function(a: string): string { return unique_id("S"); } &write_expire=session_timeout;

redef record connection += {
	session: string &log &optional;
};

hook Log::log_stream_policy(c: connection, id: Log::ID) {
    c$session = sessions[fmt("%s-%s-%s", c$id$orig_h, c$id$resp_h, c$id$resp_p)];
};
@bbannier
Copy link
Contributor

bbannier commented Jul 2, 2024

The issue seems to be due to when this event is invoked (early on).

hook Log::log_stream_policy(c: connection, id: Log::ID)
	{
	local x = c$id$orig_h;
	}
$ zeek  -Cr testing/btest/Traces/ssh/reverse-ssh.pcap /tmp/r.zeek
fatal error in <no location>: Val::CONVERTER (time/record) (1719914406.83221)
[1]    6350 abort      zeek -Cr testing/btest/Traces/ssh/reverse-ssh.pcap /tmp/r.zeek

Accessing connection in an event invoked later (like e.g., new_connection) succeeds.

@bbannier bbannier added the Type: Bug 🐛 Unexpected behavior or output. label Jul 2, 2024
@JustinAzoff
Copy link
Contributor

It's not a timing thing, those are the wrong parameters for that hook

@Mezzle
Copy link
Author

Mezzle commented Jul 2, 2024

type StreamPolicyHook: hook(rec: any, id: ID);

What should I be using for the parameters?

@awelzel
Copy link
Contributor

awelzel commented Jul 2, 2024

What should I be using for the parameters?

For rec you should take a type used for columns during a call of Log::create_stream() (e.g. Conn::Info and guard access to rec on id == Conn::LOG), or any as shown and use "generic" helpers. An yes, this seems rather brittle - not sure we can do something though given rec: any to begin with.

@Mezzle
Copy link
Author

Mezzle commented Jul 2, 2024

A stream policy hook can have any type of record come through it though, can't it? If I want to read that record - I need to define it's type, but can't? unless it'll selectively pass in only the type defined?

If I define my own record type for this - with just id - will that work?

@awelzel
Copy link
Contributor

awelzel commented Jul 2, 2024

I haven't tried, but type casting could possibly also work. For Log::log_stream_policy you'd put your own record type in the signature, and ensure that id: ID at runtime is the stream identifier you use it for. Nope, I don't think the id example you sketch. As Johanna pointed out, you might want to look at log extension fields.

If you need more "user guidance", lets continue on Slack or the forum (community.zeek.org). Again, not sure there's an easy way to protect from the runtime fatal error :-/

@JustinAzoff
Copy link
Contributor

Could we maybe have an as? operator that could work like this

type CommonInfo: record {
	uid: string &log;
	id: conn_id &log;
};

hook Log::log_stream_policy(rec: any, id: Log::ID)
	{
	if ( rec as? CommonInfo )
		{
		local info = ( rec as CommonInfo );
		print info$id$orig_h;
		}
	}

@JustinAzoff
Copy link
Contributor

Oh, just noticed we have is for this, but rec is CommonInfo does not work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug 🐛 Unexpected behavior or output.
Projects
None yet
Development

No branches or pull requests

4 participants