SteamUser LoginID customization

[JustArchi]

obfustucated_private_ip is being used by Steam to tell which connection is made from the same client / application. Using the same value for second logon will cause first session to disconnect with EResult.LogonSessionReplaced. In some scenarios we may actually want to establish second independent connection to the same account. This is especially useful for sharing the account between SteamKit-based Bot and Steam Client at the same time.

By default old logic is kept, I only made it possible to override it if LoginID is supplied in LogOnDetails, shouldn't cause any problems or regressions.

Thank you.

[codecov-io]

Current coverage is 17.56%

Merging #217 into master will decrease coverage by -0.02% as of 0daf02a

@@            master    #217   diff @@
======================================
  Files           67      67       
  Stmts         7292    7299     +7
  Branches       520     521     +1
  Methods          0       0       
======================================
  Hit           1282    1282       
  Partial         73      73       
- Missed        5937    5944     +7

Review entire Coverage Diff as of 0daf02a

Powered by Codecov. Updated on successful CI builds.

[Netshroud]

Not sure I like the idea of manually setting this field... I would have thought of setting an IPAddress.

At the very least, make LoginID nullable (uint?). Try to avoid magic sentinel values where possible.

[JustArchi]

Setting IPAddress would still result in possibility for a conflict if IP address would actually be used, also it would further complicate things instead of just being unique ID field. I thought of that, and in the end if the "hack" ever stops working, I want it to do just that instead of a possibility to execute things on wrong IP addresses.

uint? is OK.

[Netshroud]

My thinking with IPAddress was that you could [randomly] select an IP address from a reserved range (e.g. 127/8, 10/8, 192.168/16 etc.) so that Valve's servers are still getting a legitimate value.

I'm hesitant to throw garbage at the CMs after we did it for so long with the Machine ID, and then that came back to bite us later.

[JustArchi]

The usage of LoginID is specific and even if Valve does change something regarding that property, only programmers who actually did use custom LoginID will get problems, as 99.9% of users, including everything made up to that point, still uses old valid logic. I'm not even sure if there is any project apart from mine ASF that would potentially need custom LoginID anyway.

We can always come back to this later and think of better solution, e.g. randomizing private IPs as you suggested. For now I don't know how customizing that property would cause any possible side-effects. I wouldn't send you a pull request if I didn't carefully test that option before.

Thing is that setting LoginID to specific value can't cause any potential conflicts, while randomizing valid IPAddress can still cause a conflict if local network does use that address too.

If I had any better solution to the problem I'm facing, I'd suggest that instead. For now I think this pull request is OK, and if we ever find any obstacle because of custom LoginID (which is optional), I'll come up with another pull request fixing it.

MachineID was mandatory, and affected every project. LoginID is optional and very specific to workaround a problem I mentioned in the pull request.

[voided]

We can always come back to this later and think of better solution

We're not in a rush, lets write The Right Code™ that doesn't involve deprecating poorly designed fields in the future.

[JustArchi]

Randomizing IP is not a solution, it'll actually cause even more problems than it solves. I'm not sure if you understand that.

By accepting the way how it is done now, we're making it possible for programmer to set that property based on whatever algorithm he wishes, which includes a possibility for him to randomize a local IP and apply obfuscation mask.

If we change it to get random IP address on each login (if let's say RandomizeLoginID is set), we will actually cause even worse situation because every LogOn request will be unique, hence, won't disconnect any previous session to the same account. This is even worse because with current logic, I can implement unique IDs myself and handle sessions however I want, while after implementing randomization of IPs I will be able to only say if the session should remain unique or not.

You're overcomplicating things and trying to invent idiot-proof solution rather than only implementing the way for own logic. I can't even imagine how complicated it would become if we stored list of unique randomly generated local IPs for each account, connected with what, steam user object? Programmer can make new one, how he'll be able to set old randomly generated IP to new object? How hard it would be for programmer to tell if that session should be unique or not? What if he wants to start one unique session and one non-unique? What if he wants to start 1 session that should share IP with steam client, 2 independent ones and 2 shared?

This pull request is as bad, as Valve's logic for their network to base unique sessions on that obfuscated IP property. We can either accept it in current form, which is nice, short, easy to understand, and leaves programmer with full control how he wants to apply it to his project, or cause idiot-proof mess which would dramatically decrease code readibility, would be unreliable, and would require dozen of properties to be able to adapt the logic to many different situations.

I do understand the importance of good code, I do understand that this pull request considers a hack, but based on my knowledge and my own projects I say, that this is the best way for now and we shouldn't try to implement something that will overcomplicate things only to take responsibility from programmer and try to do things for him. We don't know how he wants to use the property, hence we should make it available for him, and not consider that "we know better", because we don't.

I'm not the only one that made LoginID customizable.

Just my 5 cents, if you don't have any better idea and still don't agree with me, I think there's little to be done and we can close this pull request, because it's unlikely that I can convince you to my opinion.

[voided]

I'm not saying that we need to settle on a specific design immediately (such as randomizing the IP), just that there should be some discussion about the correct method.

Frankly, I like the idea of being able to specify any sort of "login id", but I'm curious of the consequences of doing so since the field is aptly named private_ip. Some considerations to be had here could be if the in-home Streaming feature is affected by this field.

[JustArchi]

That's why I did my own review of all possible drawbacks before suggesting the pull request.

The original property is based on localIp ^ MsgClientLogon.ObfuscationMask. This is a good way for Steam Client because private local IP is always unique for entire local network, so we can handle things like making sure that there is only one session active from given PC. I guess steam network applies XOR to that to get local IP back, and based on it can guess if session should be considered as in-home streaming if local network is the same as the one already connected, or not because it's totally different IP.

I did try various different combinations, including 0, 1337, and 3131961357 (which is 0xbaadf00d, obfuscation mask), and everything worked properly. I didn't notice any drawbacks or other weird issues during using any of them, and the "most-correct" LoginID for me is 3131961357, because it will result in 0 value after XOR operation, but as with every other custom value, resulting IP will actually be invalid, so unless we fake IP address and apply obfuscation mask, any LoginID will be invalid, in terms of the resulting IP.

I don't want to fake IP address for many different reasons. The most important is that if in fact Valve uses that property for something more advanced, I don't know, SteamGuard stuff for example, I prefer to send CMs invalid IP that will be deemed invalid and won't be used at some point during executing request, rather than allowing the fake IP to pass all the checks and cause possibly wrong situations such as false alarm of being logged from unknown PC etc.

Programmer can (and should) apply his own logic to that LoginID property, depending what he considers appropriate. I couldn't find any possible drawbacks, at least for now, and setting LoginID helps me to make it possible for Bot to log in on the same account being used by Steam Client, on the same PC, and at the same time, while still using the same LoginID across my Program, so any non-intentional connects can be found and shut down by Steam network, instead of leaving ghost unique sessions somewhere.

[Netshroud]

Why is it that you can assign an integer LoginID (e.g. 42), but not an IPv4 LoginID (e.g. 127.0.0.42)?

[JustArchi]

Of course I can, but I don't want to, for reasons I stated above.

I could add another propety like LoginIP, but do we really want that? That would make it 2 extra properties, while only one can be used. I prefer to use uint-based LoginID, because I can type any value there, while using IPAddress requires valid (in terms of networking) value.

[JustArchi]

Bump.

[voided]

I think the XML documentation should be a little more clear about the purpose of the field. It should mention something about being related to multiple signon for a single account.

Additionally,

will cause this to be automatically generated

Isn't very clear. I think it should specify that it's generated based off the machine's primary bind address, otherwise consumers might believe that it's generated automatically in such a way that users shouldn't need to worry about unique values for this field.

[JustArchi]

Done, I think the documentation covers the purpose of the field precisely now.

[Netshroud]

Nit: Replace == null with .HasValue to be explicit about what is happening. (Roslyn does this when compiling anyway.)

[Netshroud]

LGTM, just pray that Valve don't start enforcing that the value is an IP Address.

[JustArchi]

As requested, I also inversed order to be more readable (HasValue is better than !HasValue)

[JustArchi]

Thank you 👍 .

If there is anything more that I can ask for, I'd be very grateful if new SK release is planned to come out soon, but that highly depends on your release schedule, so I can only kindly ask for it (always better to use official version rather than self-compiled fork or hack).

Cheers!

[voided]

@Netshroud was preparing some things for a 1.7.0 release, I imagine we just need to merge a few pending PRs and we can release it.

[JustArchi]

Great to hear it, thank you! 👍

[Netshroud]

There you go. 😄

[JustArchi]

Much appreciated 👍