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

NetworkExtension uses NSPrivacyAccessedAPICategoryFileTimestamp causing App Store hurdle #4377

Closed
jamilbk opened this issue Mar 28, 2024 · 6 comments · Fixed by #4378, #4390, #4395 or #4400
Closed

NetworkExtension uses NSPrivacyAccessedAPICategoryFileTimestamp causing App Store hurdle #4377

jamilbk opened this issue Mar 28, 2024 · 6 comments · Fixed by #4378, #4390, #4395 or #4400
Assignees
@jamilbk
Labels
area/apple_client Issues related to the Apple client area/connlib Firezone's core connectivity library business_value/high Required by > 50% of our customer base kind/refactor Code refactoring
Milestone
1.0 GA

Comments

@jamilbk
Copy link
Member

jamilbk commented Mar 28, 2024

Hello,

We noticed one or more issues with a recent submission for TestFlight review for the following app:

Firezone
Version 1.0.0
Build 1711611179
Although submission for TestFlight review was successful, you may want to correct the following issues in your next submission for TestFlight review. Once you've corrected the issues, upload a new binary to App Store Connect.

ITMS-91053: Missing API declaration - Your app’s code in the “Firezone” file references one or more APIs that require reasons, including the following API categories: NSPrivacyAccessedAPICategoryFileTimestamp. While no action is required at this time, starting May 1, 2024, when you upload a new app or app update, you must include a NSPrivacyAccessedAPITypes array in your app’s privacy manifest to provide approved reasons for these APIs used by your app’s code. For more details about this policy, including a list of required reason APIs and approved reasons for usage, visit: https://developer.apple.com/documentation/bundleresources/privacy_manifest_files/describing_use_of_required_reason_api.

ITMS-91053: Missing API declaration - Your app’s code in the “PlugIns/FirezoneNetworkExtensioniOS.appex/FirezoneNetworkExtensioniOS” file references one or more APIs that require reasons, including the following API categories: NSPrivacyAccessedAPICategoryFileTimestamp. While no action is required at this time, starting May 1, 2024, when you upload a new app or app update, you must include a NSPrivacyAccessedAPITypes array in your app’s privacy manifest to provide approved reasons for these APIs used by your app’s code. For more details about this policy, including a list of required reason APIs and approved reasons for usage, visit: https://developer.apple.com/documentation/bundleresources/privacy_manifest_files/describing_use_of_required_reason_api.

Apple Developer Relations
@jamilbk jamilbk added the area/apple_client Issues related to the Apple client label Mar 28, 2024
@jamilbk jamilbk added this to the 1.0 GA milestone Mar 28, 2024
@jamilbk jamilbk self-assigned this Mar 28, 2024
@jamilbk jamilbk mentioned this issue Mar 28, 2024
Merged
github-merge-queue bot pushed a commit that referenced this issue Mar 28, 2024
@jamilbk
refactor(apple): Use AppleArchive not Zipfiles for log export (#4378)
5c2863a 
We were using an external library to compress the log folder on Apple,
when Apple has native APIs for such things. The external library also
doesn't have a good security track record, so this work was prioritized
for GA.

Default compression level I'm seeing is about 30:1, whereas the previous
Zip compression level seemed to be disabled.

Fixes #4377 
Fixes #4362
@jamilbk
Copy link
Member Author

jamilbk commented Mar 28, 2024

Looks like something inside the NetworkExtension uses the API as well.

Hello,

We noticed one or more issues with a recent submission for TestFlight review for the following app:

Firezone
Version 1.0.0
Build 1711660079
Although submission for TestFlight review was successful, you may want to correct the following issues in your next submission for TestFlight review. Once you've corrected the issues, upload a new binary to App Store Connect.

ITMS-91053: Missing API declaration - Your app’s code in the “PlugIns/FirezoneNetworkExtensioniOS.appex/FirezoneNetworkExtensioniOS” file references one or more APIs that require reasons, including the following API categories: NSPrivacyAccessedAPICategoryFileTimestamp. While no action is required at this time, starting May 1, 2024, when you upload a new app or app update, you must include a NSPrivacyAccessedAPITypes array in your app’s privacy manifest to provide approved reasons for these APIs used by your app’s code. For more details about this policy, including a list of required reason APIs and approved reasons for usage, visit: https://developer.apple.com/documentation/bundleresources/privacy_manifest_files/describing_use_of_required_reason_api.

Apple Developer Relations

@jamilbk jamilbk reopened this Mar 28, 2024
@jamilbk jamilbk changed the title ZIPFoundation uses NSPrivacyAccessedAPICategoryFileTimestamp causing App Store Privacy issue NetworkExtension uses NSPrivacyAccessedAPICategoryFileTimestamp causing App Store Privacy issue Mar 28, 2024
@jamilbk
Copy link
Member Author

jamilbk commented Mar 28, 2024

It's probably coming from tracing_appender: https://github.com/tokio-rs/tracing/blob/master/tracing-appender/src/rolling.rs#L605

This was referenced Mar 28, 2024
Open
Merged
github-merge-queue bot pushed a commit that referenced this issue Mar 29, 2024
@jamilbk
fix(connlib): Don't roll log files (#4390)
1cfa803 
Fixes #4377 
Closes #3910 

If we decide to implement diagnostic log collection in the future it
will be opt-in and use something like Sentry.
@jamilbk
Copy link
Member Author

jamilbk commented Mar 29, 2024 •
edited

Unfortunately this is still an issue even without the log roller. Just got the email from Apple. 😢

Perhaps there are other places where tracing_appender is reading file timestamp metadata.

Refs tokio-rs/tracing#2918

@jamilbk jamilbk reopened this Mar 29, 2024
@jamilbk jamilbk added business_value/high Required by > 50% of our customer base kind/refactor Code refactoring area/connlib Firezone's core connectivity library labels Mar 29, 2024
@jamilbk jamilbk removed this from the 1.0 GA milestone Mar 29, 2024
@jamilbk jamilbk changed the title NetworkExtension uses NSPrivacyAccessedAPICategoryFileTimestamp causing App Store Privacy issue NetworkExtension uses NSPrivacyAccessedAPICategoryFileTimestamp causing App Store hurdle Mar 29, 2024
This was referenced Mar 29, 2024
Closed
Merged
@jamilbk jamilbk added this to the 1.0 GA milestone Mar 29, 2024
github-merge-queue bot pushed a commit that referenced this issue Mar 29, 2024
@jamilbk @ReactorScram
chore(connlib): Remove atomicwrites and tokio::fs from apple compile …
c30138b 
…path (#4395)

Fixes #4377 


Manually verified by running `nm` on the resulting binaries. I'll open
another PR to handle #4393

---------

Signed-off-by: Jamil <jamilbk@users.noreply.github.com>
Co-authored-by: Reactor Scram <ReactorScram@users.noreply.github.com>
@jamilbk jamilbk reopened this Mar 30, 2024
@jamilbk
Copy link
Member Author

jamilbk commented Mar 30, 2024

Just received the error again. The saga continues.

@jamilbk
Copy link
Member Author

jamilbk commented Mar 30, 2024

I think I found the last place it's being used:

https://github.com/tokio-rs/tokio/blob/master/tokio/src/net/unix/pipe.rs#L1376

That means that codepath is likely being hit. I'll see if I can fork and patch around it to test the theory.

@jamilbk
Copy link
Member Author

jamilbk commented Mar 30, 2024 •
edited

I've confirmed that this change successfully removes the last remaining _fstat from the debug target.

HOWEVER, _stat and _fstat are still present in the --release target.

Very strange. --release is actually pulling in more syscalls than the debug target. My current theory is that rustc is pulling in those from the System framework during release builds with -lSystem.

If I run nm on the resulting network extension binary, we can see that's where they're from. This would make sense, System contains the libc headers presumably.

(undefined) external _stat (from libSystem)

(undefined) external _fstat (from libSystem)

This was referenced Mar 30, 2024
Merged
Merged
jamilbk added a commit that referenced this issue Mar 30, 2024
@jamilbk
chore(connlib): Add PrivacyManifest (#4400)
97d4d77 
It seems that even after getting rid of all of our direct dependencies
on `fstat`, rust compiles our library against `libSystem` anyway (libc)
and so these calls are impossible to get rid of.

Instead, we declare a privacy manifest.

fixes #4377
This was referenced Mar 30, 2024
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jamilbk jamilbk

Labels
area/apple_client Issues related to the Apple client area/connlib Firezone's core connectivity library business_value/high Required by > 50% of our customer base kind/refactor Code refactoring
Projects
None yet
Milestone
1.0 GA
1 participant
@jamilbk