search

CSIS / EnrollmentStation

Enrollment Station for enrolling Yubico smart cards in a Windows PKI
Other
46 stars 19 forks source link

EnrollmentStation crashes when more than one card reader is present #20

Closed tk4100 closed 7 years ago

tk4100 commented 7 years ago

Windows Server 2012 R2 Standard x64, via RDP.

Application: EnrollmentStation.exe Framework Version: v4.0.30319 Description: The process was terminated due to an unhandled exception. Exception Info: System.AccessViolationException Stack: at EnrollmentStation.Code.YubikeyNeoManager.YkNeoManagerDiscover(IntPtr) at EnrollmentStation.Code.YubikeyNeoManager.RefreshDevice() at EnrollmentStation.Code.YubikeyDetector.BackgroundWork() at System.Threading.Tasks.Task.InnerInvoke() at System.Threading.Tasks.Task.Execute() at System.Threading.Tasks.Task.ExecutionContextCallback(System.Object) at System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object, Boolean) at System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object, Boolean) at System.Threading.Tasks.Task.ExecuteWithThreadLocal(System.Threading.Tasks.Task ByRef) at System.Threading.Tasks.Task.ExecuteEntry(Boolean) at System.Threading.Tasks.Task.System.Threading.IThreadPoolWorkItem.ExecuteWorkItem() at System.Threading.ThreadPoolWorkQueue.Dispatch() at System.Threading._ThreadPoolWaitCallback.PerformWaitCallback()

tk4100 commented 7 years ago

I've got a non-production setup using this program right now, if there is any data I can provide that would help.

mike-csis commented 7 years ago

Yea, we've had issues with that. The problem also presents itself when you want to have a Virtual Smart Card for the Enrollment Agent running.

The issue seems to be that we have trouble reverse engineering the Yubico library in terms of being able to select the specific reader we want (basically, all readers are named the same in Yubicos case). Before we can do that, I don't see us being able to support this case.

tk4100 commented 7 years ago

There isn't a way to select from a list of readers to choose from (even if they all appear identical) instead of crashing? Or maybe even just throwing an error saying multiple readers are not supported.

I've forwarded the above to Yubico support, but I'm not sure how much effect that'll have.

mike-csis commented 7 years ago

We might be able show a message instead of crashing. I've not done this so far since the problem is very uncommon right now, and that we don't if there are cases out there in which you have two readers present but it still works.

As for selection, it's not index-based, but name-based. We select by name, and since they're identical, we have no way of knowing what is chosen.. That being said, I've never actually been able to select anything, so I'm pretty sure my reversing of the official client is incorrect or incomplete.

mike-csis commented 7 years ago

I gave it another go and was able to select a device (wow).. Apparently, they do use different names, only the part changing is the last character - I might have had an off-by-one in my previous tests.

I'll try giving it a go today if I can find the time for it.

mike-csis commented 7 years ago

Hey,

I've made a build which supports multiple keys in a sensible way, using the newly found selection technique.. I've made a number of other refactoring changes as well, so I won't be pushing this just yet. Would you, @tk4100, mind testing this build?

It's version is 0.3.4.2 and is a BETA BUILD. Backup stuff and don't use keys you're afraid to lose.

Build: 0.3.4.2-BETA.zip

Screenshot of new warning. image

tk4100 commented 7 years ago

Sorry for the lag here, happy to test!

I'll create/destroy some certs and see what happens.

Genbox commented 7 years ago

Hi @tk4100

Yubico has contacted us regarding this issue, and I've told them about the fix. Let us know what you find out.

tk4100 commented 7 years ago

They just emailed me as well. A few days ago I had suggested to them that it was their library causing the issue here.

mike-csis commented 7 years ago

@tk4100 did you have a chance to get some results? :)

tk4100 commented 7 years ago

Sorry Mike, not yet. I've been pulled onto another project for a little while, but I haven't forgotten you!

mike-csis commented 7 years ago

@tk4100 I performed some tests myself as some time freed up. I've pushed release 0.3.5.0 since it all worked. You're more than welcome to give that release a go, and see if it resolves your issue completely.

I'll close this for now, you may reopen it if the issue isn't resolved.

The release is here: https://github.com/CSIS/EnrollmentStation/releases/tag/0.3.5.0

tk4100 commented 7 years ago

I'll still test when able, if I notice anything I'll let you know. Thanks!

tk4100 commented 7 years ago

Tested OK with me! I told you I'd be back...