Skip to content

Yubikey: Remove unused authorizedYubikeys#1428

Merged
brianmcgillion merged 2 commits intotiiuae:mainfrom
vunnyso:vs-removeKeys
Sep 23, 2025
Merged

Yubikey: Remove unused authorizedYubikeys#1428
brianmcgillion merged 2 commits intotiiuae:mainfrom
vunnyso:vs-removeKeys

Conversation

@vunnyso
Copy link
Copy Markdown
Collaborator

@vunnyso vunnyso commented Sep 23, 2025 •
edited
Loading

Description of Changes

  • Removed fixed u2fKeys from reference keys module and the authfile option from PAM U2F settings.
  • Now relies on externally provided configuration for U2F keys, improving modularity and flexibility.
  • Aims to prevent the hardcoding of Yubikey devices as Qemu parameters. Instead, Yubikey detection will be managed via vhotplug.

Type of Change

  • New Feature
  • Bug Fix
  • Improvement / Refactor

Related Issues / Tickets

Fixes: https://jira.tii.ae/browse/SSRCSP-7201

Checklist

  • Clear summary in PR description
  • Detailed and meaningful commit message(s)
  • Commits are logically organized and squashed if appropriate
  • Contribution guidelines followed
  • Ghaf documentation updated with the commit - https://tiiuae.github.io/ghaf/
  • Author has run make-checks and it passes
  • All automatic GitHub Action checks pass - see actions
  • Author has added reviewers and removed PR draft status

Testing Instructions

Applicable Targets

  • Orin AGX aarch64
  • Orin NX aarch64
  • Lenovo X1 x86_64
  • Dell Latitude x86_64
  • System 76 x86_64

Installation Method

  • Requires full re-installation
  • Can be updated with nixos-rebuild ... switch
  • Other:

Test Steps To Verify:

  1. Make sure it fixes bug mentioned in Tickets section.
  2. Yubikey enrolment should work fine with and without USB Hub connected.
  3. Test all Yubikey related test cases.

@vunnyso
Yubikey: Remove unused authorizedYubikeys
3fc3630 
- Removed fixed `u2fKeys` from reference keys module
  and the `authfile` option from PAM U2F settings.
- Now relies on externally provided configuration for U2F
  keys, improving modularity and flexibility.

Signed-off-by: Vunny Sodhi <vunny.sodhi@tii.ae>
@vunnyso
Copy link
Copy Markdown
Collaborator Author

vunnyso commented Sep 23, 2025

@leivos-unikie / @milva-unikie can you please confirm it fixes the issue mentioned?

@leivos-unikie leivos-unikie added the Needs Testing CI Team to pre-verify label Sep 23, 2025
@vunnyso vunnyso marked this pull request as draft September 23, 2025 09:40
@leivos-unikie
Copy link
Copy Markdown
Contributor

leivos-unikie commented Sep 23, 2025

The issue https://jira.tii.ae/browse/SSRCSP-7201 still reproduces.

Ghaf boots to GUI / user creation prompt when Yubikey is connected via USB hub but not when directly plugged into laptop USB port.

After booting with Yubikey plugged via USB hub I initialized the key and created a user. Then rebooted with the Yubikey plugged directly to laptop, and it didn't boot to GUI even after initializing the Yubikey.

@leivos-unikie leivos-unikie removed the Needs Testing CI Team to pre-verify label Sep 23, 2025
@vunnyso
usb: Remove Yubikey device entry from external devices
e22be71 
This approach aims to prevent the hardcoding of Yubikey device
as Qemu parameters. Instead, Yubikey detection will be managed
via vhotplug module.

Signed-off-by: Vunny Sodhi <vunny.sodhi@tii.ae>
@vunnyso vunnyso marked this pull request as ready for review September 23, 2025 11:22
@vunnyso vunnyso requested review from gngram and nesteroff September 23, 2025 11:25
@leivos-unikie
Copy link
Copy Markdown
Contributor

leivos-unikie commented Sep 23, 2025 •
edited
Loading

Tested on Lenovo-X1

Initializing Yubikey works now fine when plugged directly to laptop. Logging and unlocking with Yubikey and by password works. Unplugging Yubikey locks the screen. Yubikey works both when directly plugged and via USB hub.

@leivos-unikie leivos-unikie added the Tested on Lenovo X1 Carbon This PR has been tested on Lenovo X1 Carbon label Sep 23, 2025
kajusnau
kajusnau reviewed Sep 23, 2025
View reviewed changes
kajusnau
kajusnau approved these changes Sep 23, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@kajusnau kajusnau left a comment

Choose a reason for hiding this comment

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

Adjust the typo if necessary, otherwise looks good

@brianmcgillion brianmcgillion merged commit 60be4c4 into tiiuae:main Sep 23, 2025
27 of 28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@brianmcgillion brianmcgillion brianmcgillion approved these changes

@kajusnau kajusnau kajusnau approved these changes

@gngram gngram Awaiting requested review from gngram

+1 more reviewer

@nesteroff nesteroff nesteroff approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Tested on Lenovo X1 Carbon This PR has been tested on Lenovo X1 Carbon

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@vunnyso @leivos-unikie @brianmcgillion @nesteroff @kajusnau