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

Fix LastSeenAt in client_create function #3459

Merged
merged 3 commits into from
Apr 29, 2024

Conversation

predictiple
Copy link
Contributor

I discovered that the last_seen_at parameter of the client_create function doesn't seem to be implemented. This is my attempt to make it work but it's still not working.

When I run

SELECT client_create(
         hostname="windozer",
         client_id="C.COJPAN59O9KJ1",
         os="windows",
         first_seen_at=timestamp(epoch=now() - 3600),
         last_seen_at=timestamp(epoch=now()),
         mac_addresses=["b8:ee:65:7c:0d:ff"],
         labels=["lab","test"])
FROM scope()

it does return the client information with the correct last_seen_at value (which it didn't do before this change).

But when I run

SELECT client_info(client_id="C.COJPAN59O9KJ1") FROM scope()

then the last_seen_at value is 0.

I've tried mirroring the code for first_seen_at but I can't see what else needs to change to make this work.

ps. I might have messed up the protobuf stuff. I only needed the 1 change to vql.proto but when I ran ./make_proto.sh it unexpectedly made a lot of changes to all the .pb.go files.

@predictiple predictiple marked this pull request as draft April 27, 2024 09:04
@predictiple predictiple changed the title Add LastSeenAt to client_create function Fix LastSeenAt in client_create function Apr 27, 2024
@scudette
Copy link
Contributor

Normally last seen at time comes from the actual time the client was connected so it will always be updated anyway. Maybe it's not a good idea to let people set it?

It's only going to make sense for offline virtual clients other clients will be constantly updating.

But for those clients the first seen time is more important?

@predictiple
Copy link
Contributor Author

I think it could be useful to be able to set last_seen_at. For example, let's say that you have an analysis query that use last_seen_at as one of it's criteria. Maybe the query shouldn't use last_seen_at but let's assume that it does and that there's a good reason for it, whatever that may be. That query will be fine for connected clients but will fall short in the following scenarios:

  • imported offline collections might be accidentally excluded because such clients have no last_seen_at
  • deaddisk collections might be accidentally included because the "virtual client" will have a more recent last_seen_at than the actual date of the disk image. In this scenario it would be nice to be able to set it back to the date when the image was created.

Basically create_client() provides a handy way to rewrite client info and last_seen_at is necessarily part of that. It's complementary to the client_info() functon and ideally most of what client_info() returns should be able to be written back using client_create(). This allows for reading, making corrections and writing back to client info.

Another use I can imagine is where you have legitimate duplicate hostnames and you want to rename one of them for reporting purposes.

@weslambert
Copy link
Contributor

This makes sense to me. Another option is to report a last_seen value for imports when they are imported, or based off of the collection time. I don't have a lot of input for the dead disk option.

@scudette
Copy link
Contributor

Your use cases imply we should update last seen time on import (which we do already) and when a dead disk client connects its last seen time will be updated too (dead disk clients are just normal clients but with virtualized dead disk visibility).

Since last seen time is not an actual event we cant have an artifact triggered on it so it is only going to be a case where someone checks it and since the value is ephemeral in all normal cases (as it reflects the actual last time seen) the artifact will see valid last seen time.

Perhaps it makes sense to just update last_seen_time to create time on client creation.

@predictiple
Copy link
Contributor Author

It doesn't seem like we update last seen on import. If I create_client and then ImportCollection to that client_id the last seen remains unset. Setting it in that situation seems like a good idea, or at least providing an option to set it via the import artifact, which implies adding that to some function.

I tested also with a dead disk client and it behaves as you've described. In most cases that's fine and won't be a practical concern even though there are philosophical arguments that can be made about what "last seen" means under those circumstances.

My actual use case is just simulating clients for testing and documentation. If I want to test a query - say for example "delete dead clients" - it would be handy to create a bunch of clients of various OSes with a variety of first_seen/last_seen dates. Then maybe ImportCollection on them.

The documentation for client_create() includes a last_seen_at parameter. So either it should work or the documentation needs fixing.

As I mentioned, it would be ideal if client_create() could be the reverse of client_info(), i.e. anything that the latter returns should be writable with client_create(). But that's a much bigger ask obviously.

@predictiple
Copy link
Contributor Author

Interestingly import_collection() does try to set last_seen_at but it uses clients.NewClientFunction, the same as client_create(), which ignores the last_seen_at value.

@scudette scudette marked this pull request as ready for review April 29, 2024 16:31
@scudette scudette merged commit 8c1fa9e into Velocidex:master Apr 29, 2024
3 checks passed
@predictiple predictiple deleted the lastseenat branch April 29, 2024 17:07
scudette added a commit that referenced this pull request May 2, 2024
I discovered that the `last_seen_at` parameter of the client_create
function doesn't seem to be implemented. This is my attempt to make it
work but it's still not working.

When I run
```sql
SELECT client_create(
         hostname="windozer",
         client_id="C.COJPAN59O9KJ1",
         os="windows",
         first_seen_at=timestamp(epoch=now() - 3600),
         last_seen_at=timestamp(epoch=now()),
         mac_addresses=["b8:ee:65:7c:0d:ff"],
         labels=["lab","test"])
FROM scope()
```
it does return the client information with the correct `last_seen_at`
value (which it didn't do before this change).

But when I run
```sql
SELECT client_info(client_id="C.COJPAN59O9KJ1") FROM scope()
```
then the `last_seen_at` value is 0.

I've tried mirroring the code for `first_seen_at` but I can't see what
else needs to change to make this work.

ps. I might have messed up the protobuf stuff. I only needed the 1
change to vql.proto but when I ran `./make_proto.sh` it unexpectedly
made a lot of changes to all the .pb.go files.

---------

Co-authored-by: Mike Cohen <mike@velocidex.com>
scudette added a commit that referenced this pull request May 2, 2024
I discovered that the `last_seen_at` parameter of the client_create
function doesn't seem to be implemented. This is my attempt to make it
work but it's still not working.

When I run
```sql
SELECT client_create(
         hostname="windozer",
         client_id="C.COJPAN59O9KJ1",
         os="windows",
         first_seen_at=timestamp(epoch=now() - 3600),
         last_seen_at=timestamp(epoch=now()),
         mac_addresses=["b8:ee:65:7c:0d:ff"],
         labels=["lab","test"])
FROM scope()
```
it does return the client information with the correct `last_seen_at`
value (which it didn't do before this change).

But when I run
```sql
SELECT client_info(client_id="C.COJPAN59O9KJ1") FROM scope()
```
then the `last_seen_at` value is 0.

I've tried mirroring the code for `first_seen_at` but I can't see what
else needs to change to make this work.

ps. I might have messed up the protobuf stuff. I only needed the 1
change to vql.proto but when I ran `./make_proto.sh` it unexpectedly
made a lot of changes to all the .pb.go files.

---------

Co-authored-by: Mike Cohen <mike@velocidex.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants