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

[Auditbeat] Review Auditbeat's auditd and FIM modules with regards to conforming to ECS #10111

Closed
webmat opened this issue Jan 16, 2019 · 12 comments

Comments

@webmat
Copy link
Contributor

webmat commented Jan 16, 2019

Event categorization is out of scope for the 7.0 feature freeze, since we haven't made a concrete plan yet.

@webmat
Copy link
Contributor Author

webmat commented Jan 16, 2019

auditd

Things are looking pretty good, still a few small findings.

Full event

Sorry for the big paste, but this is a bit more complete than what's in auditbeat/module/auditd/_meta/data.json :-)

  {
    "@timestamp": "2019-01-16T16:24:35.439Z",
    "process": {
      "name": "adduser",
      "executable": "/usr/bin/perl",
      "cwd": "/home/vagrant",
      "args": [
        "/usr/bin/perl",
        "/usr/sbin/adduser",
        "--system",
        "--shell",
        "/bin/sh",
        "--gecos",
        "git version control",
        "--group",
        "--disabled-password",
        "--home",
        "/home/git",
        "git"
      ],
      "pid": "2172",
      "ppid": "2171",
      "title": "/usr/bin/perl /usr/sbin/adduser --system --shell /bin/sh --gecos git version control --group --disabled-password --home /home/gi"
    },
    "file": {
      "device": "00:00",
      "inode": "23928",
      "mode": "0755",
      "uid": "0",
      "gid": "0",
      "owner": "root",
      "group": "root",
      "path": "/usr/sbin/adduser"
    },
    "ecs": {
      "version": "1.0.0-beta2"
    },
    "auditd": {
      "paths": [
        {
          "name": "/usr/sbin/adduser",
          "rdev": "00:00",
          "dev": "08:01",
          "inode": "23928",
          "item": "0",
          "mode": "0100755",
          "nametype": "NORMAL",
          "ogid": "0",
          "ouid": "0"
        },
        {
          "ogid": "0",
          "rdev": "00:00",
          "dev": "08:01",
          "inode": "6519",
          "item": "1",
          "mode": "0100755",
          "name": "/usr/bin/perl",
          "nametype": "NORMAL",
          "ouid": "0"
        },
        {
          "rdev": "00:00",
          "inode": "1992",
          "item": "2",
          "mode": "0100755",
          "name": "/lib64/ld-linux-x86-64.so.2",
          "dev": "08:01",
          "nametype": "NORMAL",
          "ogid": "0",
          "ouid": "0"
        }
      ],
      "sequence": 328,
      "result": "success",
      "session": "4",
      "data": {
        "a0": "561e808d7bf8",
        "tty": "pts2",
        "argc": "12",
        "syscall": "execve",
        "arch": "x86_64",
        "a2": "561e808d3340",
        "a1": "561e808d7fc8",
        "exit": "0",
        "a3": "561e808e5000"
      },
      "summary": {
        "actor": {
          "primary": "vagrant",
          "secondary": "root"
        },
        "object": {
          "primary": "/usr/sbin/adduser",
          "type": "file"
        },
        "how": "adduser"
      }
    },
    "event": {
      "action": "executed",
      "module": "auditd",
      "category": "audit-rule",
      "type": "syscall"
    },
    "user": {
      "gid": "0",
      "egid": "0",
      "fsuid": "0",
      "name_map": {
        "auid": "vagrant",
        "egid": "root",
        "suid": "root",
        "euid": "root",
        "fsgid": "root",
        "fsuid": "root",
        "gid": "root",
        "sgid": "root",
        "uid": "root"
      },
      "uid": "0",
      "fsgid": "0",
      "suid": "0",
      "euid": "0",
      "auid": "1000",
      "sgid": "0"
    },
    "tags": [
      "exec"
    ],
    "host": {
      "os": {
        "family": "debian",
        "name": "Ubuntu",
        "kernel": "4.4.0-141-generic",
        "codename": "xenial",
        "platform": "ubuntu",
        "version": "16.04.5 LTS (Xenial Xerus)"
      },
      "id": "0fbf7167b91d4a638fa27721c4979897",
      "containerized": false,
      "hostname": "ubuntu-xenial",
      "architecture": "x86_64",
      "name": "ubuntu-xenial"
    },
    "agent": {
      "id": "854f70dc-41b1-48d4-8af1-e73ebd3686e7",
      "version": "7.0.0",
      "type": "auditbeat",
      "ephemeral_id": "73be6278-848a-44da-88cc-f379ecf4a38e",
      "hostname": "ubuntu-xenial"
    },
    "service": {
      "type": "auditd"
    }
  }

user

The Auditd module can nest a lot of information under user, especially when there's privilege escalation going on. In the event above, vagrant is sudoing as root.

ECS uses the user field set to describe one user (It's id, name, full_name, etc.), where the Auditd module here uses the namespace to report all of the possible user IDs that will lead to the effective user ID for the event (e.g. suid and auid).

When ECS tackles this, I think the fields for this will either be elsewhere in the event structure, or be exactly this, because this is the exact nomenclature in Posix environments. So I'm not too worried about leaving this as is for now.

Two notable things, however, is that user.id and user.name are not populated yet.

I think the final privilege level should be used to populate user.id and user.name by copying the data over, not by renaming the existing fields. In the above case, user.id:0, user.name:root; in the case of a normal service suid (e.g. Apache), user.name:apache.

WDYT @ruflin and @MikePaquette?

Rest of the event

  • Out of scope for this review: agent, host, ecs and the base fields.
  • auditd section is custom, that's perfectly fine
  • process is fine with one exception
    • process.cwd should be migrated to process.working_directory
  • file and service are fine
  • Type coercions: ideally the PIDs should be coerced to longs. The index mapping has them as long already, so we're mostly fine. But coercing would also ensure the _source also always looks the same across event sources.
    • Note that I'm specifically leaving the extra user IDs out of this comment, since they're not in ECS, and they're currently keyword indexed.

Conclusion

  • process.cwd should be migrated to process.working_directory
  • Coerce process.pid and process.ppid to long
  • Populate user.id and user.name with the effective user
  • Populate group.id and group.name with the effective group

@webmat
Copy link
Contributor Author

webmat commented Jan 16, 2019

fim

FIM events are very small, so not much to review, things are looking fine :-)

  {
      "@timestamp": "2019-01-16T16:39:03.428Z",
      "ecs": {
        "version": "1.0.0-beta2"
      },
      "agent": {
        "id": "854f70dc-41b1-48d4-8af1-e73ebd3686e7",
        "version": "7.0.0",
        "type": "auditbeat",
        "ephemeral_id": "2708466d-88a6-4ba4-94fb-b2b0194f73a9",
        "hostname": "ubuntu-xenial"
      },
      "hash": {
        "sha1": "5ed6f0d4afce5b8d9d7e4d06acc6d1580f419144"
      },
      "event": {
        "dataset": "file",
        "action": [
          "created"
        ],
        "module": "file_integrity"
      },
      "service": {
        "type": "file_integrity"
      },
      "file": {
        "mtime": "2019-01-16T16:39:03.419Z",
        "type": "file",
        "uid": 0,
        "gid": 0,
        "inode": "5398",
        "ctime": "2019-01-16T16:39:03.419Z",
        "owner": "root",
        "size": 1775,
        "mode": "0644",
        "group": "root",
        "path": "/etc/passwd"
      },
      "host": {
        "architecture": "x86_64",
        "os": {
          "kernel": "4.4.0-141-generic",
          "codename": "xenial",
          "platform": "ubuntu",
          "version": "16.04.5 LTS (Xenial Xerus)",
          "family": "debian",
          "name": "Ubuntu"
        },
        "id": "0fbf7167b91d4a638fa27721c4979897",
        "containerized": false,
        "name": "ubuntu-xenial",
        "hostname": "ubuntu-xenial"
      }
    }
  • Out of scope for this review: agent, host, ecs and the base fields.
  • file and service are fine
  • hash is not defined by ECS, but I'm fine with it: if we were to introduce a hash field set at the top level, all its keys would likely be keyword, and if hash.sha1 is defined, the same semantics would likely apply.

Conclusion

FIM is good as is for me

@MikePaquette
Copy link

@webmat
I agree with your recommendation:

I think the final privilege level should be used to populate user.id and user.name by copying the data over, not by renaming the existing fields.

FYI, also as discussed in ECS #234 when we introduce related.user.*, it would be good to copy the name_map values to this array.

@andrewkroh
Copy link
Member

andrewkroh commented Jan 17, 2019

This PR relates the discussion of the user data model: https://github.com/elastic/beats/pull/7841/files

Use

  • user.id.uid
  • user.name.uid (conflicts with user.name)

instead of

  • user.uid
  • user.name_map.uid

@webmat
Copy link
Contributor Author

webmat commented Jan 17, 2019

Yeah what's proposed in there is to move id under id.*. This would be a painful migration again (like source). I don't think we can tackle this at this time. I like the structure he's suggesting, though. We should consider this, and perhaps nest this structure elsewhere.

I'll want to carefully look at how things work on Windows as well. See if we can create something that work for both worlds, or if we need to have one structure for POSIX and one for Windows.

@webmat
Copy link
Contributor Author

webmat commented Jan 17, 2019

To follow up on the discussion we've had elsewhere, I would also recommend populating group.id with user.egid. This will help correlation with other places that are using group.id this way.

I agree that ECS will need to:

  • define how to represent user privilege escalation
  • clarify or review whether a user's group ID, when seen on an event belongs at group.id or nested under the user. This work will be related to the privilege escalation work. Just like with user IDs, multiple group IDs may have to be considered before determining the effective group ID

@cwurm I also think you mentioned that the sets of IDs represented here for privilege escalation is different than what has been happening in the new system module. I agree we would benefit from harmonizing this. However since this isn't in ECS yet and we can't tackle this before FF, I would say it's a decision that is up to you and Andrew on whether to move the fields around on one of the modules to align with the other.

Although you made the point that these details may make more sense under process. than under user., which also makes sense. Perhaps we touch neither of them for now?

@webmat
Copy link
Contributor Author

webmat commented Jan 17, 2019

I've updated my "Conclusion" section for the auditd module to mention populating group.

@cwurm
Copy link
Contributor

cwurm commented Jan 18, 2019

would also recommend populating group.id with user.egid

Why the effective ID? I think it would make more sense to have the real ID, so that when searching for all actions by a user we would find both the non-elevated actions (where real ID == effective ID) and elevated actions (where real ID != effective ID, and effective ID usually root).

It also feels weird to me to set group.id, but keep egid/sgid in user, not group. A POSIX group has only an ID and a name (so much less than a user) so it's a pretty small 2-field structure to have separately. And whenever there's a special group ID (egid/sgid/etc.) there is also always a corresponding special user ID (euid/suid/etc.) so it would make sense to keep them together as well.

I proposed a nested structure in #9963. For auditd, it could look like this:

"user": {
  "id": "0",            // UID on POSIX, SID on Windows - for display purposes, no point in querying across platforms
  "name": "root",
  "group_id": "0"       // GID on POSIX, SID of the primary access token on Windows
  "group_name": "root",
  "effective": {
    "id": "0",
    "name": "root",
    "group_id": "0",
    "group_name": "root",
  },
  "saved": {
    "id": "0",
    "name": "root",
    "group_id": "0",
    "group_name": "root",
  },
  "filesystem": {
    "id": "0",
    "name": "root",
    "group_id": "0",
    "group_name": "root"
  },
  "audit" : {
    "id": "1000",
    "name": "vagrant"
  }
}

The advantages:

  1. No name_map, access to secondary names is natural, e.g. user.audit.name. This gets even more relevant if we want to add more information to a user, e.g.:
    • user.effective.shell vs. user.shell_map.euid
    • user.impersonation.privileges vs. user.privileges_map.iid (an example from Windows, see the last point as well)
  2. No conflict with existing fields, e.g. vs. the user.name.euid idea
  3. No new unique field names at the leaf level (vs. uid/euid/suid, etc.). This would also make documenting it in ECS easy by simply allowing nesting of the user object.
  4. Easily extensible without changes to the data model, e.g. at some point we will add Windows impersonation tokens and then all we have to do is add another impersonation nested user object (what would we call it otherwise, iid for impersonation ID?).

The nesting concept is applicable to other cases as well, e.g. just some ideas:

  • wrapped network traffic with different destinations
    "destination":
      "address": "<outer address>",
      "inner": {
        "address": "<inner address>"
    
  • Docker on a Linux VM:
    "os": {
      "name": "Alpine Linux",
      "outer": {
        "name": "Ubuntu Linux"
    

What do people think?

@webmat
Copy link
Contributor Author

webmat commented Jan 18, 2019

The structure you propose is beautiful in its simplicity. I really love the fact that it's just the user object being nested over again, under various names representing 'effective', 'saved', for Posix environments, and use whatever name makes sense in the Windows world when we get there.

The question of whether to show the effective user id or the real person was indeed why I pinged @MikePaquette and @ruflin initially. I was on the fence about what we should be highlighting

Seeing it as presented above makes me a fan of using the uid, not effective, in user.id. The structure being a direct mapping of what we have in posix environments (with names expanded), without trying to guess what we should highlight will also probably turn out to be a big plus. There's nothing to explain when you see it, it's the direct mapping, and people will know what they want to look at, without needing to reverse engineer our mapping.

Only thing I would change in there is nest the group field set, so use dots like this:

"user": {
  "id": "0",        // UID on POSIX, SID on Windows - for display purposes, no point in querying across platforms
  "name": "root",
  "group" {
    "id": "0",      // GID on POSIX, SID of the primary access token on Windows
    "name": "root"
  }
  "effective": {
    "id": "0",
    "name": "root",
    "group" {
      "id": "0",
      "name": "root"
    }
  },
  "saved": {
    "id": "0",
    "name": "root",
    "group" {
      "id": "0",
      "name": "root"
    }
  },
  "filesystem": {
    "id": "0",
    "name": "root",
    "group" {
      "id": "0",
      "name": "root"
    }
  },
  "audit" : {
    "id": "1000",
    "name": "vagrant"
  }
}

Thanks for putting this much thought into this, @cwurm! It's the work I wanted us to do after FF, but I think you came up with a great proposal here :-)

@cwurm
Copy link
Contributor

cwurm commented Jan 18, 2019

Thanks @webmat. And group adds the final flourish.

andrewkroh added a commit to andrewkroh/beats that referenced this issue Jan 19, 2019
- Rename `process.cwd` to `process.working_directory` in auditd module.
- Change data type of `process.pid` and `process.ppid` to number in JSON output.
- Add user.id (same as UID) and user.name.
- Add group.id (same as GID) and group.name.

Issue elastic#10111
andrewkroh added a commit that referenced this issue Jan 21, 2019
* Changed auditd fields for ECS

- Rename `process.cwd` to `process.working_directory` in auditd module.
- Change data type of `process.pid` and `process.ppid` to number in JSON output.
- Add user.id (same as UID) and user.name.
- Add group.id (same as GID) and group.name.

Issue #10111

* Change file.uid/file.gid to string in JSON output

The JSON data type was number but ECS says it should be a keyword which is a JSON string.

Fixes #9607
webmat added a commit that referenced this issue Jan 30, 2019
This implementation is based on a discussion that happened in issue #10111, and implements Christoph's proposal on how to represent privilege escalation. He also implemented it for the Auditbeat Auditd module.

Note that very few of the migrated fields used to be documented.

- Define these new fields only for Filebeat. They're not in ECS, but may eventually get in there, as nestings of the `user` field set:
  - user.terminal
  - user.audit.group.id
  - user.audit.id
  - user.effective.group.id
  - user.effective.id
  - user.filesystem.group.id
  - user.filesystem.id
  - user.owner.group.id
  - user.owner.id
  - user.saved.group.id
  - user.saved.id
- Migrate the following fields to ECS, and alias the old fields to the new one
  - auditd.log.acct => user.name
  - auditd.log.agid => user.audit.group.id
  - auditd.log.arch => host.architecture
  - auditd.log.auid => user.audit.id
  - auditd.log.cmd => process.args (went from cmdline to args array, so no alias)
  - auditd.log.comm => process.name
  - auditd.log.dst => destination.address
  - auditd.log.egid => user.effective.group.id
  - auditd.log.euid => user.effective.id
  - auditd.log.exe => process.executable
  - auditd.log.fsgid => user.filesystem.group.id
  - auditd.log.fsuid => user.filesystem.id
  - auditd.log.geoip => source.geo.*
  - auditd.log.gid => user.group.id
  - auditd.log.msg => message
  - auditd.log.ogid => user.owner.group.id
  - auditd.log.ouid => user.owner.id
  - auditd.log.pid => process.pid (long)
  - auditd.log.ppid => process.ppid (long)
  - auditd.log.record_type => event.action (lowercased)
  - auditd.log.res => event.outcome
  - auditd.log.rport => source.port
  - auditd.log.sgid => user.saved.group.id
  - auditd.log.src => source.address
  - auditd.log.suid => user.saved.id
  - auditd.log.terminal => user.terminal
  - auditd.log.tty => user.terminal
  - auditd.log.uid => user.id
- Auditd custom fields not migrated, but that are now explicitly documented in the index template:
  - auditd.log.addr
  - auditd.log.laddr
  - auditd.log.lport
  - auditd.log.tty
  - auditd.log.rport
- Add more log samples & expected to the integration tests
@elasticmachine
Copy link
Collaborator

Pinging @elastic/siem (Team:SIEM)

@adriansr
Copy link
Contributor

adriansr commented Jan 17, 2020

I've double checked the suggested changes for auditd and all of them have been applied in #10195. I think we can close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants