Skip to content

Signing: extract and verify intermediate key#2715

Merged
NachoSoto merged 3 commits into
mainfrom
nacho/verify-intermediate-key
Jun 30, 2023
Merged

Signing: extract and verify intermediate key#2715
NachoSoto merged 3 commits into
mainfrom
nacho/verify-intermediate-key

Conversation

@NachoSoto

Copy link
Copy Markdown
Contributor

Last step of the new Signature format. Follow up to #2679 and #2698.

This reverts the public key change in #2679, since that was the intermediate key.
This now extracts the new intermediate public key from the signature, and verifies it using the public key.

@NachoSoto NachoSoto requested review from a team and bisho June 26, 2023 22:28
data: intermediatePublicKey))

do {
return try Self.createPublicKey(with: intermediatePublicKey)

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.

We considered caching this, but I just ran a benchmark on my device and the whole process (creating this key and verifying the original signature) takes 0.17 seconds.

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.

Sorry I meant 0.17ms!

@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.

Love this! Going to try to get this done in Android ASAP

Comment thread Sources/Security/Signing.swift Outdated

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.

So if the intermediate key is expired, we assume it's an attacker correct? As long as the backend makes sure this doesn't happen, this should be fine 👍

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.

Yup /cc @bisho

@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.

Love this! Going to try to get this done in Android ASAP

@NachoSoto NachoSoto force-pushed the nacho/changed-etag-signatures-2 branch from e9e2d8e to 4d7e631 Compare June 27, 2023 14:31
@NachoSoto NachoSoto force-pushed the nacho/verify-intermediate-key branch from 0d1bd54 to 7aa47ba Compare June 27, 2023 14:32
@NachoSoto NachoSoto force-pushed the nacho/changed-etag-signatures-2 branch 3 times, most recently from b4e4adc to dd41618 Compare June 29, 2023 16:12
@NachoSoto NachoSoto force-pushed the nacho/verify-intermediate-key branch from 7aa47ba to 4dc2d8a Compare June 29, 2023 16:28
@NachoSoto NachoSoto force-pushed the nacho/changed-etag-signatures-2 branch 7 times, most recently from fb1b4c9 to 7181dba Compare June 29, 2023 19:02
@NachoSoto NachoSoto force-pushed the nacho/verify-intermediate-key branch 2 times, most recently from 54b4e49 to 69a2415 Compare June 29, 2023 21:14
@NachoSoto NachoSoto requested a review from tonidero June 29, 2023 21:14

@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!

@tonidero

Copy link
Copy Markdown
Contributor

Some tests seem to be failing though

@NachoSoto

Copy link
Copy Markdown
Contributor Author

Some tests seem to be failing though

Yeah that's testVerifyKnownSignatureWithNoNonceAndNoEtag because I need a signature signed with the real private root key, so I'm waiting for offerings to have static signatures.

@NachoSoto NachoSoto force-pushed the nacho/changed-etag-signatures-2 branch from ba53303 to 1876819 Compare June 30, 2023 19:10
Base automatically changed from nacho/changed-etag-signatures-2 to main June 30, 2023 19:24
@NachoSoto NachoSoto force-pushed the nacho/verify-intermediate-key branch from 69a2415 to fcbc752 Compare June 30, 2023 19:29
@NachoSoto NachoSoto force-pushed the nacho/verify-intermediate-key branch from fcbc752 to bef3bef Compare June 30, 2023 19:30
@NachoSoto NachoSoto enabled auto-merge (squash) June 30, 2023 19:31
@NachoSoto NachoSoto merged commit e0ffea5 into main Jun 30, 2023
@NachoSoto NachoSoto deleted the nacho/verify-intermediate-key branch June 30, 2023 19:44
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