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.

Sign up for GitHub

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

Jump to bottom

pe: add authenticode support #358

Closed
wants to merge 1 commit into from
Closed

pe: add authenticode support #358

wants to merge 1 commit into from

Conversation

baloo
Copy link
Contributor

@baloo baloo commented Mar 10, 2023 •
edited

Authenticode is the hashing format used to sign PE binaries. This provides the hash to be signed.

Usage:

use goblin::pe::PE;
use sha2::Sha256;
use std::{fs::File, io::Read};

fn main() {
    let mut buf = Vec::new();
    let mut f =
        File::open("/nix/store/bhsxra1hc7yhja2kzw5rdds90i3w3a54-linux-5.10.147/bzImage").unwrap();
    f.read_to_end(&mut buf).unwrap();

    let pe = PE::parse(&buf).unwrap();
    let hash = pe.authenticode_digest::<Sha256>();

    println!("hash: {:x?}", hash);
}

Fixes #355
cc @RaitoBezarius

@baloo baloo marked this pull request as draft March 10, 2023 06:02
@baloo baloo force-pushed the baloo/authenticode-support branch 2 times, most recently from 37e49e6 to 39eadb5 Compare March 10, 2023 06:26
@baloo baloo marked this pull request as ready for review March 10, 2023 06:26
@baloo baloo force-pushed the baloo/authenticode-support branch 7 times, most recently from 4ef7f0d to 48adabc Compare March 11, 2023 06:39
RaitoBezarius
RaitoBezarius suggested changes Mar 11, 2023
View reviewed changes
Copy link
Contributor

@RaitoBezarius RaitoBezarius left a comment

Choose a reason for hiding this comment

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

Some nitpicks but amazing <3. Thank you so much!

src/pe/authenticode.rs Outdated Show resolved Hide resolved
src/pe/authenticode.rs Outdated Show resolved Hide resolved
src/pe/authenticode.rs Outdated Show resolved Hide resolved
src/pe/mod.rs
@@ -29,6 +31,11 @@ use log::debug;
#[derive(Debug)]
/// An analyzed PE32/PE32+ binary
pub struct PE<'a> {
#[cfg(feature = "pe_source")]
/// Underlying bytes
bytes: &'a [u8],
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not so sure about this, what would be the rationale to introduce this vs. just letting the caller passing the original bytes around?
I will let @m4b weighs on as I don't have fully context on the design goals of the library.

src/pe/mod.rs Outdated Show resolved Hide resolved
src/pe/optional_header.rs Show resolved Hide resolved
src/pe/optional_header.rs Show resolved Hide resolved
@baloo baloo force-pushed the baloo/authenticode-support branch 4 times, most recently from e21d5c5 to b322e50 Compare March 11, 2023 19:13
@baloo
pe: add authenticode support
a2f65e3 
Authenticode is the hashing format used to sign PE binaries. This
provides the hash to be signed.
@baloo baloo force-pushed the baloo/authenticode-support branch from b322e50 to a2f65e3 Compare March 11, 2023 19:14
@m4b
Copy link
Owner

m4b commented Mar 13, 2023

For similar reasons I've noted in #360 it's going to take some serious convincing to add more deps to goblin.

@baloo baloo mentioned this pull request Mar 13, 2023
Closed
@baloo
Copy link
Contributor Author

baloo commented Mar 13, 2023

I get the point about not adding dependencies to goblin itself. I'd be more than happy to implement those as trait and get that implemented in a dependency of both goblin and digest, but there are some data that I need goblin to emit. Namely, the sections I need to omit from the authenticode computation, I don't think I can't get that done outside goblin.

I think it would also be nice to have the "source bytes" in the PE objects, if you're alright with that.

Anyway, thanks for maintaining goblin in the first place.

@baloo baloo mentioned this pull request Mar 13, 2023
Merged
@baloo
Copy link
Contributor Author

baloo commented Mar 13, 2023

(made an alternative in #362)

@baloo baloo closed this Mar 13, 2023
@baloo baloo deleted the baloo/authenticode-support branch May 5, 2023 03:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RaitoBezarius RaitoBezarius RaitoBezarius requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Compute Authentihash
3 participants
@baloo @m4b @RaitoBezarius