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

Entitlement to sign the QtWebEngineProcess with an exception. #2445

Merged
merged 1 commit into from
Sep 18, 2020

Conversation

camilasan
Copy link
Member

@camilasan camilasan commented Sep 17, 2020

Fix for #1793: The problem seems to be related enabling hardened runtime.
This exception allows the webview to load.

@camilasan camilasan added this to the Desktop 3.0 milestone Sep 17, 2020
@camilasan camilasan changed the title @camilasan Entitlement to sign the QtWebEngineProcess with an exception. Entitlement to sign the QtWebEngineProcess with an exception. Sep 17, 2020
Copy link
Member

@misch7 misch7 left a comment

Choose a reason for hiding this comment

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

Great catch! :-)

Sad we have to opt-out from some protection here but of course that's not surprising.

Dropping this link for documentation:
https://developer.apple.com/documentation/bundleresources/entitlements/com_apple_security_cs_disable-executable-page-protection

@misch7
Copy link
Member

misch7 commented Sep 18, 2020

/rebase

Fix for #1793: The problem seems to be related enabling hardened runtime.
This exception allows the webview to load.

Signed-off-by: Camila San <hello@camila.codes>
@nextcloud-desktop-bot
Copy link

AppImage file: Nextcloud-PR-2445-9c5a51bb07bb2a1a1730bab9144d5d6e674edad5-x86_64.AppImage

To test this change/fix you can simply download above AppImage file and test it.

Please make sure to quit your existing Nextcloud app and backup your data.

@misch7 misch7 merged commit f451e1d into master Sep 18, 2020
@misch7 misch7 deleted the fix-mac-catalina-login branch September 18, 2020 04:44
@camilasan
Copy link
Member Author

/backport to stable-3.0

@@ -16,3 +16,4 @@ configure_file(create_mac.sh.cmake ${CMAKE_CURRENT_BINARY_DIR}/create_mac.sh)
configure_file(macosx.pkgproj.cmake ${CMAKE_CURRENT_BINARY_DIR}/macosx.pkgproj)
configure_file(pre_install.sh.cmake ${CMAKE_CURRENT_BINARY_DIR}/pre_install.sh)
configure_file(post_install.sh.cmake ${CMAKE_CURRENT_BINARY_DIR}/post_install.sh)
configure_file(QtWebEngineProcess.entitlements ${CMAKE_CURRENT_BINARY_DIR}/QtWebEngineProcess.entitlements)
Copy link
Member

Choose a reason for hiding this comment

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

I think this line is unneeded, cmake is not doing any processing on that file.

Copy link
Member Author

Choose a reason for hiding this comment

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

to be honest I was following what is been done for the other files and this path in the build folder is what is being used in our daily build script and brander to execute the sh files and now to sign the QtWebEngineProcess. Or should I have also called it QtWebEngineProcess.entitlements.cmake? I am not sure I know why that is done with the other files.

Copy link
Member

Choose a reason for hiding this comment

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

OK, I guess our build script and sh files are a bit limited then. It'd be better if they'd knew about the source dir as well.

As for the other files, if you got something .cmake used with configure_file, it will generate a new file (usually dropping the .cmake extension) in which CMake variables are expanded (@VARIABLE_NAME@ syntax). In this case it's a bit surprising to process a file containing no such variable.

Now looking at the doc, I see there's COPYONLY option I didn't know about which would have been fitting here. For details: https://cmake.org/cmake/help/latest/command/configure_file.html

Oh well, in that particular case no harm done apart from processing the file while it wasn't needed.

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

Successfully merging this pull request may close these issues.

None yet

4 participants