Skip to content

Conversation

ayfaouzi
Copy link
Contributor

@ayfaouzi ayfaouzi commented Mar 3, 2025

  • 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? Yes
  • If submitting schema/fields updates, have you generated new artifacts by running make and committed those changes? Yes
  • Is your pull request against main? Unless there is a good reason otherwise, we prefer pull requests against main and will backport as needed.
  • Have you added an entry to the CHANGELOG.next.md? Yes.

@ayfaouzi ayfaouzi requested a review from a team as a code owner March 3, 2025 20:43
Copy link

github-actions bot commented Mar 3, 2025

Documentation changes preview: https://ecs_bk_2452.docs-preview.app.elstc.co/diff

Copy link

github-actions bot commented Mar 3, 2025

Warning

It looks like this PR modifies one or more .asciidoc files. These files are being migrated to Markdown, and any changes merged now will be lost. See the migration guide for details.

1 similar comment
Copy link

github-actions bot commented Mar 3, 2025

Warning

It looks like this PR modifies one or more .asciidoc files. These files are being migrated to Markdown, and any changes merged now will be lost. See the migration guide for details.

@ayfaouzi ayfaouzi requested review from mjwolf and trisch-me March 4, 2025 21:54
@ayfaouzi
Copy link
Contributor Author

ayfaouzi commented Mar 5, 2025

@trisch-me as per our discussion in slack, and as I am only extending code_signature, I don't need an RPC update right ?

@trisch-me
Copy link
Contributor

hey @ayfaouzi what I meant in the slack is that we can skip addition to the OTEL for this namespace. But I don’t think we should skip ECS RFC process
@mjwolf your opinion?

@ayfaouzi
Copy link
Contributor Author

What do you think @mjwolf ?

@mjwolf
Copy link
Contributor

mjwolf commented Mar 21, 2025

@ayfaouzi, I think that given this is one field, we don't need to go through the complete RFC process, but I would like to do some things from the RFC process. Could you:

  • Mark the field as beta, by adding "beta: This field is beta and subject to change." to it.
  • Add additional reviews who are subject matter experts, who could comment on the need for this field, and how it will be used.
  • Review the reuse. Does this field apply to all the additional schemas that it's being added to during generation

@ayfaouzi ayfaouzi force-pushed the ayfaouzi-codesignature-thumbprint branch from 45468be to 2efbad3 Compare March 24, 2025 21:42
@ayfaouzi ayfaouzi requested a review from jdu2600 March 24, 2025 21:42
@ayfaouzi ayfaouzi self-assigned this Mar 24, 2025
@ayfaouzi ayfaouzi force-pushed the ayfaouzi-codesignature-thumbprint branch from 2efbad3 to 77bb0b0 Compare March 24, 2025 22:11
@ayfaouzi
Copy link
Contributor Author

Thanks @mjwolf for the feedback.

git update-index --refresh this looks like it's failing the unit tests.

@jdu2600 what do you think of ?

Review the reuse. Does this field apply to all the additional schemas that it's being added to during generation

@jdu2600
Copy link

jdu2600 commented Mar 27, 2025

We've chosen the SHA256 certificate hash for robustness. Do we need to support MD5 and SHA1 in ECS?


VirusTotal calls the SHA1 certificate hash "Thumbprint" and this field "Thumbprint SHA256" -

Sample VirusTotal API json -

{
  "valid usage": "Code Signing",
  "thumbprint_sha256": "CD0E144DD10BAC221FE2FB901058D16450A0578B3C47C770908F2E9ADA28EF12",
  "name": "GlobalSign GCC R45 EV CodeSigning CA 2020",
  "algorithm": "sha256RSA",
  "thumbprint_md5": "E6EB41AD6404317AF8A18B64F98C2BCF",
  "valid from": "2020-07-28 00:00:00",
  "valid to": "2030-07-28 00:00:00",
  "serial number": "77 BD 0E 05 B7 59 0B B6 1D 47 61 53 1E 3F 75 ED",
  "cert issuer": "GlobalSign Code Signing Root R45",
  "thumbprint": "C10BB76AD4EE815242406A1E3E1117FFEC743D4F"
},

certutil.exe calls this field "Cert Hash(sha256)" -

Cert Hash(md5): 7b40b5b0904b6a2578780cd70a200f57
Cert Hash(sha1): f55115d2439ce0a7529ffaaea654be2c71dce955
Cert Hash(sha256): 9267a08c9fc07b6ab194dc4df3121b264e825330a39ffc42cdb0942f5115eb97
Signature Hash: 7aa483a68f4954d4b0e23421b74869090ec6b86ae8e0ae9f1124eb2014c55d03

explorer.exe calls the the sha1 certificate hash "Thumbprint" -

@jdu2600 jdu2600 requested a review from joe-desimone March 27, 2025 04:29
@ayfaouzi
Copy link
Contributor Author

ayfaouzi commented Apr 3, 2025

Feedback addressed @jdu2600, thanks !

@joe-desimone can you please have a look ?

@ayfaouzi ayfaouzi requested review from jdu2600 and mjwolf April 3, 2025 22:33
@ayfaouzi ayfaouzi changed the title Add thumbprint to code_signature.* Add thumbprint_sha256 to code_signature.* Apr 3, 2025
Copy link

@jdu2600 jdu2600 left a comment

Choose a reason for hiding this comment

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

LGTM.
Suggest that we include the pattern - even though it is a hint for now.

@ayfaouzi
Copy link
Contributor Author

Can you please have a look again ?

@ayfaouzi ayfaouzi merged commit 5f48175 into main May 12, 2025
5 checks passed
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.

5 participants