AES-GCM runtime availability check, Advertise / initialize AES-GCM cipher only if supported#197
Conversation
Some backends may not support AES-GCM on all systems (example: libsodium). Add a new static AES_GCM_CipherContext::IsAvailable() function.
Some backends may not support AES-GCM on all systems. Be more precise in what algorithms are advertised / initialized.
past-due
changed the title
| m_msgCryptLocal.add_ciphers( k_ESteamNetworkingSocketsCipher_AES_256_GCM ); | ||
| } | ||
| Assert( m_msgCryptLocal.ciphers_size() > 0 ); | ||
| break; |
There was a problem hiding this comment.
I think this situation should be fatal to the connection. (If the connection is configured to require encryption, it should be fatal if encryption is not available.)
| m_cryptContextRecv.Wipe(); | ||
| } | ||
| m_eNegotiatedCipher = k_ESteamNetworkingSocketsCipher_INVALID; | ||
| m_cbEncryptionOverhead = k_cbAESGCMTagSize; |
There was a problem hiding this comment.
It should be legal to always wipe these, even if they aren't initialized?
Stepping back:
If we're going to support more than one cipher we need to refactor this. because m_cryptContextSend is too generic of a name if it is always going to be an instance of AES-GCM. We probably should add a new abstract interface ISymmetricEncryptContext, with two virtual methods, Encrypt() and the destructor. We would never "wipe" a context, we would just delete it, and the destructor would always securely wipe it. AES_GCM_EncryptContext would implement this interface. (The old mix-in interface design pattern --- yes, multiple-base classes.) So then we would have std::unique_ptr<ISymmetricEncryptContext> m_pEncryptContext. (And the same for decrypt.) So you would deal in concrete types when you were initializing, but once initialization was complete, you would just have the abstract type.
There's a lot of stuff going on in the Steam branch that needs to be merged and cleaned up in crypto land (recently upgraded to a new version of OpenSSL), but if you want to take a stab at it, I can pull that and then I can clean this up when I get back in sync with Steam.
There was a problem hiding this comment.
I'd debated that, but I wasn't sure how extensive a refactor would be eagerly welcomed. 😄
Will take a look when I have a moment. 👍
|
Closed in favor of #375 🥳 |
2 participants
Implements the first two tasks of #196
Opening as a draft for feedback.