bitlockerConversionStatus broken by type change

[ItsMattL]

It looks like commit f838a777a94dd27f6846613eba7357d54eb76c88 inadvertently broke the bitlockerConversionStatus by changing the types from int32 to uint32. Our dependent code started returning the "unable to get conversion status while getting BitLocker info; the volume is locked" error. Further investigation suggested this was due to !ok triggering on the type conversion, rather than on the value itself.

It should be possible to confirm this by checking the behavior with reflect.TypeOf: fmt.Printf("Result type: %s", reflect.TypeOf(statusResultRaw.Value()))

[iamacarpet]

Apologies @ItsMattL ,

I hadn’t spotted that in review, it was a poor review on my part and we haven’t updated to the tip of master for a while.

You may well be a more active user than I am myself so I’d like your opinion: would you class anything in that commit as useful, or would you revert the whole thing?

[ItsMattL]

Yeah no worries. The constants certainly seem useful. I'll also say I haven't validated the types on all of the values to see if uint is correct for any of them, so ideally someone else could do a verification on the behavior here and make sure it's not something weird in our version of ole or something. The Microsoft docs are only so reliable in my experience, at least when being translated through go-ole. Sometimes the types match exactly, other times you get these weird mismatches. I'm curious if this actually worked for the original contributor, or if it was entirely based on the API docs.

[iamacarpet]

Ditto on the Microsoft docs not matching what types go-ole seems to return, took a lot of trial an error when first writing this stuff.

I’ll be honest and say that at work, I’m too tied up with other projects and will likely only get a few days later in the year to properly do anything related to this, so I can’t fully commit the time to resolving this at the minute, as I think you are right that it’ll need some proper testing & validation if we’re going to change the types.

I’ll revert the commit when I’m back in the office so the tip of master is back to something that had been stable for a fairly long time.

In the meantime, @safinsingh , are you interested in joining the conversation and possibly fixing your contribution?

[safinsingh]

I believe I mentioned in my pull request that the return value should be an unsigned integer according to these Microsoft docs: https://docs.microsoft.com/en-us/windows/win32/secprov/getconversionstatus-win32-encryptablevolume. I would make sure there is no issue with the environment and that statusResultRaw.Value() is not returning 0x80310000 before reverting the types. It could very well be some sort of issue with the docs themselves because they haven't been entirely reliable in my experience as well. I just want to ensure utmost correctness of the types which is why I would want to make sure that the issue is not with your environment first.

[iamacarpet]

Just had a bit more time to look into this and I was wrong, we are using a version of master that has this included.

I believe we are just using a different function that wasn't touched by this change, GetBitLockerRecoveryInfo.

I can see in the history, there was one change where I swapped some values to uint32 myself: https://github.com/iamacarpet/go-win64api/commit/0fa5b6bd91f5a356f6696961efbafa197ad27d45 Then swapped it back again as it wasn't working (but only in some situations): https://github.com/iamacarpet/go-win64api/commit/315d753666e0b6c3dba0d2f28ed14017bee501e0

I'm going from memory here, but we don't really touch this code unless something breaks in our internal app & we have to spend time on it, so I believe we started getting weird behaviour in a particular version of Windows 10 (we use Windows 10 x64 exclusively), hence the change to uint32 which initially fixed it.

Another Windows 10 update seemed to change things back to needing int32 instead, but we had to support both versions simultaneously (any that's why the second commit didn't just change the type inference back).

I never touched the code you are both referencing as I don't believe we are using it in our app, but perhaps it needs the same kind of check to support both?

I agree that it would be really helpful to confirm if you are both seeing inconsistent behaviour:

• What types are being returned?
• What versions of Windows are you using? Full build numbers ideally.
• What version of go-ole are you both using? The tip of master appears to have it fixed in go.mod to github.com/go-ole/go-ole v1.2.6, can you confirm if this is what your local project is following?