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

[feature/confidential-protection] Confidential Content Protection #1430

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

hosy
Copy link
Collaborator

@hosy hosy commented Dec 13, 2024

Description

Confidential View and Screenshot Protection

This update introduces screenshot protection for confidentially marked views in the app and the share sheet extension. When a screenshot is taken, confidential views will not display their content in the screenshot.

To discourage taking photos of confidential views, these views now include a watermark displaying the account’s email and username.

Screenshot protection can be enabled via an MDM parameter.

File or path names displayed in UI elements that are difficult to watermark are redacted after the third character. This ensures the user can identify the file or path name to some extent, but it remains unreadable for security.

Additionally, to prevent sharing confidential data when screenshot protection or confidential view marking is enabled, the following features are disabled:

  • File Provider access
  • Shortcuts Intents
  • System sharing dialog actions
  • Copying files
  • Text recognition on images

This behavior can be overridden using an MDM parameter; however, overriding it is not recommended.

MDM Parameter

MDM Parameter Description
confidential.allow-screenshots Controls whether screenshots are allowed or not. If not allowed confidential views will be marked as sensitive and are not visible in screenshots.
confidential.mark-confidential-views Controls if views which contains sensitive content contains a watermark or not.
confidential.allow-overwrite-confidential-mdm-settings Controls if confidential related MDM settings can be overwritten.

Screenshots

Confidential View

File List Content View Spaces File Provider Shortcuts
Simulator Screenshot - iPhone 16 Plus - 2024-12-16 at 16 05 16 Simulator Screenshot - iPhone 16 Plus - 2024-12-16 at 14 46 09 Simulator Screenshot - iPhone 16 Plus - 2024-12-16 at 14 18 19 Simulator Screenshot - iPhone 16 Plus - 2024-12-16 at 14 18 51 Simulator Screenshot - iPhone 16 Plus - 2024-12-16 at 14 19 39

Screenshot Protection

App Share Extension
IMG_0089 IMG_0088

Testing

Add at least one of the following MDM parameters to the Branding.plist:

Key Type Value
confidential.allow-screenshots Boolean NO
confidential.mark-confidential-views Boolean YES

If you want to allow File Provider access or Actions set the key confidential.allow-overwrite-confidential-mdm-settings to Boolean YES.

Screenshot protection is only working on a physical device!

What to test:

  • Light/dark appearance
  • File Provider access
  • Share sheet
  • Shortcuts
  • All content and editing views
  • Device rotation
  • Multi window environment (iPad)
  • Menu sheets
  • Sidebar
  • Screenshot protection should not show any file names, folders or document content
  • Confidential views (file names, folders, or document content) should contain watermarks, or names should be redacted.

QA:

Test plan: https://github.com/owncloud/QA/blob/master/Mobile/iOS/Executions/Version%2012.4/Content%20Protection.md

Reports:

Related Issue

Motivation and Context

How Has This Been Tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • Added an issue with details about all relevant changes in the iOS documentation repository.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • Added changelog files for the fixed issues in folder changelog/unreleased

hosy added 3 commits December 13, 2024 10:41
This update introduces screenshot protection for confidentially marked views in the app and the share sheet extension. When a screenshot is taken, confidential views will not display their content in the screenshot.

To discourage taking photos of confidential views, these views now include a watermark displaying the account’s email and username.
Screenshot protection can be enabled via an MDM parameter.

Additionally, to prevent sharing confidential data when screenshot protection or confidential view marking is enabled, the following features are disabled:
- File Provider access
- Shortcuts Intents
- System sharing dialog actions
- Copying files
- Text recognition on images
This behavior can be overridden using an MDM parameter; however, overriding it is not recommended.
- Adjusted watermark angle for improved alignment
- Updated GitHub Action workflow:
  - Fetch the latest available OS version
  - Retrieve iPhone simulator device for the selected OS version
@DeepDiver1975
Copy link
Member

To fix CI I suggest to use this action https://github.com/marketplace/actions/mxcl-xcodebuild

In a second step let's define different combinations which we want to build and test in the future

hosy added 2 commits December 16, 2024 14:56
- Updated default values for confidential MDM settings
- Adjusted rotation angle for confidential text
- Added a subtitle to small confidential views and modified text color opacity
- Preserved accessory views by ensuring watermarks are not applied to them
- Added missing file from the last commit
@hosy hosy changed the title [feature/confidential-protection] Confidential View and Screenshot Protection [feature/confidential-protection] Confidential Content Protection Dec 16, 2024
@hosy hosy marked this pull request as ready for review December 17, 2024 09:50
@hosy hosy self-assigned this Dec 17, 2024
@hosy hosy added this to the 12.4-Next milestone Dec 17, 2024
@jesmrec
Copy link
Contributor

jesmrec commented Dec 19, 2024

First, preliminary and exploratory testing, with commit 35fa8a981

Assumed: protection only affects to:

  • Files (name and other parameters)
  • Folders name in navigation
  • File contents
  • Spaces (name and other parameters)

is that right?

1. Screenshot protection:

Using the following set up:

<key>confidential.allow-screenshots</key>
<false/>
  • Side menu is screen-shooted (shouldn't?). I caught a video, the content is blank due to the protection. I took two screenshots: the first one over the list of files is blank (correct ✅ ), the second one over the side menu shows the info
ScreenRecording_12-18-2024.12-22-30_1.MP4

are the username and the account sensible information?

the issue is even more visible in iPad, where the sidebar menu is always in foreground.

  • It's not posible to share content with other users or by link. It's posible in the web and in Android app. That causes a regression.

2. Watermarking

no issues detected at this point.

3. confidential.allow-overwrite-confidential-mdm-settings

true: I wasn't able to test the correct access to the FP's because i always get the following error:

Screenshot 2024-12-19 at 10 35 10

false: FP access has been disabled by the administrator

that was just a first preliminary testing, final testing will be done when CR passed. This first step helped to design the feature test plan.

@hosy
Copy link
Collaborator Author

hosy commented Dec 20, 2024

@hosy There is a redraw error when switching the collection view layout from list to grid or other formats. This issue needs to be fixed.

hosy and others added 7 commits January 16, 2025 14:34
- ConfidentialManager:
	- implement OCClassSettingsSource to inject settings
	- remove effectless "action.allow-image-interactions" from .disallowedActions
- IntentSettings, ImageScrollView, OCFileProviderSettings: remove ConfidentialManager references (=> equivalent implemented via ConfidentialManager<OCClassSettingsSource>)
- Action: use actionIdentifier directly, instead of rawValue
- FileProvider: add ConfidentialManager to build target
- ShareViewController: remove redundant screenshot protection code
- BrowserNavigationViewController: fully adopt UIView.withScreenshotProtection
- ConfidentialContentView, SecureTextField: fix SwiftLint warnings
…n from init/allowedLocationFilter to .provideViewController()
Copy link
Contributor

@felix-schwarz felix-schwarz left a comment

Choose a reason for hiding this comment

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

I've implemented the remaining open findings and suggestions.

@jesmrec
Copy link
Contributor

jesmrec commented Jan 21, 2025

is that QA-ready? @felix-schwarz @hosy

@felix-schwarz
Copy link
Contributor

@jesmrec Yes it is. I was just busy updating the iOS Team project and would've told you in a minute :)

@jesmrec
Copy link
Contributor

jesmrec commented Jan 21, 2025

During QA phase it's assumed that protection only affects to:

  • Files (name and other parameters)
  • Folders name in navigation
  • File contents
  • Spaces (name and other parameters)

@jesmrec
Copy link
Contributor

jesmrec commented Jan 21, 2025

(1) [FIXED]

<key>confidential.mark-confidential-views</key>
<true/>

with that setup, i notice that the space name is not redacted here:

IMG_2542

it should be also redacted as the other red-rounded appearances.

Could you take a look @hosy @felix-schwarz

iPhoneXR iOS18
``d3287bf0d`

@jesmrec
Copy link
Contributor

jesmrec commented Jan 21, 2025

(2) (question)

This is maybe a pretty corner case, or a feature (not a bug)

First three letters of space name are visible and the other letters are redacted. But, if the space name has just three letters (for example, an subject or name abbreviation), it won't be redacted at all.

Should space names to be completely redacted? maybe reducing to two letters of visibilty will avoid more cases of short words or abbreviations that could be infered.

@hosy
Copy link
Collaborator Author

hosy commented Jan 21, 2025

@jesmrec (1) is fixed and some additional titles in copy and delete actions
(2) needs to be discussed. At the moment I would leave it as it is. Thank you!

@jesmrec
Copy link
Contributor

jesmrec commented Jan 21, 2025

@hosy, thanks for fixing (1). You gave me the clue to find other visible file names:

  • The "Move" folder picker shows also the file name:
Screenshot 2025-01-21 at 17 45 58
  • The "Sharing view", "Create link" and "Invite" shows the file name:
Screenshot 2025-01-21 at 17 46 46

@DeepDiver1975
Copy link
Member

conflict on submodulle .... @hosy FYI

@jesmrec
Copy link
Contributor

jesmrec commented Jan 21, 2025

(3) [FIXED]

with the following setup

<key>confidential.allow-screenshots</key>
<false/>
<key>confidential.allow-overwrite-confidential-mdm-settings</key>
<true/>

i get the following error, not being able to open the fp's content:

ScreenRecording_01-21-2025.18-03-10_1.MP4

iPhoneXR iOS18
9df71cfd1

@jesmrec
Copy link
Contributor

jesmrec commented Jan 22, 2025

@hosy @felix-schwarz in one of our meetings we talked about subtitling videos to avoid recording the audio under the mark-confidential-view set up. Was that finally implemented? any detail about it? i could not reproduce that.

@DeepDiver1975
Copy link
Member

Was that finally implemented?

Only a POC was implemented but we left this out of this pull request to not overload this feature

@jesmrec
Copy link
Contributor

jesmrec commented Jan 22, 2025

(4)

  1. Open any image or PDF file with the Markup option

Current:

here i found some problems:

  • files are not opened (seems to be a regression, so, feel free to fix here, it could be delayed to the final release regression test). I can reproduce this in iPhone (iOS18), but not in iPadOS (iOS17) where it works fine.

  • In iPadOS17, the image is visible in markup view. But, taking a screenshot with confidential.allow-screenshots as false, shows the image in the screenshot (it shouldn't).

  • Taking an screenshot, the file name on the top is visible when confidential.allow-screenshots is false

  • With mark-confidential-views enabled, file name on the top is not redacted.

  • If the image/pdf file is not downloaded, a download dialog appears on the bottom with the file name, that is not redacted either.

  • With protection features enabled, the open in icon is shown on the top of markup screen. Open in is a banned operation

Could it be a good idea to remove the Markup as well as Copy or Open in ? on the one hand, it prevents all that errors, on the other hand, as payment feature, it's bad for the user to be prevented of a feature that was paid.

iPhoneXR iOS18
iPadAir2 , iOS 17.4
9df71cfd1

- allow redacting item names in Move and Sharing operations (QA finding fix)
@felix-schwarz
Copy link
Contributor

felix-schwarz commented Jan 23, 2025

@hosy, thanks for fixing (1). You gave me the clue to find other visible file names:

  • The "Move" folder picker shows also the file name:
Screenshot 2025-01-21 at 17 45 58 * The "Sharing view", "Create link" and "Invite" shows the file name: Screenshot 2025-01-21 at 17 46 46

@jesmrec I made sure these are also redacted as of 6f41013, but I'm wondering if this is really necessary. To quote the original description for the redact item name feature: "[…] UI elements that are difficult to watermark are redacted after the third character".

But since those areas already are watermarked, should they really also be redacted?

@felix-schwarz
Copy link
Contributor

felix-schwarz commented Jan 23, 2025

@jesmrec Re (3): assuming this happened on an actual device, I'd think it's a typical memory limit crash & error message that's responsible for this. Please try again with the latest commits, which now include the latest mem-optimizations.

@jesmrec
Copy link
Contributor

jesmrec commented Jan 24, 2025

@jesmrec Re (3): assuming this happened on an actual device, I'd think it's a typical memory limit crash & error message that's responsible for this. Please try again with the latest commits, which now include the latest mem-optimizations.

(3) is fixed 👍

(1) also fixed

(2) is a question, ftm we can keep 3 letters in the redacted names

only (4) is pending.

After that, i will do a second QA round to assure nothing was broken.

	- add markup action to list of disallowedActions. The markup action could not be modified to implement protection mechanisms - not even on the CALayer level - without interaction with the system-provided view breaking
- ConfidentialContentView:
	- break up ConfidentialContentView into Watermark (struct) and a CGContext extension to draw watermarks
	- refactor ConfidentialContentView to call the CGContext extension and use the Watermark struct
	- implement a CALayer subclass that also draws its content via the CGContext extension using the Watermark struct
	- extend the UIView extension to allow choosing between watermarking by subview or sublayer
	- context: these changes were made in an attempt to see if watermarking can be made work with EditDocumentViewController, which eventually failed (see above)
- SecureTextField: remove unused code path, eliminating the compiler warning
- EditDocumentViewController: change place where the watermark view is injected, allowing it to be sized correctly directly
- addresses finding (4) in #1430
@felix-schwarz
Copy link
Contributor

Re (4): the markup feature is delivered by the QLPreviewController system component, which is implemented as remote UI (=> kind of like screen sharing the contents from another process, and sending input events to that other process). This can be seen in the view debugger in the view tree:
Bildschirmfoto 2025-01-24 um 17 31 19

Unfortunately, all attempts to add the protection features around it eventually failed. I documented it with this code commend:

The markup action could not be modified to implement protection mechanisms -
not even on the CALayer level - without interaction with the system-provided
view breaking and becoming unusable in different ways. A possible reason for
this is that the markup feature is delivered by the OS as Remote UI (visible
as QLRemoteUIHostViewController in the view hierarchy) and that otherwise working
approaches to "passing through" events are not usable with these.

I therefore added the markup action to the list of actions to disallow if protection features are used. It simply doesn't currently seem technically possible to offer that action under the protection umbrella.

From my POV this PR is now ready for a final QA round @jesmrec.

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.

4 participants