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

Search for default pinned certificates in the main bundle #3752

Merged

Conversation

@0xced
Copy link
Collaborator

0xced commented Oct 14, 2016

Using [NSBundle bundleForClass:[self class]] is dangerous because the code will behave differently depending on how AFNetworking is integrated.

  • If it is integrated as a static library (for example using CocoaPods), certificates will be searched in the main bundle.
  • If it is integrated as a framework (for example using Carthage), certificates will be searched in the AFNetworking framework.

Even though this behavior is documented, this is dangerous. Using the main bundle makes the behavior deterministic.

Fixes #3575

Using `[NSBundle bundleForClass:[self class]]` is dangerous because the code will behave differently depending on how AFNetworking is integrated.

* If it is integrated as a static library (for example using CocoaPods), certificates will be searched in the main bundle.

* If it is integrated as a framework (for example using Carthage), certificates will be searched in the AFNetworking framework.

Even though this behavior is documented, this is dangerous. Using the main bundle makes the behavior deterministic.

Fixes #3575
@0xced 0xced added the security label Oct 14, 2016
@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Oct 14, 2016

Current coverage is 87.42% (diff: 100%)

Merging #3752 into master will decrease coverage by 0.01%

@@             master      #3752   diff @@
==========================================
  Files            45         45          
  Lines          6227       6220     -7   
  Methods        1104       1102     -2   
  Messages          0          0          
  Branches        416        416          
==========================================
- Hits           5445       5438     -7   
  Misses          779        779          
  Partials          3          3          

Powered by Codecov. Last update 4f3c694...4298369

@jshier jshier added the needs review label Oct 21, 2019
@jshier

This comment has been minimized.

Copy link
Contributor

jshier commented Jan 5, 2020

Old, but still valid, and this change matches Alamofire's behavior.

@jshier jshier merged commit 2da270b into AFNetworking:master Jan 5, 2020
3 checks passed
3 checks passed
codecov/patch 100% of diff hit (target 87.44%)
Details
codecov/project Absolute coverage decreased by -0.01% but relative coverage increased by +12.55% compared to 4f3c694
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@0xced 0xced deleted the 0xced:default-pinned-certificates-main-bundle branch Jan 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.