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

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

Merged
merged 6 commits into from
Nov 26, 2024
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5d949a1
Use the managed signer to remove the code signature from singlefile b…
jtschuster Nov 21, 2024
1ee174d
Return before calling codesign on non-Mac hosts
jtschuster Nov 22, 2024
3f85250
PR Feedback:
jtschuster Nov 22, 2024
f8ecd43
Resize after memory mapped file is disposed
jtschuster Nov 25, 2024
186dfd2
Rename stream-based helper method
jtschuster Nov 26, 2024
8fb1f72
Rename TryRemoveCodesign, pass OSPlatform.OSX to bundler when testing…
jtschuster Nov 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/installer/managed/Microsoft.NET.HostModel/Bundle/Bundler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ public string GenerateBundle(IReadOnlyList<FileSpec> fileSpecs)
{
if (_target.IsOSX)
{
MachObjectFile.TryRemoveCodesign(bundle);
MachObjectFile.RemoveCodesign(bundle);
}
bundle.Position = bundle.Length;
foreach (var fileSpec in fileSpecs)
Expand Down
3 changes: 1 addition & 2 deletions src/installer/managed/Microsoft.NET.HostModel/MachO/MachObjectFile.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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).

{
long? newLength;
bool resized;
Expand All @@ -184,7 +184,6 @@ public static bool TryRemoveCodesign(FileStream bundle)
{
bundle.SetLength(newLength.Value);
}
return resized;
}

/// <summary>
Expand Down
Loading