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

Use SecKeyGetBlockSize instead of kSecAttrKeySizeInBits #198

Merged
merged 1 commit into from
Aug 17, 2023

Conversation

dnadoba
Copy link
Member

@dnadoba dnadoba commented Aug 17, 2023

Checklist

  • I've run tests to see all new and existing tests pass
  • I've followed the code style of the rest of the project
  • I've read the Contribution Guidelines
  • I've updated the documentation if necessary

If you've made changes to gyb files

  • I've run .script/generate_boilerplate_files_with_gyb and included updated generated files in a commit of this pull request

Motivation:

Security.framework has change to return the exact bit count of the key size instead of the block size in bytes in macOS 14 beta and aligned releases. However, BoringSSL and the previous implementation relyed on that.
SecKeyCopyAttributes is also rather expensive to call and keySizeInBits is accessed immediately (twice) after initialisation.

Modifications:

call SecKeyGetBlockSize(_:)`` instead of looking up the kSecAttrKeySizeInBits` in the attributes dictionary.

Result:

test pass again and allocations are reduced.

@dnadoba dnadoba added the patch-version-bump-only For PRs that when merged will only cause a bump of the patch version, ie. 1.0.x -> 1.0.(x+1) label Aug 17, 2023
@Lukasa Lukasa enabled auto-merge (squash) August 17, 2023 13:22
@Lukasa
Copy link
Collaborator

Lukasa commented Aug 17, 2023

@swift-server-bot add to allowlist

@Lukasa Lukasa merged commit 953b34a into apple:main Aug 17, 2023
6 of 8 checks passed
@dnadoba dnadoba deleted the dn-use-get-block-size branch August 17, 2023 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch-version-bump-only For PRs that when merged will only cause a bump of the patch version, ie. 1.0.x -> 1.0.(x+1)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants