[PATCH v2 0/1] MR5522: sppc: Add stub SLInstallLicense().
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56316 -- v2: sppc: Add stub SLInstallLicense(). https://gitlab.winehq.org/wine/wine/-/merge_requests/5522
From: Vijay Kiran Kamuju <infyquest(a)gmail.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56316 --- dlls/sppc/sppc.c | 10 ++++++++++ dlls/sppc/sppc.spec | 2 +- include/slpublic.h | 1 + 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/dlls/sppc/sppc.c b/dlls/sppc/sppc.c index 8819961d7c6..5c612247a22 100644 --- a/dlls/sppc/sppc.c +++ b/dlls/sppc/sppc.c @@ -39,6 +39,16 @@ HRESULT WINAPI SLGetLicensingStatusInformation(HSLC handle, const SLID *app, con return SL_E_RIGHT_NOT_CONSUMED; } +HRESULT WINAPI SLInstallLicense(HSLC handle, UINT size, const BYTE *license, SLID *fileid) +{ + FIXME("(%p %u %p %p) stub\n", handle, size, license, fileid ); + + if (!handle) + return E_INVALIDARG; + + return S_OK; +} + HRESULT WINAPI SLOpen(HSLC *handle) { FIXME("(%p) stub\n", handle ); diff --git a/dlls/sppc/sppc.spec b/dlls/sppc/sppc.spec index 6926d4d8b6d..2763b6acb3c 100644 --- a/dlls/sppc/sppc.spec +++ b/dlls/sppc/sppc.spec @@ -46,7 +46,7 @@ @ stub SLGetProductSkuInformation @ stub SLGetSLIDList @ stub SLGetServiceInformation -@ stub SLInstallLicense +@ stdcall SLInstallLicense(ptr long ptr ptr) @ stub SLInstallProofOfPurchase @ stub SLInstallProofOfPurchaseEx @ stub SLIsGenuineLocalEx diff --git a/include/slpublic.h b/include/slpublic.h index 97fa8c798c9..841e3bcbf2f 100644 --- a/include/slpublic.h +++ b/include/slpublic.h @@ -65,6 +65,7 @@ typedef struct _tagSL_LICENSING_STATUS SLCAPI HRESULT WINAPI SLGetLicensingStatusInformation(HSLC, const SLID*, const SLID*, LPCWSTR, UINT*, SL_LICENSING_STATUS**); SLCAPI HRESULT WINAPI SLGetWindowsInformation(LPCWSTR, SLDATATYPE*, UINT*, LPBYTE*); SLCAPI HRESULT WINAPI SLGetWindowsInformationDWORD(LPCWSTR, LPDWORD); +SLCAPI HRESULT WINAPI SLInstallLicense(HSLC,UINT,const BYTE *,SLID*); SLCAPI HRESULT WINAPI SLOpen(HSLC*); #ifdef __cplusplus -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5522
This isn't going to work. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5522#note_68254
On Sat Apr 20 11:11:24 2024 +0000, Nikolay Sivov wrote:
This isn't going to work. Do I need to set the fileid to a valid/null GUID?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5522#note_68261
On Sat Apr 20 11:11:24 2024 +0000, Vijay Kiran Kamuju wrote:
Do I need to set the fileid to a valid/null GUID? Licensing system is a now a Windows component, so stubs like that are not going to help anything.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5522#note_68262
On Sat Apr 20 11:16:07 2024 +0000, Nikolay Sivov wrote:
Licensing system is a now a Windows component, so stubs like that are not going to help anything. Atleast this can make installation go further, when registering during installation.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5522#note_68280
On Sat Apr 20 18:20:17 2024 +0000, Vijay Kiran Kamuju wrote:
Atleast this can make installation go further, when registering during installation. @nsivov Wine is full of stubs to make programs not crash, that's fine.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5522#note_68293
On Sun Apr 21 20:08:38 2024 +0000, Loïc Rebmeister wrote:
@nsivov Wine is full of stubs to make programs not crash, that's fine. I think the problem is that the program is going to crash anyway shortly after calling the stub. Did you see any visible improvement with this patch?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5522#note_68681
On Wed Apr 24 06:42:35 2024 +0000, Alex Henrie wrote:
I think the problem is that the program is going to crash anyway shortly after calling the stub. Did you see any visible improvement with this patch? Well, even if it's just crashing later, it is still an improvement, stubs are always a first step before any kind (partial or full) implementations.
The logging with `FIXME` should be here to help with debugging too in the future. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5522#note_68763
On Wed Apr 24 18:30:35 2024 +0000, Loïc Rebmeister wrote:
Well, even if it's just crashing later, it is still an improvement, stubs are always a first step before any kind (partial or full) implementations. The logging with `FIXME` should be here to help with debugging too in the future. I started to write a reply about how it might be impossible to implement these functions in a way that works at all, but then I remembered that in merge requests !2374 and !2377 @danielzgtg already demonstrated that stubbing them gets MS Office to work. Unfortunately, those patches have still not been accepted, and at this point they have gone so long without review that their author has probably given up on collaborating with the Wine project.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5522#note_68816
On Thu Apr 25 05:02:02 2024 +0000, Alex Henrie wrote:
I started to write a reply about how it might be impossible to implement these functions in a way that works at all, but then I remembered that in merge requests !2374 and !2377 @danielzgtg already demonstrated that stubbing them gets MS Office to work. Unfortunately, those patches have still not been accepted, and at this point they have gone so long without review that their author has probably given up on collaborating with the Wine project. The MR https://gitlab.winehq.org/wine/wine/-/merge_requests/2374 already contain this code.
I have been checking Wine MRs, and a lot of obviously simple, small, and helpful MRs are getting ignored into dust. I'll just put that to Wine developers being very busy, but this definitely feels very suspicious. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5522#note_68832
On Thu Apr 25 10:28:54 2024 +0000, Loïc Rebmeister wrote:
The MR https://gitlab.winehq.org/wine/wine/-/merge_requests/2374 already contain this code. I have been checking Wine MRs, and a lot of obviously simple, small, and helpful MRs are getting ignored into dust. I'll just put that to Wine developers being very busy, but this definitely feels very suspicious. The point is that stubs are not going to make licensing work, and Office won't work without proper licensing. What !2377 does it an attempt to circumvent it, which we obviously don't want.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5522#note_68833
On Thu Apr 25 18:05:45 2024 +0000, Loïc Rebmeister wrote:
Yeah, application specific workaround are meh, but https://gitlab.winehq.org/wine/wine/-/merge_requests/2374 doesn't contain any application specific workaround. Adding a stub that doesn't actually help anything is basically just extra noise, and more code in the tree. In cases where someone is actively working on a proper implementation, it also often gets in their way. It may not be a big deal, but it's not exactly like there are zero disadvantages either.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5522#note_68870
On Thu Apr 25 10:38:34 2024 +0000, Nikolay Sivov wrote:
The point is that stubs are not going to make licensing work, and Office won't work without proper licensing. What !2377 does it an attempt to circumvent it, which we obviously don't want. Yeah, application specific workaround are meh, but https://gitlab.winehq.org/wine/wine/-/merge_requests/2374 doesn't contain any application specific workaround.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5522#note_68869
On Thu Apr 25 19:10:22 2024 +0000, Elizabeth Figura wrote:
Adding a stub that doesn't actually help anything is basically just extra noise, and more code in the tree. In cases where someone is actively working on a proper implementation, it also often gets in their way. It may not be a big deal, but it's not exactly like there are zero disadvantages either. For me a stubs that allow a programs to properly run further is a win-win situation as long as the stub stays on it's side.
The "In cases where someone is actively working on a proper implementation, it also often gets in their way." parts is not really a big deal as wine contributors already maintain patches on top of the latest code-base for multiples month already, adapting a patch on top of a stub is really not a big deal, I have dealt with far worse. Those are not really a problem, sure it can take a few minutes, bit it's not a big deal, and almost everyone doing an MR to Wine has to deal with it already in some way. If you still really want to go to the way of the "no stub" route, proper guidelines should be there in written form so there won't be 4th MR opened with the same stub, as most peoples would still think this would be useful to spend their time on this and are likely to open many more of the same MRs for nothing. I really want to try and take a look on how to do a proper implementation though, as I have a sneaking suspicion it's not as complex to implement as it seems. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5522#note_68990
For me a stubs that allow a programs to properly run further is a win-win situation as long as the stub stays on it's side.
How is it a win in any way? You're not making the program actually work, and you're adding more useless code to the repository. Nor is it contributing particularly interesting work; stubs are not hard to write.
The "In cases where someone is actively working on a proper implementation, it also often gets in their way." parts is not really a big deal as wine contributors already maintain patches on top of the latest code-base for multiples month already, adapting a patch on top of a stub is really not a big deal, I have dealt with far worse.
Those are not really a problem, sure it can take a few minutes, bit it's not a big deal, and almost everyone doing an MR to Wine has to deal with it already in some way.
Sure, it's not hard, but it's a pain. I've had to deal with that before and I'd really rather not again.
If you still really want to go to the way of the "no stub" route, proper guidelines should be there in written form so there won't be 4th MR opened with the same stub, as most peoples would still think this would be useful to spend their time on this and are likely to open many more of the same MRs for nothing.
Sure. Where would you suggest to put this advice?
I really want to try and take a look on how to do a proper implementation though, as I have a sneaking suspicion it's not as complex to implement as it seems.
It's actually pretty much impossible. The licenses are encrypted with a private key which is distributed as part of the OS, and which we cannot legally redistribute. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5522#note_69045
How is it a win in any way? You're not making the program actually work, and you're adding more useless code to the repository. Nor is it contributing particularly interesting work; stubs are not hard to write.
I agree stubs are not interesting, boring, and easy to write, but I still believe they are useful, and help programs work better the majority of the time, wine often spam "FIXME (...) stub" a lot when running programs, while the program appear to run completely fine. I think the fact https://gitlab.winehq.org/wine/wine/-/merge_requests/2374 made an installer work is a **huge** win for Wine, even if the installed program it installed didn't work properly afterward.
From what other peoples reported, having `SLInstallLicense`, `SLGetSLIDList`, and `SLGetApplicationPolicy` all as stubs in sppc without any extra hacks makes Office365 able to fully install and show a window saying there is a missing license, and it freezes afterwards due to an error related to `v8jsi.dll`, which the code seems to be available here https://github.com/microsoft/v8-jsi under MIT.
Sure, it's not hard, but it's a pain. I've had to deal with that before and I'd really rather not again.
Well, this is definitely foreshadowing a personal project of mine.
Sure. Where would you suggest to put this advice?
Both on the wiki and somewhere visible in the wine code-base. For the wiki, I'll say putting a link to contributing guidelines there https://wiki.winehq.org/Developers and there https://wiki.winehq.org/Source_Code should be the minimum, cause I often visit the "Source_Code" page to access the gitlab (So I assume other devs may do that too) and the "Developers" because it's where the guidelines are expected to be. For the codebase, a link to where the guidelines are in `README.md` should be enough. Including reference or link to guidelines in multiples places is very important to make sure first time contributors don't miss them.
It's actually pretty much impossible. The licenses are encrypted with a private key which is distributed as part of the OS, and which we cannot legally redistribute.
<details> <summary>Info I gathered for partial sppc implementation (Optional read)</summary> Trying to get info while not reading/accessing closed/restricted information/code, I found the following code which could be helpful in some way, some of these contain stubs but some useful info: - <https://github.com/AR1972/Injector/tree/master/asus> <- Contain an example license an example usage, the attached license isn't encrypted in any way, and sent as-is into `SLInstallLicense`, if we can at least partially support unencrypted licenses maybe it's a good start? - <https://github.com/TGSAN/slshim/blob/master/slshim.c> <- Mostly stubs, but still implement some functions giving more insight into how to implement sppc funtions. I was also able to determine all the calls go thru a windows service that process licenses, I was thinking of making all the license processing stuff locally on the dll instead on an external service, since I only did a surface investigation to avoid reading anything restricted, I don't know where the license data is stored, and thought on making wine use a custom folder to store licenses. </details> But now that I'm reassessing the situation with this new info, I can confidently say going from stubs to the maximum level of implementation legally possible in wine will bring fewer benefit to wine than going from nothing to stubs will, off course any stubs would need to return that the license is invalid for legal reasons. Probably making a patch-set fixing `v8jsi.dll` issues would make theses sppc stubs an undeniable last requirement for Office365 to fully run, and make the stubs no longer part of the "useless clutter" category, would it be fine if I make an MR called "Fully fixing Office365" containing theses stubs if I make sure to use the minimum amount of stubs required to make both the installer and the program itself work properly? # Stub guideline tangent @zfigura Also could you give me an example of guidelines on where stubs are or are not allowed, just try to write it like if it was a real guideline, I'll try to follow it to the best of my abilities when making MRs, while keeping in mind what you wrote here is temporary. I think an example of guidelines for stubs what would align with your line of thought while being reasonable would be: ``` # Contributing stub guideline Try to avoid stubs and semi-stubs to a minimum, you may only make new stubs or updates existing stubs if **all** of theses condition are met: 1. Making a full implementation or semi-stub require a substantial amount of effort or is not possible in wine. 2. The stub is required to allow a stable and normal execution of a program. 3. The stub must not attempt to bypass, circumvent, or workaround any licensing or content protection system. 4. The stub contain a `FIXME` for debug, a `TRACE` can be used instead **only** if the stubs is being called frequently enough that it is causing noticeable performance issues on the targeted program. 5. The stub behavior is consistent regardless of process name or loaded executable. You **must** explicitly justify that each of the stubs in your MR follow each of theses rules, and you must explicitly tell which programs is benefiting from the stubs, and in which way it is benefiting from the stub, with a link on where to get a legal copy of the programs if available. For special cases such as semi-stubs, you must still include justification like stubs. When improving an existing stub, you must still justify your motivation of the stubs if your are not making a full or partial implementation. When making COM classes (Aka. vtbl/interface) that contain multiple functions, you may justify the need of a stub per COM class instead of per stub. See: https://wiki.winehq.org/Wine_Developer's_Guide/COM_in_Wine#Adding_the_stub_class Note: Your stubs may be rejected if other part of wine prevent the program from running properly, as a general rule, only try to upstream stubs when wine is fully ready to handle the software with your MR applied. ``` (I took a lot of time to proof read this myself) The condition about "licensing" is for legal reason, as in my opinion, the `Sleep(1000)` in https://gitlab.winehq.org/wine/wine/-/merge_requests/2377 is a clear legal liability for the Wine project. (Note: I'm not a lawyer.) (See "DMCA anti-circumvention provisions" at https://www.law.cornell.edu/uscode/text/17/1201) The MR https://gitlab.winehq.org/wine/wine/-/merge_requests/5484 is an example of why the exception to COM classes is very important. I even make an explicit reference to https://wiki.winehq.org/Wine_Developer's_Guide/COM_in_Wine#Adding_the_stub_class in the guideline. I hope this will inspire you to write a corrected policy on stubs, I would suggest you copy mine as a base and change what you don't like about it. I really want clearer guidelines, this would help a lot and stop peoples wondering why their MR is ignored, as the community could point theses developers to the guidelines. I know it's a long read, but it should be worth your time. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5522#note_69162
participants (6)
-
Alex Henrie (＠alexhenrie) -
Elizabeth Figura (＠zfigura)
-
Loïc Rebmeister
-
Nikolay Sivov (＠nsivov)
-
Vijay Kiran Kamuju
-
Vijay Kiran Kamuju (＠infyquest)