Skip to content

Add 'Serial' struct method returning serial number directly from Yubi…#463

Merged
joshdrake merged 2 commits intomasterfrom
josh/yubikey-piv-serial
Mar 27, 2024
Merged

Add 'Serial' struct method returning serial number directly from Yubi…#463
joshdrake merged 2 commits intomasterfrom
josh/yubikey-piv-serial

Conversation

@joshdrake
Copy link
Copy Markdown
Contributor

…Key.

This change provides a public struct method to obtain the serial number of a Yubikey directly from the the device; in the piv package this employs the native PIV instruction with a fallback for older models.

This enables client code using this library to always get the device serial number, even if the attestation key/certificate has been evicted/imported over.

💔Thank you!

@CLAassistant
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Comment thread kms/yubikey/yubikey.go Outdated
Comment on lines +363 to +369
func (k *YubiKey) Serial() string {
if serial, err := k.yk.Serial(); err == nil {
return strconv.FormatUint(uint64(serial), 10)
}

return ""
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You should return an error, wrapping it with something like:

fmt.Errorf("error getting Yubikey's serial number: %w", err)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@joshdrake joshdrake requested a review from maraino March 25, 2024 21:53
@joshdrake joshdrake merged commit cfab90e into master Mar 27, 2024
@joshdrake joshdrake deleted the josh/yubikey-piv-serial branch March 27, 2024 01:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants