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

Fix macOS notarization (signing), migrate to notarytool #12735

Merged
merged 5 commits into from
Mar 29, 2024
Merged

Fix macOS notarization (signing), migrate to notarytool #12735

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0393570
migrate to notarytool in script
adamPetho Mar 27, 2024
ce9a2a0
remove unused upload ID and checks
adamPetho Mar 27, 2024
deb40f9
add useful logs
adamPetho Mar 27, 2024
2121b4b
remove unused params
adamPetho Mar 27, 2024
1f97e7b
add comment
adamPetho Mar 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
86 changes: 12 additions & 74 deletions WalletWasabi.Packager/MacSignTools.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Collections.Generic;

Check notice on line 1 in WalletWasabi.Packager/MacSignTools.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (master)

✅ Getting better: Primitive Obsession 

The ratio of primitive types in function arguments decreases from 83.33% to 81.25%, threshold = 30.0%. The functions in this file have too many primitive types (e.g. int, double, float) in their function argument lists. Using many primitive types lead to the code smell Primitive Obsession. Avoid adding more primitive arguments.

Check notice on line 1 in WalletWasabi.Packager/MacSignTools.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (master)

✅ Getting better: String Heavy Function Arguments 

The ratio of strings in function arguments decreases from 83.33% to 81.25%, threshold = 39.0%. The functions in this file have a high ratio of strings as arguments. Avoid adding more.

Check notice on line 1 in WalletWasabi.Packager/MacSignTools.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (master)

✅ No longer an issue: Overall Code Complexity 

The mean cyclomatic complexity in this module is no longer above the threshold
using System.Diagnostics;
using System.IO;
using System.IO.Compression;
Expand Down Expand Up @@ -185,136 +185,137 @@
WaitProcessToFinish(process, "ditto");
}

Notarize(appleId, password, appNotarizeFilePath, bundleIdentifier);
Notarize(appleId, appNotarizeFilePath);
Staple(appPath);

using (var process = Process.Start(new ProcessStartInfo
{
FileName = "spctl",
Arguments = $"-a -t exec -vv \"{appPath}\"",
WorkingDirectory = workingDir,
RedirectStandardError = true
}))
{
var nonNullProcess = WaitProcessToFinish(process, "spctl");
string result = nonNullProcess.StandardError.ReadToEnd();
if (!result.Contains(": accepted"))
{
throw new InvalidOperationException(result);
}
}

Console.WriteLine("Phase: creating the dmg.");

if (File.Exists(dmgFilePath))
{
File.Delete(dmgFilePath);
}

Console.WriteLine("Phase: creating dmg.");

IoHelpers.CopyFilesRecursively(new DirectoryInfo(dmgContentsDir), new DirectoryInfo(dmgPath));

File.Copy(Path.Combine(contentsPath, "WasabiLogo.icns"), Path.Combine(dmgPath, ".VolumeIcon.icns"), true);

var temp = Path.Combine(dmgPath, ".DS_Store.dat");
File.Move(temp, Path.Combine(dmgPath, ".DS_Store"), true);

using (var process = Process.Start(new ProcessStartInfo
{
FileName = "ln",
Arguments = "-s /Applications",
WorkingDirectory = dmgPath
}))
{
WaitProcessToFinish(process, "ln");
}

var hdutilCreateArgs = string.Join(
" ",
new string[]
{
"create",
$"\"{dmgUnzippedFilePath}\"",
"-ov",
$"-volname \"Wasabi Wallet\"",
"-fs HFS+",
$"-srcfolder \"{dmgPath}\""
});

using (var process = Process.Start(new ProcessStartInfo
{
FileName = "hdiutil",
Arguments = hdutilCreateArgs,
WorkingDirectory = dmgPath
}))
{
WaitProcessToFinish(process, "hdiutil");
}

var hdutilConvertArgs = string.Join(
" ",
new string[]
{
"convert",
$"\"{dmgUnzippedFilePath}\"",
"-format UDZO",
$"-o \"{dmgFilePath}\""
});

using (var process = Process.Start(new ProcessStartInfo
{
FileName = "hdiutil",
Arguments = hdutilConvertArgs,
WorkingDirectory = dmgPath
}))
{
WaitProcessToFinish(process, "hdiutil");
}

Console.WriteLine("Phase: signing the dmg file.");

SignFile($"{signArguments} --entitlements \"{entitlementsPath}\" \"{dmgFilePath}\"", dmgPath);

Console.WriteLine("Phase: verifying the signature.");

Verify(dmgFilePath);

Console.WriteLine("Phase: notarize dmg");
Notarize(appleId, password, dmgFilePath, bundleIdentifier);
Notarize(appleId, dmgFilePath);

Console.WriteLine("Phase: staple dmp");
Staple(dmgFilePath);

using (var process = Process.Start(new ProcessStartInfo
{
FileName = "spctl",
Arguments = $"-a -t open --context context:primary-signature -v \"{dmgFilePath}\"",
WorkingDirectory = workingDir,
RedirectStandardError = true
}))
{
var nonNullProcess = WaitProcessToFinish(process, "spctl");
string result = nonNullProcess.StandardError.ReadToEnd();
if (!result.Contains(": accepted"))
{
throw new InvalidOperationException(result);
}
}

File.Move(dmgFilePath, desktopDmgFilePath);
DeleteWithChmod(workingDir);

Console.WriteLine("Phase: finish.");
Console.WriteLine($"Phase: finished for {dmgFileName}.");

var toRemovableFilePath = Path.Combine(removableDriveFolder, Path.GetFileName(desktopDmgFilePath));
File.Move(desktopDmgFilePath, toRemovableFilePath, true);

if (File.Exists(zipPath))
{
File.Delete(zipPath);
}
}
Console.WriteLine("Phase: finished successfully.");

Check warning on line 318 in WalletWasabi.Packager/MacSignTools.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (master)

❌ Getting worse: Complex Method 

Sign already has high cyclomatic complexity, and now it increases in Lines of Code from 245 to 246. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.
}

private static Process WaitProcessToFinish(Process? process, string processName)
Expand All @@ -327,85 +328,22 @@
return process;
}

private static void Notarize(string appleId, string password, string filePath, string bundleIdentifier)
private static void Notarize(string appleId, string filePath)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed the unused params

{
string? uploadId = null;

Console.WriteLine("Start notarizing, uploading file.");

using (var process = Process.Start(new ProcessStartInfo
// -p WasabiNotarize = Saved the credentials in the keychain profile which keeps the password safe on the local machine. Name of the profile is "WasabiNotarize".
using var process = Process.Start(new ProcessStartInfo
{
FileName = "xcrun",
Arguments = $"altool --notarize-app -t osx -f \"{filePath}\" --primary-bundle-id \"{bundleIdentifier}\" -u \"{appleId}\" -p \"{password}\" --output-format xml",
Arguments = $"notarytool submit --wait --apple-id \"{appleId}\" -p \"WasabiNotarize\" \"{filePath}\" ",
RedirectStandardOutput = true,
}))
{
var nonNullProcess = WaitProcessToFinish(process, "xcrum");
string result = nonNullProcess.StandardOutput.ReadToEnd();

if (result.Contains("The software asset has already been uploaded. The upload ID is"))
{
// Example: The software asset has already been uploaded. The upload ID is 7689dc08-d6c8-4783-8d28-33e575f5c967
uploadId = result.Split('"').First(line => line.Contains("The software asset has already been uploaded.")).Split("The upload ID is")[^1].Trim();
}
else if (result.Contains("No errors uploading"))
{
// Example: <key>RequestUUID</key>\n\t\t<string>2a2a164f-2ae7-4293-8357-5d5a5cdd580a</string>

var lines = result.Split('\n');

for (int i = 0; i < lines.Length; i++)
{
string line = lines[i].Trim();
if (!line.TrimStart().StartsWith("<key>", StringComparison.InvariantCultureIgnoreCase))
{
continue;
}

if (line.Contains("<key>RequestUUID</key>", StringComparison.InvariantCulture))
{
uploadId = lines[i + 1].Trim().Replace("<string>", "").Replace("</string>", "");
}
}
}
Comment on lines -346 to -370
Copy link
Collaborator Author

@adamPetho adamPetho Mar 27, 2024 •
edited

Choose a reason for hiding this comment

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

We don't need the uploadId anymore, so these hacks are obsolete.

}

if (uploadId is null)
{
throw new InvalidOperationException("Cannot get uploadId. Notarization failed.");
}
});

Stopwatch sw = new();
sw.Start();
while (true) // Wait for the notarization.
{
Console.WriteLine($"Checking notarization status. Elapsed time: {sw.Elapsed}");
using var process = Process.Start(new ProcessStartInfo
{
FileName = "xcrun",
Arguments = $"altool --notarization-info \"{uploadId}\" -u \"{appleId}\" -p \"{password}\"",
RedirectStandardError = true,
RedirectStandardOutput = true,
});
var nonNullProcess = WaitProcessToFinish(process, "xcrum");
string result = $"{nonNullProcess.StandardError.ReadToEnd()} {nonNullProcess.StandardOutput.ReadToEnd()}";
if (result.Contains("Status Message: Package Approved"))
{
break;
}
if (result.Contains("Status: in progress"))
{
Thread.Sleep(4000);
continue;
}
if (result.Contains("Could not find the RequestUUID"))
{
Thread.Sleep(4000);
continue;
}
Comment on lines -378 to -405
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No need to periodically send a request to Apple about the notarization's status.

Thanks to the --wait option.

var nonNullProcess = WaitProcessToFinish(process, "xcrum");
string result = nonNullProcess.StandardOutput.ReadToEnd();

throw new InvalidOperationException(result);
}
Console.WriteLine(result);

Check notice on line 346 in WalletWasabi.Packager/MacSignTools.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (master)

✅ No longer an issue: Bumpy Road Ahead 

Notarize is no longer above the threshold for logical blocks with deeply nested code. The Bumpy Road code smell is a function that contains multiple chunks of nested conditional logic. The deeper the nesting and the more bumps, the lower the code health.
}

private static void Staple(string filePath)
Expand Down