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

Use the managed signer to remove the code signature from singlefile bundles #110063

Merged
merged 6 commits into from
Nov 26, 2024

Conversation

jtschuster
Copy link
Member

@jtschuster jtschuster commented Nov 21, 2024

When working on using the managed Mac signer in the SDK on non-mac hosts, I found that since the bundler uses codesign to remove the signature, we end up with an invalid signature for single file osx executables. This PR updates the bundler to use the managed signer to remove the signature.

Signing bundles requires a little more thought and effort since the headers/load commands need to be updated to include the bundle data in the file. This will be done in a separate PR.

Part of #110055.

@jtschuster jtschuster added the area-HostModel Microsoft.NET.HostModel issues label Nov 21, 2024
@jtschuster jtschuster added this to the 10.0.0 milestone Nov 21, 2024
Copy link
Contributor

Tagging subscribers to this area: @vitek-karas, @agocke
See info in area-owners.md if you want to be subscribed.

Copy link
Contributor

Tagging subscribers to this area: @agocke, @vitek-karas, @VSadov
See info in area-owners.md if you want to be subscribed.

@am11
Copy link
Member

am11 commented Nov 21, 2024

since the bundler uses codesign to remove the signature, we end up with an invalid signature for single file osx executables

Was it due to some unintentional refactoring? SingleFile was a no-diff in #108992 because it required follow-up work. I understand that ultimately we want to use managed codesign everywhere, just trying to understand do we know what went wrong?

@jtschuster
Copy link
Member Author

since the bundler uses codesign to remove the signature, we end up with an invalid signature for single file osx executables

Was it due to some unintentional refactoring? SingleFile was a no-diff in #108992 because it required follow-up work. I understand that ultimately we want to use managed codesign everywhere, just trying to understand do we know what went wrong?

Nothing went wrong, we would only end up with an invalid signature if we merge the SDK PR (dotnet/sdk#45019) before this. HostWriter.CreateAppHost would create a signed SingleFile host on Windows or Linux, but then Bundler wouldn't remove it before creating the bundle, since it only does signing tasks with codesign.

- Add comment why single file hosts aren't signed on non-mac yet
- Set length of file only if the signature is removed
@@ -170,7 +170,7 @@ public static bool TryRemoveCodesign(MemoryMappedViewAccessor memoryMappedViewAc
/// Removes the code signature load command and signature, and resizes the file if necessary.
/// Returns true if the signature was removed, false otherwise.
/// </summary>
public static bool TryRemoveCodesign(FileStream bundle)
public static void RemoveCodesign(FileStream bundle)
Copy link
Member

Choose a reason for hiding this comment

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

nit: this can be:

    public static void RemoveCodesign(FileStream bundle)
    {
        // Windows doesn't allow a FileStream to be resized while the file is memory mapped, so we must dispose of the memory mapped file first.
        using MemoryMappedFile mmap = MemoryMappedFile.CreateFromFile(bundle, null, 0, MemoryMappedFileAccess.ReadWrite, HandleInheritability.None, true);
        using MemoryMappedViewAccessor accessor = mmap.CreateViewAccessor(0, 0, MemoryMappedFileAccess.ReadWrite);
        if (TryRemoveCodesign(accessor, out long? newLength))
        {
            bundle.SetLength(newLength.Value);
        }
    }

Copy link
Member

Choose a reason for hiding this comment

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

Maybe in else case, we should throw InvalidOperationException("Unable to remove Codesign from bundle.").

Copy link
Member Author

Choose a reason for hiding this comment

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

I was getting failures on Windows when trying to resize the file length while the memory mapped file was still open, so the SetLength has to come after the using block.

The TryRemoveCodesign will only return false if the file isn't a Mach-O binary or there already isn't a signature present. We could throw if it's not a Mach-O, but I don't think it's a failure if the signature isn't present. Maybe we could rename both to RemoveSignatureIfExists instead?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, alright. I think RemoveSignatureIfExists makes sense. If algorithmically we can tell that there is going to be a signature before calling this method, then we can simply throw the exception when TryRemoveCodesign fails (i.e. failure mode is something other than signature wasn't present).

@jtschuster jtschuster merged commit a538054 into dotnet:main Nov 26, 2024
70 checks passed
mikelle-rogers pushed a commit to mikelle-rogers/runtime that referenced this pull request Dec 10, 2024
…undles (dotnet#110063)

Since the bundler uses `codesign` to remove the signature, we end up with an invalid signature for single file osx executables on non-Mac hosts rather than no signature at all. This PR updates the bundler to use the managed signer to remove the signature.

Signing bundles requires a little more thought and effort since the headers/load commands need to be updated to include the bundle data in the file. This will be done in a separate PR.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-HostModel Microsoft.NET.HostModel issues
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants