-
Notifications
You must be signed in to change notification settings - Fork 65
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
adding Shared access to multiple processes #100
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This probably won't work. Even if you have a shared connection, as soon as you open a transaction you block the card until the transaction is closed:
Lines 162 to 169 in 4b80d66
tx, err := h.Begin() | |
if err != nil { | |
return nil, fmt.Errorf("beginning smart card transaction: %w", err) | |
} | |
if err := ykSelectApplication(tx, aidPIV[:]); err != nil { | |
tx.Close() | |
return nil, fmt.Errorf("selecting piv applet: %w", err) | |
} |
We probably need to replace accesses to the YubiKey.tx field with a function that either reuses the one transaction in exclusive mode, or creates then closes the connection after.
func (yk *YubiKey) withTx(fn func(tx * scTx) error) error {
// ...
}
Then functions go from
// Serial returns the YubiKey's serial number.
func (yk *YubiKey) Serial() (uint32, error) {
return ykSerial(yk.tx, yk.version)
}
To
// Serial returns the YubiKey's serial number.
func (yk *YubiKey) Serial() (uint32, error) {
var serial uint32
err := yk.withTx(func(tx *scTx) error {
var err error
serial, err = ykSerial(tx, yk.version)
return err
})
return serial, err
}
@rajnikant12345 would you like me to take over this one? |
Yes that would be great, because this issue is moved to November on my side. Thanks a lot |
@ericchiang , sorry for late checkin. I was busy with other related stories. |
@ericchiang is this already checked in from your side? Thanks |
Thanks for the follow up! Sorry for the delay, but please ping if you need me to run the CI |
Thanks @ericchiang for reviewing |
@ericchiang anything needed from my side on this? |
I see there are now some conflicts on the branch - if conflicts were resolved, could this be tested again and potentially merged? |
Related: #47