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

do not enforce case insensitive uniqueness for piv_cac uuid or dn_signature #231

Merged
merged 2 commits into from
May 11, 2021

Conversation

mitchellhenke
Copy link
Contributor

The piv_cacs table has case-sensitive indices on dn_signature and uuid, but issues a query for a uniqueness check to for case insensitive equality in the form of SELECT ? AS one FROM ? WHERE LOWER(?.?) = LOWER($?) LIMIT $?. That query is not indexed, and leads to table scans that have been getting slower over time.

t.index ["dn_signature"], name: "index_piv_cacs_on_dn_signature", unique: true
t.index ["uuid"], name: "index_piv_cacs_on_uuid", unique: true

The initial implementation here attempted to have a case-insensitive check, but it was misconfigured by using:

# results in case-insensitive check
validates :uuid, presence: true, uniqueness: true, case_sensitive: false

This misconfiguration was fixed in the Rails 6.1 upgrade (#213), and we then started running the unindexed queries.

This change reverts it back to being case-sensitive, which should be safe since it previously operated this way. I believe this is also the correct behavior since dn_signature is Base64 encoded, where an a is not the same as an A. Our UUIDs are always generated as being lowercase with SecureRandom.uuid as well.

This PR also includes some dependency updates to patch GHSA-hjg4-8q5f-x6fm

Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM! great catch

@mitchellhenke mitchellhenke merged commit fc9bbb0 into main May 11, 2021
@mitchellhenke mitchellhenke deleted the mitchellhenke/use-piv-cac-indices branch May 11, 2021 19:09
mitchellhenke added a commit that referenced this pull request May 17, 2021
* Update hostdata version to support imdsv2 (#229)

* Use identity-hostdata v3.2.0

Co-authored-by: Zach Margolis <zachary.margolis@gsa.gov>

* Bump rexml from 3.2.4 to 3.2.5 (#230)

Bumps [rexml](https://github.com/ruby/rexml) from 3.2.4 to 3.2.5.
- [Release notes](https://github.com/ruby/rexml/releases)
- [Changelog](https://github.com/ruby/rexml/blob/master/NEWS.md)
- [Commits](ruby/rexml@v3.2.4...v3.2.5)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* do not enforce case insensitive uniqueness for piv_cac uuid or dn_signature (#231)

* do not enforce case insensitive uniqueness for piv_cac uuid or dn_signature

* update rails

Co-authored-by: Brian Crissup <crissupb@amazon.com>
Co-authored-by: Zach Margolis <zachary.margolis@gsa.gov>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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