Also:
- STEAMNETWORKINGSOCKETS_STEAM now mens "running on steam", not "running using
the steam client". STEAMNETWORKINGSOCKETS_STEAMCLIENT is for that.
- Refactored stats stuff, moved it into the namespace. At one point I thought
I might expose some stuff in a public interface. For now, keeping it internal.
- Removed concept of Steam "universe" from this branch of the code.
- Don't use OVERRIDE, override works.
Deleted a ton of old unused cruft left behind when I ripped this out from Steam.
Don't use tchar for all of our asserts. I don't know why this was done, but it
is dumb, and wasn't finished anyway. We'll just use unicode when interacting
with WIn32 APIs in UNICODE mode. (For XboxOne.)
Fix a pretty sharp edge: when inserting into CUtlHashMap with a function that
only takes a key and inserts the default value, we were using "default
initialization" ("new T"), which does not initialize PODs. Switch to
"value initialization" ("new T{}").
Also some minor compiler compatibility fixes. The version of gcc we're using
here didn't like the {}-style call of the base class constructor, so use ()
instead. *shrug*
Also deleted a kludge for an older version of Visual Studio. We were
supporting that for a while in Steam, but we've upgraded our min specs and
so I'm doing the same here.
Inline a bunch of stuff in the internal implementtion. This can make a big
difference in the debug build.
A much simpler, saner iterator implementation. We don't need full STL
iterator functionality. We just want range-based for loops to work. And we
don't want to expose a non-const reference to the key!!!!!
And some other stuff from Steam branch.
Added AllowLocalUnsignedCert virtual function so connections can customize
this behaviour. Don't read IP_AllowWithoutAuth in base class, that is only
applicable for UDP connections.
We're still not totally locked down in all cases, because there are some
weird edge cases I am worried about breaking on Steam that I haven't
had a chance to test.
Do not automatically trust UDP connections to localhost. That might be
another process, maybe a MITM adversary! Instead, move this custom trust
logic to CSteamNetworkConnectionlocalhostLoopback.
Also don't use OVERRIDE #define, use the override keyword directly. We are
using all kinds of C++ 11 features, so I'd be really surprised if there exists
a compiler that 1.) Doesn't support override 2.) Supports everything else we
are using in the code 3.) We actually care about supporting.
There is something here to fix I think. It hasn't been causing problems, but
I believe we should be more explicitly resetting the expected packet number
when a connection is initialized. (And the first packet should be assigned
a packet number.)
Two changes to cleanup. First, to it earlier, as soon as the connection
switches into a state where we know we will not need to send or receive
messages anymore. (Don't wait until object destruction.) Second, we
were not freeing up any queued messages in the m_messagesQueued linked
list.
Also fix a monir bug in the linger functionality. The connection is still
"connected" for all internal purposes, it just is not visible to the API
anymore. So we need our stats object to stay alive, and we expect our peer
to be replying in a timely manner.
Add some asserts and paranoia.
Add CertStore_Print, which uses.....std::ostream (I know, gross). std::ostream
uses.....***EXCEPTIONS*** (barf). I wish there were a good generic way to
write to a stream without bringing in a bunch of dependencies. (E.g. "use
boost" is not helpful.) I'll probably end up adding a #define to disable this.
Also touched the trust code to make sure it always produces a message, even if
trust is calculated multiple times and the value changes.
We were setting the flag too agressively, and not really allowing for any
opportunism.
Really, tracer pings in stats messages are not that useful end-to-end, and I
probably should delete them, because every single data packet has acks that
serve as ping measurements. If the conversation is very thin, it is perhaps
possible that you could go a while without being able to use the acks for
latency measurement, and tracer ping requests would kick in. But it's just
as likely that a plain old keepalive would end up be sent, too.
Tracer ping requests are much more useful for maintaining a latency
measurement with the relay, on relayed connections, because there is nothing
that happens constantly due to normal traffic that can be used to measure
latency with the relay.
Calling virtual functions in a destructor seems almost always bad, it should
generate a compiler warning. Made destructors protected, replace direct
invocations of operator delete with an explicit Destroy() method, which
ends (in the base class implementation) by calling "delete this".
This is a pretty big perf issue, since it means every time we check *any*
cert, we re-evaluate all of the cert signatures in the entire cert store.
(The design assumes that the numbers of certs will be relatively few,
but this is still pretty bad.)
No functional change intended since any child connection would be
using the same CSharedSocket, and so the BBeginAccept() call that
follows would have failed anyway.
Unfortunately, these don't actually compile right now under either meson
or CMake. (We have our own bespoke project generator in Steam, which was
how these tests were created.) I need some help getting the protobuf
and crypto dependency logic refactored and sharable, if we want to get
those compiling in this project. But I figured I'd go ahead and checkin
the source.
These bugs were discovered using the novel method of writing a test suite
and actually running it. (Checking that in shortly.)
Also tweaked the #define that controls the single hardcoded root CA,
if any. I do not have any test code for when we do not want to do this
right now. For the open source code probably we want to leave it dynamic
by default.
And at the same time, clarified the flow, and treat an
empty identity the same "local host", which is intended to be
used as "anonymous" in places where some identity data is required.
Store a list of public keys and certs we have for them, and implement logic
to evaluate their trust.
New cert checking flow in the connection code. I found a *tiny* bug where
we were not actually checking the signature of the connection crypt info
using the signature in the user cert (!). We were checking to make sure that
the user cert was valid, but anybody in the middle could have substituted
their own ephemeral keypair for key exchange, and we wouldn't have known.
Certs can now be issued to multiple AppIDs.
Cert tool can now emit json output and bind certs to particular AppIDs.
In a few places I need to call this through a base class pointer in
the Steam relay code. Ths stats tracker heirarchy is really complicated.
Might be nice to clean it up and find a way that we never do virtual
function dispatch when we could have just inlined code.
We only needed to know if SSE2 support is present, only in 32-bit, and only
if you are using our 25519 code, which most people hopefully will *not* use.
(Use OpenSSL.)
To answer this one question in this one place, we were using a whole framework
to get a ton of other info, and all of the code had ben ripped out of Steam
and was really gross.
Fletcher Dunn
Steven Noonan
Maciej Sinilo
bobsayshilol