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

feat: parsing code signature data + more load commands for mach-o #78

Merged
merged 18 commits into from
Feb 23, 2024

Conversation

latonis
Copy link
Contributor

@latonis latonis commented Feb 2, 2024

This PR implements LC_CODE_SIGNATURE parsing for Mach-O and then parses the CS_Superblob, CS_Blob, and CS_Indexes that are available via the offsets in the code signature load command.

  • available to parse in the CS_blobs are certificates and entitlements
  • both implemented

Additionally implemented LC_UUID, LC_DYLD_INFO, LC_DYLD_INFO_ONLY, LC_BUILD_VERSION, LC_VERSION_MIN_MACOSX, LC_VERSION_MIN_IPHONEOS, LC_VERSION_MIN_TVOS, and LC_VERSION_MIN_WATCHOS

@latonis latonis changed the title feat: parsing code signature data feat: parsing code signature data for mach-0 Feb 2, 2024
@latonis latonis changed the title feat: parsing code signature data for mach-0 feat: parsing code signature data for mach-o Feb 2, 2024
@latonis latonis changed the title feat: parsing code signature data for mach-o feat: parsing code signature data + more load commands for mach-o Feb 7, 2024
@latonis
Copy link
Contributor Author

latonis commented Feb 10, 2024

modules::pe::tests::checksum is failing on the nightly test but passing on the other such as stable and no-default-features 🤔

@latonis
Copy link
Contributor Author

latonis commented Feb 10, 2024

@plusvic, everything (including all the old PRs for Mach-O) has been refactored for the new parsing style and is ready for review 😸

lib/Cargo.toml Outdated Show resolved Hide resolved
lib/src/modules/macho/mod.rs Outdated Show resolved Hide resolved
@plusvic plusvic merged commit 8550f8c into VirusTotal:main Feb 23, 2024
15 checks passed
@latonis latonis deleted the macho-codesig branch February 26, 2024 03:12
@plusvic
Copy link
Member

plusvic commented Feb 26, 2024

@latonis I had to remove part of the code introduced in this PR, specifically the portion related to signature parsing (see: 154994e). The problem was the cryptographic-message-syntax crate, which introduced a lot of dependencies on other crates that don't make too much sense. This dependencies were creating linking issues when I tried to compile a larger Golang program that uses YARA-X under the hood. For instance, the cryptographic-message-syntax depends on reqwest, which in turns depends on tokio and some other large crates.

I did a quick try to replace cryptographic-message-syntax with the cms crate, but didn't found a direct equivalent to the SignedData::parse_ber function. If you can take a look at it, it will be great.

@latonis
Copy link
Contributor Author

latonis commented Feb 26, 2024

@latonis I had to remove part of the code introduced in this PR, specifically the portion related to signature parsing (see: 154994e). The problem was the cryptographic-message-syntax crate, which introduced a lot of dependencies on other crates that don't make too much sense. This dependencies were creating linking issues when I tried to compile a larger Golang program that uses YARA-X under the hood. For instance, the cryptographic-message-syntax depends on reqwest, which in turns depends on tokio and some other large crates.

I did a quick try to replace cryptographic-message-syntax with the cms crate, but didn't found a direct equivalent to the SignedData::parse_ber function. If you can take a look at it, it will be great.

Ack, I'll take a look. The unfortunate part (which you discovered) is that Apple uses BER format for the embedded certificates and this was the only library I could find that supports parsing them. All others supported DER but not BER. I noticed the `cryptographic-message-syntax' crate has an issue for this actually (all the dependencies): indygreg/cryptography-rs#9

I'll see if it's worth implementing a PR to make the nuisance (and optional) dependencies truly optional with crate features or if implementing the parsing myself is better.

@latonis
Copy link
Contributor Author

latonis commented Feb 26, 2024

I've opened a PR on the lib to implement optional dependencies via features:
indygreg/cryptography-rs#21

@plusvic
Copy link
Member

plusvic commented Feb 26, 2024

In the future I would like to replace the PE authenticode parser with some pure Rust implementation that doesn't depend on OpenSSL, it looks like the crates in https://github.com/RustCrypto are the most comprehensive collection of cryptographic data structures and algorithm, and it would be great if we can minimize the number of dependencies by using the same set of creates for everything related to cryptography.

@latonis
Copy link
Contributor Author

latonis commented Feb 27, 2024

Ah okay, I'll take a look into that. 😸

@latonis
Copy link
Contributor Author

latonis commented Mar 3, 2024

Coming back to this, Apple previously used BER encoding and has now switched to DER format [1]. So we will need to support both to support the scanning and parsing of older binaries and malware. I'll get a PR going to parse DER format (the newer one), but I'll need to continue investigating how we can parse the BER format ones to parse older samples.

[1] https://developer.apple.com/documentation/xcode/using-the-latest-code-signature-format

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.

2 participants