Skip to content

Trusted Entitlements: produce verification failures for static endpoints with no signature#2752

Merged
NachoSoto merged 3 commits into
mainfrom
nacho/detect-missing-signatures
Jul 6, 2023
Merged

Trusted Entitlements: produce verification failures for static endpoints with no signature#2752
NachoSoto merged 3 commits into
mainfrom
nacho/detect-missing-signatures

Conversation

@NachoSoto

Copy link
Copy Markdown
Contributor

@NachoSoto NachoSoto force-pushed the nacho/detect-missing-signatures branch from b110783 to 163337e Compare July 6, 2023 00:33
}

let needsNonce = self.path.needsNonceForSigning
return !needsNonce || (needsNonce && self.nonce != nil)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hmm I wonder, is this part needed? In Android I'm only checking that supportsSignatureVerification is true and that entitlement verification is enabled. In that situation, we should always require a signature I think? Whether the path requires a nonce or not, shouldn't affect whether we require a signature I think.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oh the issue was some of the failing tests were wrong. I thought I needed this check but I don't! Fixed.


private extension HTTPRequest {

var requiresSignature: Bool {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should this also check if entitlement verification is enabled?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's done by the calling method. But I removed this property so it's more clear now.

@NachoSoto NachoSoto requested a review from tonidero July 6, 2023 15:43
case .signature_failed_verification:
return "Signature failed verification"

case .signature_passed_verification:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I added this too like Android.

@tonidero tonidero left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good!

@NachoSoto NachoSoto enabled auto-merge (squash) July 6, 2023 15:50
@NachoSoto NachoSoto merged commit bd3ac5f into main Jul 6, 2023
@NachoSoto NachoSoto deleted the nacho/detect-missing-signatures branch July 6, 2023 17:08
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.

2 participants