Skip to content

Fix swapped bind values when updating a stored tls-auth key - #737

Open
vidhi559 wants to merge 1 commit into
chris2511:mainfrom
vidhi559:fix-takey-update-bind
Open

vidhi559 wants to merge 1 commit into
chris2511:mainfrom
vidhi559:fix-takey-update-bind

Conversation

@vidhi559

Copy link
Copy Markdown

importTaKey() shares one pair of bindValue() calls between its INSERT and its UPDATE, but the two take their parameters in opposite order: the INSERT is (item, value) while the UPDATE is
"SET value = ? WHERE item = ?".

Replacing an existing tls-auth key therefore wrote the item id into the value column and looked the row up by matching the integer item column against the base64 key data. No row matches on SQLite, and PostgreSQL rejects the comparison outright, so the stored key was never replaced and the user was told the import had failed.

Bind each branch separately.

importTaKey() shares one pair of bindValue() calls between its INSERT
and its UPDATE, but the two take their parameters in opposite order:
the INSERT is (item, value) while the UPDATE is
"SET value = ? WHERE item = ?".

Replacing an existing tls-auth key therefore wrote the item id into the
value column and looked the row up by matching the integer item column
against the base64 key data. No row matches on SQLite, and PostgreSQL
rejects the comparison outright, so the stored key was never replaced
and the user was told the import had failed.

Bind each branch separately.
@vidhi559

Copy link
Copy Markdown
Author

The macOS job failure looks unrelated to this change.

My commit only reorders two bindValue() calls in lib/pki_x509.cpp. That file
compiles cleanly in the log and is archived into libxcalib.a; the job fails
afterwards at link time with "ld: framework 'AGL' not found", for both xca and
testxca. AGL is not referenced anywhere in the project - it comes from Qt's own
CMake config, and Apple has removed AGL from the current macOS SDK on the
runner image.

The other jobs were cancelled when this one failed, so they have no result yet.
Happy to rebase or re-run once the runner or Qt version is sorted, but I don't
think there is anything to change in this PR.

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