Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OCB2 is insecure #4219

Closed
3 tasks
streaps opened this issue May 29, 2020 · 42 comments · Fixed by #4227
Closed
3 tasks

OCB2 is insecure #4219

streaps opened this issue May 29, 2020 · 42 comments · Fixed by #4227

Comments

@streaps
Copy link

streaps commented May 29, 2020

OCB2 is insecure:

«An internal building block of OCB2 is the tweakable blockcipher obtained by operating a regular blockcipher in XEX∗ mode. The latter provides security only when evaluated in accordance with certain technical restrictions that, as we note, are not always respected by OCB2. This leads to devastating attacks against OCB2's security promises: We develop a range of very practical attacks that, amongst others, demonstrate universal forgeries and full plaintext recovery. We complete our report with proposals for (provably) repairing OCB2. To our understanding, as a direct consequence of our findings, OCB2 is currently in a process of removal from ISO standards. Our attacks do not apply to OCB1 and OCB3, and our privacy attacks on OCB2 require an active adversary.»
https://eprint.iacr.org/2019/311
see also https://en.wikipedia.org/wiki/OCB_mode

I'm not sure if Mumble is vulnerable to these attacks (by design or implementation), but if it is (which I think it is):

I do think this should be fixed, but another issue is that it is not properly communicated to the users of Mumble. The client and and the website mention OCB-AES128, which usually means OCB3, maybe OCB1, but not OCB2.

  • We need to tell the user the correct name and that it is broken encryption.
  • It has to be fixed
  • it should be mentioned in the protocol documentation.

I wonder if OCB3 (RFC 7253) could easily be added as another encryption option. It seems to be a secure encryption method and it has the big advantage that we can use already existing implementations (OpenSSL, pycroptodome, ...).

I know there is the discussion of non-OCB alternatives (#3918), but I still wonder if OCB3 wouldn't be the simpler drop-in replacement for now.

Regarding future crypto that will work with web clients I would just use QUIC (which uses TLS).

I'm not an expert in this field, so please correct me if I'm wrong.

@Krzmbrzl
Copy link
Member

We're already looking to change the encryption. See #3918

For the documentation there's #3487

As far as I know we can't "just replace" the current encryption no matter what as either that "new encryption" is the same as the old (aka nothing changes) or we break backwards compatibility.

And as I think this is also true when switching from OCB2 to OCB3, we might as well take the leap and use whatever comes up in #3918.

/cc @davidebeatrici

@davidebeatrici
Copy link
Member

Correct.

@streaps
Copy link
Author

streaps commented May 29, 2020

How long would we like to wait until something comes up? "drop-in" was maybe not the right word. I meant it would work the same. It's still fast.

@Krzmbrzl
Copy link
Member

@Avatat wanted to do some crypto benchmarks for us in order to figure out which might be best suited for Mumble.

I think we wanted to base our decision based on that

@Krzmbrzl
Copy link
Member

I think we shouldn't break backwards compatibility 2 times in short succession and if we don't do that we can't easily fix the issue at hand.

I tend to close this issue in favor of the mentioned ones... Objections?

@streaps
Copy link
Author

streaps commented May 29, 2020

Yes, objections. This is a huge security vulnerability and this issue addresses not only the code.

@TerryGeng
Copy link
Contributor

Of course, security vulnerability should be addressed. I think at least in the next version, we need to send a message to old clients to tell them their mumble is outdated and there are security concerns. Maybe in the next version, we can adopt another cipher and use it as the default one. If decryption goes wrong, fallback to OCB2. Or we just force old clients use TCP connections by dropping their packets and mumble will switch to TCP according to

CryptState &cs = connection->csCrypt;
cs.uiRemoteGood = msg.good();
cs.uiRemoteLate = msg.late();
cs.uiRemoteLost = msg.lost();
cs.uiRemoteResync = msg.resync();
accTCP(static_cast<double>(tTimestamp.elapsed() - msg.timestamp()) / 1000.0);
if (((cs.uiRemoteGood == 0) || (cs.uiGood == 0)) && bUdp && (tTimestamp.elapsed() > 20000000ULL)) {
bUdp = false;
if (! NetworkConfig::TcpModeEnabled()) {
if ((cs.uiRemoteGood == 0) && (cs.uiGood == 0))
g.mw->msgBox(tr("UDP packets cannot be sent to or received from the server. Switching to TCP mode."));
else if (cs.uiRemoteGood == 0)
g.mw->msgBox(tr("UDP packets cannot be sent to the server. Switching to TCP mode."));
else
g.mw->msgBox(tr("UDP packets cannot be received from the server. Switching to TCP mode."));
database->setUdp(qbaDigest, false);
}
} else if (!bUdp && (cs.uiRemoteGood > 3) && (cs.uiGood > 3)) {

@Krzmbrzl
Copy link
Member

We can't really add a new cipher without breaking compatibility. And we really shouldn't do it twice in a row.

Therefore I don't think we can do much at this point unless someone takes the time and implements special case handling in the server that allows for handling old clients with OBS2, newer clients with OBS3 and then eventually even newer clients with whatever we'll end up using.
And the same on the client side.

@TerryGeng
Copy link
Contributor

TerryGeng commented May 29, 2020

@Krzmbrzl A practical way of doing this is, implement a new control message (maybe called CipherSync or something) containing the type of cipher used and keys while keeping the old CryptSetup message.

For old version, the UDP negotiation starts with the server sending a old CryptSetup message over TCP.
I guess we can send both a CryptSetup and a CipherSync packet, providing the cipher the server supports in the CipherSync packet.
For new clients, it drops the CryptSetup packet and replies with a CipherSync to tell the server which cipher it uses, probably OSB3 or ChaCha, whatever, depends on the platform.
If the server receives no CipherSync reply, then it knows this is an old client that doesn't support new ciphers. In this case, we send the client messages telling them they are using outdated clients and need to update. Then the server falls back to OCB2.

As long as the new cipher candidates are decided, I can give it a shot :).

@streaps
Copy link
Author

streaps commented May 29, 2020

What does breaking compatibility mean in this case?

If the protocol cannot be updated without breaking stuff then maybe it needs better support for announcing and negotiating capabilities.

We always can use different ports and SRV records.

@Krzmbrzl
Copy link
Member

As long as the new cipher candidates are decided, I can give it a shot :).

That'd be great 👍

And yes your suggestion sounds good to me. Instead of a new message type though I think we can simply extend the existing CryptSetup message with a field indicating the desired enryption method. If it's not set, then fall back to OCB2.

And we shouldn't just refuse OCB2 connections imo (maybe add a config setting to do so if desired) as otherwise folks won't be able to connect to newer servers if they're stuck with an older Mumble version.

@Krzmbrzl
Copy link
Member

What does breaking compatibility mean in this case?

It means that if we encrypt differently on the server the client can't decipher it and vice versa.

If the protocol cannot be updated without breaking stuff then maybe it needs better support for announcing and negotiating capabilities.

I agree but afaik there is no support for that yet 🤔

@Avatat
Copy link
Contributor

Avatat commented May 29, 2020

As I know, there was only one encryption protocol for UDP, so there was no need to negotiate anything.

We plan to do it exactly as @TerryGeng wrote - we need to add announce/negotiate protocol/messages, which will be send through secure TLS/TCP channel.
We have to complete benchmark tool (add the new metrics, add support for various OS and hardware), and see how AEGIS will perform on various platforms. If it will perform well on AES accelerated and not accelerated platforms, we will stick with it. If the performance without acceleration will be poor, we have to choose something else, or maybe two different algorithms at once.
Still, there is no support for AEGIS in any crypto library in stable release (we use a development branch of libsodium to benchmark AEGIS).

@Krzmbrzl
Copy link
Member

@Avatat what is your opinion on using OCB3 as an intermediate solution here? As long as we properly implement the "handshake" (so server and client know what they need to use with each other) I think it should be fine, right?

@Avatat
Copy link
Contributor

Avatat commented May 29, 2020

To be honest, I don't have such knowledge yet. I will try to dig it and answer today :)

@TredwellGit
Copy link
Contributor

Backwards compatibility should be actively avoided. If someone with an insecure client is in a channel then the whole voice channel is leaked even if everyone else is using a secure client. Additionally, support for old insecure crypto introduces the chance of a downgrade attack.

Things that need to be done as soon as possible:

  • Verify that Mumble is actually vulnerable.
  • Announce a way to mitigate the issue with the current release.
  • Release a new version that either mitigates or fixes the issue (whichever can be done faster).

Questions:

  • Would blocking UDP mitigate the issue because that would force TLS usage?
  • Can blocking UDP be done in the Mumble server configuration or does it have to be done with packet filtering?
  • Does the current OCB2 crypto guarantee perfect forward secrecy?

Also, this issue should be labeled priority/P0.

@TredwellGit
Copy link
Contributor

To clarify, I mean backwards compatibility with specifically OCB2 not necessarily old clients.

@Krzmbrzl
Copy link
Member

To clarify, I mean backwards compatibility with specifically OCB2 not necessarily old clients.

In this case this is pretty much the same thing though...

Would blocking UDP mitigate the issue because that would force TLS usage?

I think so, yes

Can blocking UDP be done in the Mumble server configuration or does it have to be done with packet filtering?

Afaik this had to be done via package filtering

Does the current OCB2 crypto guarantee perfect forward secrecy?

No idea.

Verify that Mumble is actually vulnerable.

Before this has been shown I think I kinda had a bad feeling about introducing a non-backwards compatible change.

What I think we can do though is to add a server config option that allows to disable specific cyphers. That way a server owner can decide whether they want to allow old clients with OCB2 or not (the default should probably be to not allow them though)

@Krzmbrzl
Copy link
Member

Krzmbrzl commented May 29, 2020

From the linked paper:

This observation immediately suggests a fix: If the involved users agree that all encryption/decryption operations are with respect to a non-empty AD, then it seems(to us) that all problems go away.

Does somebody with more background in cryptography than me (maybe @Avatat ) understand what this means for Mumble? Can we simply apply that fix?

@TredwellGit
Copy link
Contributor

To clarify, I mean backwards compatibility with specifically OCB2 not necessarily old clients.

In this case this is pretty much the same thing though...

Would blocking UDP mitigate the issue because that would force TLS usage?

I think so, yes

Old clients could be supported if there is a secure way to block UDP for OCB2 clients.

@streaps
Copy link
Author

streaps commented May 29, 2020

Can we simply apply that fix?

... and don't break the different DIY implementations of the undocumented OCB2 variant. Good question.

@TredwellGit
Copy link
Contributor

The two countermeasures just discussed require that the code of both the sender and the receiver would have to be adapted. It might be impossible to do so for instance if OCB2 is included in already shipped products that cannot be updated remotely.

In principle, each of our suggestions would require its own formal security analysis. However, we provide one only for the “XEX for the last plaintext block” fix presented in Sec. 9.1 and the “XE for the last message block” fix presented in Sec. 9.2. Our other proposals intuitively lead to a secure scheme, however, without conducting further research we cannot fully vouch for their security because these proposals do not allow the proof strategy that we adopt for OCB2f.

If an update is required, then it would be better to switch to an algorithm that has undergone cryptanalysis and is not patent encumbered.

@Krzmbrzl
Copy link
Member

Well if it is a fix to how we use the cipher, we could backport that to the 1.3.x series and don't have to completely break old clients (more or less)

@streaps
Copy link
Author

streaps commented May 29, 2020

Would blocking UDP mitigate the issue because that would force TLS usage?

It would mitigate this issue, but there might be still TLS < 1.2 issues ;)

Can blocking UDP be done in the Mumble server configuration or does it have to be done with packet filtering?

The server could just refuse to reply to a ping and don't send any UDP data to older clients, then clients would switch to TCP.

The question is if this is preferable for all (or most) use cases. Some users might prefer better audio/streaming performance over perfect encryption and are talking on an public channel anyway. AFAIK OCB2 still obfuscates the content pretty well and the paper talks about active attacks.

I think the important part is that users know that the connection is not perfectly secure, then they can decide for themself if they want to talk on a somewhat insecure line.

@streaps
Copy link
Author

streaps commented May 29, 2020

@Krzmbrzl but old clients (released before the backport) would still use the old crypto, nothing you can do about it (if the server still accepts the UDP stream). It's not a complicated handshake like in TLS, where the server has some control over the negotiated ciphers. With OCB2 in Mumble the server sends two nonces and a key, nothing else.

Even if it were possible to mitigate the vulnerability for server-to-client UDP streams, some old clients would retransmit the content back via echo / acoustic feedback over the vulnerable client-to-server UDP stream.

@Krzmbrzl
Copy link
Member

Unless we implement logic that checks what clientversion the client is and if it is too old, it will not send UDP packets to it. That should force it to switch to TCP

@TredwellGit
Copy link
Contributor

It would mitigate this issue, but there might be still TLS < 1.2 issues ;)

You are right. This needs to be updated.

mumble/src/murmur/Server.cpp

Lines 1375 to 1383 in 50aea07

#if QT_VERSION >= 0x050500
sock->setProtocol(QSsl::TlsV1_0OrLater);
#elif QT_VERSION >= 0x050400
// In Qt 5.4, QSsl::SecureProtocols is equivalent
// to "TLSv1.0 or later", which we require.
sock->setProtocol(QSsl::SecureProtocols);
#else
sock->setProtocol(QSsl::TlsV1_0);
#endif

#if QT_VERSION >= 0x050500
qtsSock->setProtocol(QSsl::TlsV1_0OrLater);
#elif QT_VERSION >= 0x050400
// In Qt 5.4, QSsl::SecureProtocols is equivalent
// to "TLSv1.0 or later", which we require.
qtsSock->setProtocol(QSsl::SecureProtocols);
#else
qtsSock->setProtocol(QSsl::TlsV1_0);
#endif

The server could just refuse to reply to a ping and don't send any UDP data to older clients, then clients would switch to TCP.

The server would need to verify over TLS that it is an older client otherwise a man in the middle could lie that the version is newer thus allowing OCB2 UDP.

Some users might prefer better audio/streaming performance over perfect encryption and are talking on an public channel anyway.

I am in the process of testing how TCP voice sounds in practice. If the encryption or authentication of the voice channel is broken, then that is a nonstarter for me. If TCP voice is unacceptably bad I am moving off of Mumble as soon as possible until the crypto is fixed.

@dt47774
Copy link

dt47774 commented May 29, 2020

Some loose thoughts which may be valuable.

Whilst I agree that backwards compatibility is important, like mentioned above, be very careful not to let the new solution be vulnerable to a downgrade attack where an attacker in the middle could force a client/server to use the old crypto.

If the server logged what encryption method was used when a client connected, it could help an admin understand who is still on older versions and could encourage them to upgrade.

On the client, relying on warning dialogues with clickthroughs can be dangerous as they are often clicked through without thought and this would affect everyone in the call.

At the cost of security in the benefit of usability, implementing something where a server asserts that they will always use cipher X or above will mean that, after a client connects to a server for the first time, it shouldn't ever be able to connect to the same server using a weaker cipher. This isn't perfect but it's user friendly.

@TredwellGit
Copy link
Contributor

Qt has support for DTLS; why doesn't Mumble consider just using that for the new UDP crypto (#3918)? https://doc.qt.io/qt-5/qssl.html#SslProtocol-enum

When I connect using TLS 1.3 it uses TLS_AES_256_GCM_SHA384 by default which is both DTLS OK and recommended by the IETF. https://www.iana.org/assignments/tls-parameters/tls-parameters.xhtml

@streaps
Copy link
Author

streaps commented May 29, 2020

Btw, the webrtc branch mumble-web uses a webrtc proxy in front of murmur. WebRTC also uses DTLS.

@TredwellGit
Copy link
Contributor

If I understand the conclusion of #4080, Ubuntu 20.04 LTS is now the baseline for Mumble. This means that features present in Qt 5.12.8, OpenSSL 1.1.1f, and C++17 (GCC 9.3.0 and Clang 10.0.0) are expected. This means that the code linked in #4219 (comment) can be replaced with just sock->setProtocol(QSsl::TlsV1_3OrLater); right now.

This might be part of a different discussion, but I am wondering about the value of the 1.3.x releases altogether. That is, we could just stop worrying about backporting and 1.4.0 could be released right now. Does the server code on master have any compatibility issues with 1.3.0 clients? I have been using the client compiled off of master daily for months and it works fine with both 1.3.0 and other master clients on a 1.3.0 server even when using features like Opus low delay.

@streaps
Copy link
Author

streaps commented May 29, 2020

The server would need to verify over TLS that it is an older client otherwise a man in the middle could lie that the version is newer thus allowing OCB2 UDP.

I don't understand. My assumption was that the server don't speak OCB2 anymore. The server also doesn't have to send the CryptSetup (nonce and key), which would make the encryption impossible for the client.

Refusing OCB2 and downgrading to TCP should work. I think most (or all?) clients support audio over TCP.

@TredwellGit
Copy link
Contributor

My assumption was that the server don't speak OCB2 anymore.

In that case, you can ignore what I said.

@Johni0702
Copy link
Contributor

Disclaimer: While I am generally interested in it, I do not regularly do crypto stuff nor have I studied it, so I may be missing something which would be obvious to a trained person.

I have read through the paper up to and including the first paragraph of 6.1, skimmed through the remainder and also read section 9 (though not 9.1 and 9.2).
If I've fully understood everything, then I don't think the larger attacks described in section 6 and prepared in section 5 can actually be applied to Mumble (that's the protocol, not just this implementation).

In particular, while the basic attacks in section 4 seem to apply almost trivially to Mumble (they don't seem particularly useful though), the scheme described in 5.2 to extract specific blockcipher mappings (which all of the later sections build on), appears to require an encryption oracle which will perform encryptions for the attacker with an attacker-chosen nonce.
In Mumble however, the nonce cannot really be controlled by the attacker (that is, it's pre-determined by the server and can only be incremented by somehow causing a packet to be sent).
So, given the attacker needs to use one of the nonces they got in section 5.1 (the first one has already been used, the others are effectively random), the chance of that nonce being the one which Mumble would next use are negligible.

While I've said above that the basic attacks on their own don't appear particularly useful, the paper does mention that they break some assumptions of OCB2 (see the sentence in parenthesise in the last paragraph of section 5).
So, to be on the safe side and prevent even the basic attacks, one should additionally implement the following suggestion from section 9 (screenshot so formatting isn't messed up), which, while not having a formal proof, will at least make the attacks as presented in the paper impossible (we'd still want to eventually move away from OCB2 ofc since we can't be sure there aren't other attacks):


In simple terms:

  1. Never encrypt any message where the second to last block starts (or ends? don't recall how len was actually implemented in OCB2. be sure not to mess this one up! if unsure, just do both) with 120 bits of zeros.
  2. Ignore any decrypted message which we would not have encrypted according to 1.

Given we mostly send compressed audio over UDP, it seems unlikely that there would ever legitimately be 120 bits of zeros, so impact should be minimal. Ping packets are probably only a single block in size, so they're not affected either.
Removing either encryption or decryption oracle will make the attacks impossible. And because a Mumble client never decrypts its own packets, if we take away both on the server, even un-patched clients should be safe (as long as they're connected to a patched server) and vice-versa (though ofc the other way around all the participating clients would have to be patched if the server isn't since any one of them could leak the full conversation).

The question is if this is preferable for all (or most) use cases. Some users might prefer better audio/streaming performance over perfect encryption and are talking on an public channel anyway. AFAIK OCB2 still obfuscates the content pretty well and the paper talks about active attacks.

As far as I understand the paper, you're correct. Applied to Mumble, the attack would require the attacker to connect to the server and be able to either listen to the victim (or rather their own forged packets which to the server look like they came from the victim) or send them audio (though short whispers can do that), depending on the direction they wish to attack. Disregarding what I wrote above, they would then be able to decrypt past (within the same session) but not future communication of that user.

From the linked paper:

This observation immediately suggests a fix: If the involved users agree that all encryption/decryption operations are with respect to a non-empty AD, then it seems(to us) that all problems go away.

Does somebody with more background in cryptography than me (maybe @Avatat ) understand what this means for Mumble? Can we simply apply that fix?

Mumble does not use (or even implement) OCB2's authenticated header feature (which is what this paper refers to with the more general term of associated data, short AD), so using a non-empty AD would be equivalent to using a completely different cipher as far as old clients are concerned.

@Krzmbrzl
Copy link
Member

@Johni0702 thank you very much for your insight on this matter!

If this really works, then I think we can simply implement the fix on the server. The question is: Where is the actual OCB2 encryption/decryption even happening? 🤔
And how do we check the second last block? Aka: Does Mumble even operate on a per-block basis or is this simply handed to some library that does this for us?

@streaps
Copy link
Author

streaps commented May 30, 2020

void CryptState::ocb_encrypt(const unsigned char *plain, unsigned char *encrypted, unsigned int len, const unsigned char *nonce, unsigned char *tag) {

@Johni0702
Copy link
Contributor

Johni0702 commented May 30, 2020

If this really works, then I think we can simply implement the fix on the server. The question is: Where is the actual OCB2 encryption/decryption even happening? thinking
And how do we check the second last block? Aka: Does Mumble even operate on a per-block basis or is this simply handed to some library that does this for us?

See streaps reply for where the encrypt and decrypt methods are located.
The last block is the more complicated one (cause it uses padding) after the while loop. The second-to-last block would be the last iteration of the loop.

Mumble's equivalent of the blocks in the paper is the keyblock type. It is always 128 bits of data.
Mumble further breaks those down into subblocks (purely an implementation detail) which are either 64 or 32 bits depending on your CPU architecture.

For the LEN(n) (it was lowercase in the paper but I'm using caps here to disambiguate it from Mumble's variables) function from the paper, Mumble uses ZERO(tmp); followed by tmp[BLOCKSIZE - 1] = SWAPPED(len * 8); (len * 8 being the n). The attack works by using a plaintext in the second to last block which matches the output of LEN(n) for any valid n. So, if we merely ensure that at least one of the first 120 bits of the plaintext in the second to last block is set, then mounting the described attack becomes impossible.

Edit (clarification): I just noticed that the type of tmp is keyblock which is subblock[], so the tmp[BLOCKSIZE - 1] doesn't actually index the last byte as I assumed it would but the whole last subblock. Given SWAPPED is basically just encoding the number as big-endian and the number can at most be 128, the result is the same though.
Side note: For 32-bit hosts Mumble uses htonl which depends on the host byte-order. For 64-bit however it doesn't. That looks like a bug which will result in 64-bit big-endian hosts being unable to communicate with all the others, since in that case Mumble still swaps the bytes producing a little-endian encoding.

@streaps
Copy link
Author

streaps commented May 31, 2020

Thanks for the valuable discussion and explanations. I still don't understand everything, but my impression is that the vulnerability cannot be exploited in Mumble, because of the data Mumble sends. There is also a simple mitigation, but I'm not sure if that is really needed or if that would be more of a precaution.

Should we remove the the P1 - Critical label?

@Krzmbrzl
Copy link
Member

but I'm not sure if that is really needed or if that would be more of a precaution.

I think as a precaution, we should implement it, if possible

@Johni0702 thank you very much for your explanation!
Since you seem to know what you're talking about, could you be motivated to create a PR and implement the suggested fix / precaution.
And in a second commit maybe also address the 64bit byte-order bug you mentioned ☝️

@streaps
Copy link
Author

streaps commented May 31, 2020

Considering the situation that Mumble uses a version of OCB2 that is not compatible with regular OCB implementations, I hope there is an easy path to switch to some standardized crypto soon.

I was going to write a simple audio client for mumble, but before I wrote one line of code I was hit by the OCB2 issue. It would be really nice, if we could just use some common crypto library (like OpenSSL, Botan, PyCryptodome) for UDP encryption too.

I still think AES128-OCB (as in OCB3 / RFC 7253) is a good choice and it is supported by OpenSSL. It was one of the two winners in the CAESAR competition for the high performance use case (https://competitions.cr.yp.to/caesar-submissions.html).

The proposal from @TerryGeng sounds simple and flexible. It doesn't seem to be too difficult to offer additional standard ciphers that are supported by most crypto libs – like AES-GCM or ChaCha20-Poly1305.

AEGIS is not the best option for a OCB2 replacement or a default codec. Support in the distributions is just not there yet. It could be a built option though.

@Krzmbrzl
Copy link
Member

Krzmbrzl commented Jun 1, 2020

As far as the flexibility is concerned, Terry said he was working on it and even if not: That's something we want to add.

And the rest of your post @streaps is OT here and should be posted in #3918

@Krzmbrzl
Copy link
Member

Krzmbrzl commented Jun 1, 2020

@Johni0702 provided the above mentioned fix/mitigation in #4227 (big thanks for that!).
As there seemed to be a few folks here that know crypto stuff a lot better than me, could you (all) go and have a look at the changes and check if you agree that this fixes the issue? That'd be awesome!

Krzmbrzl added a commit that referenced this issue Jun 6, 2020
As explained in #4219 (comment):

    In simple terms:

    Never encrypt any message where the second to last block starts (or ends? don't recall how len was actually implemented in OCB2. be sure not to mess this one up! if unsure, just do both) with 120 bits of zeros.
    Ignore any decrypted message which we would not have encrypted according to 1.

    Given we mostly send compressed audio over UDP, it seems unlikely that there would ever legitimately be 120 bits of zeros, so impact should be minimal. Ping packets are probably only a single block in size, so they're not affected either.
    Removing either encryption or decryption oracle will make the attacks impossible. And because a Mumble client never decrypts its own packets, if we take away both on the server, even un-patched clients should be safe (as long as they're connected to a patched server) and vice-versa (though ofc the other way around all the participating clients would have to be patched if the server isn't since any one of them could leak the full conversation).

The respective paper can be found at https://eprint.iacr.org/2019/311
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants