Skip to content

GitLab

  • Menu
Projects Groups Snippets
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
    • Contribute to GitLab
  • Sign in / Register
  • BIND BIND
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
  • Issues 528
    • Issues 528
    • List
    • Boards
    • Service Desk
    • Milestones
  • Merge requests 96
    • Merge requests 96
  • CI/CD
    • CI/CD
    • Pipelines
    • Jobs
    • Schedules
  • Deployments
    • Deployments
    • Environments
    • Releases
  • Packages & Registries
    • Packages & Registries
    • Container Registry
  • Monitor
    • Monitor
    • Incidents
  • Analytics
    • Analytics
    • Value stream
    • CI/CD
    • Repository
  • Wiki
    • Wiki
  • Snippets
    • Snippets
  • Activity
  • Graph
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
Collapse sidebar
  • ISC Open Source Projects
  • BINDBIND
  • Issues
  • #3406
Closed
Open
Created Jun 14, 2022 by David Benjamin@davidbenContributor

[PATCH] Clean up OpenSSL usage a bit

I tried to send this as a merge request per the instructions, but I couldn't figure out how to do that. (Trying to fork the repository on GitLab complained "You have reached your project limit" error, and trying to push to a random branch on the main repo. Hopefully these diffs are okay.)

Here are a pair of patches to clean up OpenSSL usage a bit:

0001-Simplify-BN_GENCB-handling.patch

Simplify BN_GENCB handling.

When callback was NULL, bind9 would use BN_GENCB_set_old to set a NULL callback because OpenSSL happened to allow a NULL "old" callback, but not a NULL "new" callback. Instead, the way top turn off the callback is to pass a NULL BN_GENCB itself.

Switch to doing that. Along the way, add a missing NULL check in opensslrsa_link.c, in case BN_GENCB_new failed.

(I didn't do this in the patch, but this could probably be further simplified if the BN_GENCB_new wrapper returned a zeroed malloc(sizeof(BN_GENCB)), rather than rely on a special _cb variable.)

0002-Remove-DH_clear_flags-call.patch This one took a bit of archaelogy:

Remove DH_clear_flags call.

These calls have not been needed since OpenSSL 0.9.7h.

This dates to commit 704d6eea, "Work around non-reentrancy in openssl by disabling precomputation in keys". This was in the bundled OpenSSL 0.9.3a era and made two changes. First, it registered a locking callback because, in those days, OpenSSL needed a callback to support locks. Second, it set flags to disable various bits of cached state on DH, DSA, and RSA objects.

Looking back in OpenSSL 0.9.3a, that cached state was not protected by a lock: https://github.com/openssl/openssl/blob/OpenSSL_0_9_3a/crypto/rsa/rsa_eay.c#L137-L142

However, this was fixed in OpenSSL 0.9.7h: https://github.com/openssl/openssl/commit/6ec8e63af6c1835a8b222350dbabf7bb2ace094f

The other flags (DSA and RSA) have since fallen away, DSA with the removal of DSA altogether (3994b1f9) and RSA with 3a8d4a31, "openssl 0.9.6a and higher don't have the RSA locking bug [...] other algorithms still don't do locking when performing precomputation [...]".

That seems to be referring to this OpenSSL change, which indeed fixed it for RSA but not others: https://github.com/openssl/openssl/commit/bb617a9646d95d0454edda995518f370172390e9

The 0.9.7h change above fixed it across the board, but there was never a similar update to the workaround for DSA and DH. With such OpenSSL versions long since out of support, the last remains of this workaround can finally be removed.

Assignee
Assign to
Time tracking