`Configuration`: added (`internal` for now) API to load public key by NachoSoto · Pull Request #2215 · RevenueCat/purchases-ios

NachoSoto · 2023-01-14T01:25:35Z

Changes:

Created Signing to encapsulate new signing logic and types
Added abstractions like Signing.Certificate and Signing.PublicKey over Security C types
Created Configuration.EntitlementVerificationLevel and Signing.ResponseVerificationLevel (the logic for the name difference is that publicly this is used to validate entitlements, but the internal implementation works for any response).
Created Signing.loadPublicKey
Added tests for loading public key embedded in SDK
Added internal (for now) Configuration.with(entitlementVerificationLevel:)
Added SystemInfo.responseVerificationLevel with the loaded key, if enabled

For a future PR:

Make the Configuration.with(entitlementVerificationLevel:) public
Make Configuration.EntitlementVerificationLevel public
Add those 2 to APITesters
Improve documentation by adding a link to the docs
Verify certificate issuer in test

aboedo · 2023-01-27T18:53:18Z

🤔 my thinking is that it would be useful for this to exist for testing purposes, but a customer would never need to override, right? What alternatives could we explore that don't modify the public API?
Perhaps we could have a file name where if the file exists, it overrides the key, and we use that for testing? Of course, that would mean that if the bundle is modified to include such file, it would override the key, which would make it a potential attack. Maybe we could have that only work in debug / sandbox?

NachoSoto · 2023-02-02T19:07:30Z

Yeah when I did this I didn't have full context.
Should we make that Configuration method internal? (and DEBUG only?)

NachoSoto · 2023-02-06T22:23:12Z

This should be ready now, without any new public API.

NachoSoto · 2023-02-07T01:00:18Z

Hold on, I think we need to change the key loading code since it appears we're using a different type of key.

Fixes [CSDK-632].

tonidero

A couple small things but looking good!

tonidero · 2023-02-08T10:28:21Z


+    var publicKey: Signing.PublicKey? {
+        return self.systemInfo.responseVerificationLevel.publicKey
+    }


I guess this will be used in future tests?

Oh I moved these tests somewhere else, but let me add another one for Purchases.Configuration that uses this too.
I moved them out on a refactor and also when I made this internal, but they're still useful for when we make it public.

tonidero · 2023-02-08T10:28:59Z

 // everything in those classes will still be called by XCTest, and it will cause errors.
 enum AvailabilityChecks {

+    static func iOS12APIAvailableOrSkipTest() throws {


Also unused, but maybe you plan to use it for future tests?

Oh not anymore, since this feature is iOS 13 only, I'll remove it!

tonidero · 2023-02-08T10:31:53Z

         operationDispatcher: OperationDispatcher = .default,
         bundle: Bundle = .main,
         storeKit2Setting: StoreKit2Setting = .default,
+         responseVerificationLevel: Signing.ResponseVerificationLevel = .disabled,


Should this use .default?

Yes!! Great catch.

tonidero · 2023-02-08T10:32:22Z

        private(set) var networkTimeout = Configuration.networkTimeoutDefault
        private(set) var storeKit1Timeout = Configuration.storeKitRequestTimeoutDefault
        private(set) var platformInfo: Purchases.PlatformInfo?
+        private(set) var responseVerificationLevel: Signing.ResponseVerificationLevel = .disabled


Same here, should this be .default?

Another good catch 👍🏻

@vegaro

**This is an automatic release.** ### Dependency Updates * Bump fastlane-plugin-revenuecat_internal from `738f255` to `9255366` (#2264) via dependabot[bot] (@dependabot[bot]) * Update `Gemfile.lock` (#2254) via Cesar de la Vega (@vegaro) ### Other Changes * `HTTPClient`: added support for sending `X-Nonce` (#2214) via NachoSoto (@NachoSoto) * `Configuration`: added (`internal` for now) API to load public key (#2215) via NachoSoto (@NachoSoto) * Replaced `Any` uses for workaround with `Box` (#2250) via NachoSoto (@NachoSoto) * `HTTPClientTests`: fixed failing test with missing assertions (#2262) via NachoSoto (@NachoSoto) * `HTTPClientTests`: refactored tests to use `waitUntil` (#2257) via NachoSoto (@NachoSoto) * PurchaseTester: Add Receipt Inspector UI (#2249) via Andy Boedo (@aboedo) * Adds dependabot (#2259) via Cesar de la Vega (@vegaro) * `StoreKit1WrapperTests`: avoid using `Bool.random` to fix flaky code coverage (#2258) via NachoSoto (@NachoSoto) * `IntroEligibilityCalculator`: changed logic to handle products with no subscription group (#2247) via NachoSoto (@NachoSoto)

…2267) ⚠️ 🎉 This also changes integration tests to use `EntitlementVerificationLevel.enforced` so that integration tests fail if signatures are invalid. #### Depends on: - https://github.com/RevenueCat/khepri/pull/5191 - https://github.com/RevenueCat/khepri/pull/5204 - #2214 - #2215 - #2216 - #2272 _Marking this as `feat`ure because it contains a new error in `PurchasesDiagnostics.Error`_

NachoSoto added the pr:feat A new feature label Jan 14, 2023

NachoSoto requested a review from a team January 14, 2023 01:25

NachoSoto force-pushed the configuration-public-key branch 3 times, most recently from 8f1bc1a to 51a2635 Compare January 17, 2023 22:04

NachoSoto added the blocked label Jan 25, 2023

NachoSoto force-pushed the configuration-public-key branch from 51a2635 to a4b1e3f Compare February 6, 2023 22:17

NachoSoto changed the title ~~Configuration: added ability to set a public key~~ Configuration: added (internal for now) API to load public key Feb 6, 2023

NachoSoto added refactor and removed blocked pr:feat A new feature labels Feb 6, 2023

NachoSoto force-pushed the configuration-public-key branch from a4b1e3f to 2ea4a17 Compare February 6, 2023 22:23

NachoSoto added the blocked label Feb 7, 2023

NachoSoto mentioned this pull request Feb 7, 2023

PurchasesDiagnostics: added step to verify signature verification #2267

Merged

NachoSoto added 5 commits February 7, 2023 13:02

Configuration: added ability to set a public key

3b024c3

Fixes [CSDK-632].

Load SecKey instead.

262c6bc

Embedded public key and finished API

44f2498

Using CryptoKit.Curve25519.Signing.PublicKey

b99d184

Renamed to ResponseVerificationLevel

7c520aa

NachoSoto force-pushed the configuration-public-key branch from 213b677 to 7c520aa Compare February 7, 2023 21:05

NachoSoto added refactor and removed refactor blocked labels Feb 7, 2023

NachoSoto mentioned this pull request Feb 7, 2023

[WIP] HTTPClient: added signature validation #2268

Merged

tonidero approved these changes Feb 8, 2023

View reviewed changes

Removed unused method

1066190

NachoSoto added 4 commits February 8, 2023 07:36

Use default

8d4f4c3

Removed unused import

1772c0d

Added fix-me

399b97c

Added configuration tests

4431f76

NachoSoto force-pushed the configuration-public-key branch from 415c083 to 4431f76 Compare February 8, 2023 16:42

Fixed integration tests

63823da

NachoSoto enabled auto-merge (squash) February 8, 2023 16:43

NachoSoto merged commit 76dbf00 into main Feb 8, 2023

NachoSoto deleted the configuration-public-key branch February 8, 2023 16:53

RCGitBot mentioned this pull request Feb 8, 2023

[AUTOMATIC] Release/4.17.5 #2270

Merged

NachoSoto mentioned this pull request Feb 8, 2023

HTTPClient: added signature validation and introduced ErrorCode.signatureVerificationFailed #2272

Merged

Conversation

NachoSoto commented Jan 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes:

For a future PR:

Uh oh!

aboedo commented Jan 27, 2023

Uh oh!

NachoSoto commented Feb 2, 2023

Uh oh!

NachoSoto commented Feb 6, 2023

Uh oh!

NachoSoto commented Feb 7, 2023

Uh oh!

tonidero left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

NachoSoto commented Jan 14, 2023 •

edited

Loading