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

Mac: Fixes for MacOS 13 and Xcode 14 #5002

Merged
merged 6 commits into from
Nov 8, 2022
Merged

Mac: Fixes for MacOS 13 and Xcode 14 #5002

merged 6 commits into from
Nov 8, 2022

Conversation

CharlieFenton
Copy link
Contributor

Fixes BOINC screensaver and Manager "Show Graphics" button functionality to work with MacOS 13 Ventura. This also required a different implementation of security to prevent access to user's files in the unlikely case of malevolent or buggy project graphics apps.

As of MacOS 13.0 Ventura IOSurface cannot be used to share graphics between apps unless they are running as the same user, so the screensaver can no longer run the graphics apps as user boinc_master or user boinc_project. Similarly, project graphics no longer work if run as user boinc_master or user boinc_project when run from the Manager's Show Graphics button, though I don't fully understand why.

As a result, they now must be run as the logged in user, creating a potential security risk. I have implemented a technique to block access to the user's files and other sensitive directories based on an API that Apple uses extensively but has marked deprecated because it is an Apple private API. Apple warns it has deprecated the API because the syntax used to specify the access restrictions one passes to this API is subject to change without notice. But the syntax has remained unchanged for many years, this API is used widely in Apple's software, and the security profile elements we use are quite basic and so are very unlikely to change. I can find no other way to provide this security.

@CharlieFenton CharlieFenton marked this pull request as draft November 7, 2022 06:37
@CharlieFenton
Copy link
Contributor Author

I will be releasing this hot fix for the Mac only. While I have made a major effort to not affect the graphics on other platforms, especially for the "Show graphics" button in BOINC Manager, I have no way to test this on other platforms. That functionality needs to be confirmed on Linux, MS Windows, etc. before the next release for those platforms.

I am less concerned about the screensaver code for MS Windows because the bulk of my changes are in Mac-specific files or guarded by #ifdef __APPLE__. But that functionality should also be thoroughly tested on MS Windows before a release for that platform.

@AenBleidd
Copy link
Member

@CharlieFenton, on linux compilation is failed:

MainDocument.cpp: In member function ‘int CMainDocument::WorkShowGraphics(RESULT*)’:
MainDocument.cpp:1855:17: error: ‘getPathToThisApp’ was not declared in this scope
 1855 |                 getPathToThisApp(path, sizeof(path));
      |                 ^~~~~~~~~~~~~~~~
MainDocument.cpp:1861:27: error: ‘callPosixSpawn’ was not declared in this scope
 1861 |                 iRetVal = callPosixSpawn(cmd);
      |                           ^~~~~~~~~~~~~~
MainDocument.cpp:1868:21: error: ‘struct RUNNING_GFX_APP’ has no member named ‘gfx_pid’
 1868 |             gfx_app.gfx_pid = 0;    // Initialize GetGFXPIDFromForkedPID()
      |                     ^~~~~~~
MainDocument.cpp:1869:21: error: ‘struct RUNNING_GFX_APP’ has no member named ‘gfx_pid’
 1869 |             gfx_app.gfx_pid = GetGFXPIDFromForkedPID(&gfx_app);
      |                     ^~~~~~~
MainDocument.cpp:1869:31: error: ‘GetGFXPIDFromForkedPID’ was not declared in this scope
 1869 |             gfx_app.gfx_pid = GetGFXPIDFromForkedPID(&gfx_app);
      |                               ^~~~~~~~~~~~~~~~~~~~~~

@CharlieFenton
Copy link
Contributor Author

As of MacOS 13.0 Ventura IOSurface cannot be used to share graphics between apps unless they are running as the same user, so the screensaver can no longer run the graphics apps as user boinc_master or user boinc_project.

For an explanation of why we use IOSurface in the Mac screensaver, please see #2149 and #3369.

I think this is ready to be merged when all CI checks have passed.

@CharlieFenton CharlieFenton marked this pull request as ready for review November 8, 2022 14:04
@CharlieFenton
Copy link
Contributor Author

I'm not sure, but I think the Linux failure appears to be due to an internal problem with GitHub Actions rather than related to the code:

Error: Codecov: Failed to properly upload: The process '/home/runner/work/_actions/codecov/codecov-action/v3/dist/codecov' failed with exit code 255

@AenBleidd
Copy link
Member

I'm not sure, but I think the Linux failure appears to be due to an internal problem with GitHub Actions rather than related to the code:

Error: Codecov: Failed to properly upload: The process '/home/runner/work/_actions/codecov/codecov-action/v3/dist/codecov' failed with exit code 255

Yes, you can ignore it, I'll rerun it if needed

@AenBleidd
Copy link
Member

Android failure is unrelated to this PR and will be fixed separately

@AenBleidd AenBleidd self-requested a review November 8, 2022 14:23
Copy link
Member

@AenBleidd AenBleidd left a comment

Choose a reason for hiding this comment

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

Looks good to me

@AenBleidd
Copy link
Member

@RichardHaselgrove, I don't want to block this from being merged and released for OSX only, so could you please make a note to yourself somewhere to test this functionality later when we will be testing a next major release? Thank you in advance

@AenBleidd AenBleidd merged commit 299b9aa into master Nov 8, 2022
@RichardHaselgrove
Copy link
Contributor

I'll put it on the ToDo list.

Item zero: find a project that still issues graphics apps...

@CharlieFenton CharlieFenton deleted the MacOS13_SS_Hotfix branch November 9, 2022 01:07
@CharlieFenton
Copy link
Contributor Author

Item zero: find a project that still issues graphics apps...

I tested with World Community Grid and Einstein, both of which have graphics on the Mac. I believe Rosetta does, also, but I don't currently have any Rosetta tasks to confirm that.

@RichardHaselgrove
Copy link
Contributor

Thanks. I run both those projects, but only on GPUs: the GPU versions of their apps don't include the graphics element (subject to more detailed checking later). I do have CPU-only devices, but they're currently resting because of the price of electricity. I'll fire one or two up again for testing.

@Ken-Dinwiddie
Copy link

You guys must have analyzed the complete error message already. But, because I have seen it so many tooth-grinding times, here is the complete (although discontinuous) sucker. I'm too lazy to dig the continuous version out of the UNIX weeds. Hope the corrected code gets folded in to a new release soon. And how about fixing VirtualBox for late model macOS (Ventura and later) running on M[1,2,3...] processors?
Crash.pdf

@CharlieFenton
Copy link
Contributor Author

@Ken-Dinwiddie The only changes in this BOINC 7.20.4 build from 7.20.2 should be fixes to the "Show Graphics" functionality in BOINC Manager and to the screensaver. Please download and try this build and let us know if you find any problems.

Note: I have noticed that some World Community Grid graphics apps appear to be buggy. They sometime work and sometimes don't, both with this new version 7.20.4 and the previous version 7.20.2, so that problem is not due to these changes in BOINC.

@CharlieFenton
Copy link
Contributor Author

Note: there is a known problem under MacOS 11 and later: the close button no longer works on some graphics apps, though it continues to work on others. It is unclear why. Since the graphics apps are provided by the projects, we have no way to fix this at our end. Our solution (really a workaround) was to modify the BOINC Manager so the "Show Graphics" button changes to "Stop Graphics" when the graphics app is displayed in PR #3970 and #3975.

@Ken-Dinwiddie
Copy link

Ken-Dinwiddie commented Nov 13, 2022 via email

@RichardHaselgrove
Copy link
Contributor

OK, rebooted, and the screensaver is firing up - we're in business. Is that a known thing?

@RichardHaselgrove
Copy link
Contributor

Sorry, that was a follow-up to #5010 (comment) in the other PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants