Skip to content

[RFC] Create Mach-O - Stage 1 Proposal#1346

Closed
peasead wants to merge 49 commits intoelastic:mainfrom
peasead:file.macho-create
Closed

[RFC] Create Mach-O - Stage 1 Proposal#1346
peasead wants to merge 49 commits intoelastic:mainfrom
peasead:file.macho-create

Conversation

@peasead
Copy link
Copy Markdown
Contributor

@peasead peasead commented Apr 7, 2021

RFC Preview

Resolves #1096

  • Have you signed the contributor license agreement? Yes
  • Have you followed the contributor guidelines? Yes
  • For proposing substantial changes or additions to the schema, have you reviewed the RFC process? Yes
  • If submitting code/script changes, have you verified all tests pass locally using make test? NA at this stage
  • If submitting schema/fields updates, have you generated new artifacts by running make and committed those changes? NA at this stage
  • Is your pull request against master? Unless there is a good reason otherwise, we prefer pull requests against master and will backport as needed. Yes
  • Have you added an entry to the CHANGELOG.next.md? NA at this stage

peasead and others added 30 commits October 20, 2020 14:37
Prepare link to Logs docs changing with the 7.10 release in "products…
Prepare link to Logs docs changing with the 7.10 release in "getting-…
Co-authored-by: Andrew Stucki <andrew.stucki@gmail.com>
Co-authored-by: Andrew Stucki <andrew.stucki@gmail.com>
Co-authored-by: Andrew Stucki <andrew.stucki@gmail.com>
Co-authored-by: Andrew Stucki <andrew.stucki@gmail.com>
Co-authored-by: Andrew Stucki <andrew.stucki@gmail.com>
Co-authored-by: Andrew Stucki <andrew.stucki@gmail.com>
Co-authored-by: Eric Beahan <ebeahan@gmail.com>
peasead and others added 4 commits February 16, 2021 15:02
Co-authored-by: Derek Ditch <dcode@users.noreply.github.com>
Co-authored-by: Derek Ditch <dcode@users.noreply.github.com>
@peasead peasead added the RFC label Apr 7, 2021
@peasead peasead self-assigned this Apr 7, 2021
@peasead peasead mentioned this pull request Apr 7, 2021
@peasead
Copy link
Copy Markdown
Contributor Author

peasead commented Apr 9, 2021

Thanks for the update @dcode

@ebeahan
Copy link
Copy Markdown
Member

ebeahan commented Apr 12, 2021

Since we're losing some context from the original PR #1097, was there ever a final resolution for this discussion? #1097 (comment)

| macho.headers.flags | keyword | Flags set in the Mach-O header. |
| macho.segments | nested | Segment information for the file. |
| macho.segments.name | keyword | Name of this segment. |
| macho.segments.physical_offset | long | File offset of this segment. |
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The macho.segments fields in the sample field definitions are using:

  • macho.segments.fileoff
  • macho.segments.filesize
  • macho.segments.flags
  • macho.segments.name
  • macho.segements.offset
  • macho.segments.sections
  • macho.segements.size
  • macho.segments.vmaddr
  • macho.segments.vmsize

Looks like there's some alignment, but some fields are mismatched. I prefer the more verbose naming here in the table.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, the more verbose naming is better. We'll get that cleaned up.

Copy link
Copy Markdown

@devonakerr devonakerr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - sorry for the delayed approval.

@ebeahan
Copy link
Copy Markdown
Member

ebeahan commented May 3, 2021

Thanks, @devonakerr, for reviewing.

The one item I still have is the fields list doesn't match the definitions. Once that's addressed, I believe we'll be in good shape to advance this one.

cc @peasead @dcode

@peasead
Copy link
Copy Markdown
Contributor Author

peasead commented May 4, 2021

Thanks, @devonakerr, for reviewing.

The one item I still have is the fields list doesn't match the definitions. Once that's addressed, I believe we'll be in good shape to advance this one.

cc @peasead @dcode

@ebeahan I think we're cleaned up now.


**Stage 0**

This RFC is to create the Mach-O sub-field within the `file.` fieldset. This will include 35 sub-fields. `macho` itself is a nested
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to leave the top-level ECS fields all as objects.

What about defining macho.architectures as nested and having each architecture's fields defined underneath? Borrowing from what @andrewstucki proposed in elastic/beats#24195.

- name: macho
   title: Mach-O file information.
   type: group
   description: These fields contain macOS Mach Object (Mach-O) metadata.
   fields:
     - name: architectures
        description: Object files contained inside this file by architecture
        type: nested
        fields:
          - name: cpu
             description: CPU architecture target for the file.
             type: keyword
             example: 64-bit

          - name: byte_order
             description: Byte order for the file.
             type: keyword
             example: little-endian

          - name: type
             description: Mach-O file type.
             type: keyword

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, you're suggesting changing:

  • macho.cpu.{architecture,byte_order,type,...} to macho.architecture.{cpu,byte_order,type,...}?
  • macho.* being a group instead of nested and macho.architecture.cpu{cpu,byte_order,type,...} being nested?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, mostly. One difference - I'm proposing macho.architecture being the nested field that all the other fields would fall under for each architecture type.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The goal is that the "interface" to ELF, MachO, and PE have a similar feel. This will make building analytics and understanding of the data model. I discussed with @andrewstucki a while back about means to do that. The problem is that MachO fat binaries are literally two fully, self-sufficient executables in one big MachO wrapper. This means that any field that we decide to nest as architecture specific will have to contain all the architecture-dependent data. The result of this discussion is that it probably made the most sense to make file.macho as a nested type, which drastically simplifies the data.

We should meet to discuss.

Co-authored-by: Eric Beahan <ebeahan@gmail.com>
@peasead peasead marked this pull request as draft May 28, 2021 17:22
@peasead
Copy link
Copy Markdown
Contributor Author

peasead commented Nov 11, 2021

Closing until ready to revisit this.

@peasead peasead closed this Nov 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Create file.mach-o

5 participants