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

configured sslciphers not respected. #6465

Open
eebssk1 opened this issue Jun 12, 2024 · 9 comments
Open

configured sslciphers not respected. #6465

eebssk1 opened this issue Jun 12, 2024 · 9 comments
Labels
bug A bug (error) in the software good first issue Good for first-time contributors server

Comments

@eebssk1
Copy link

eebssk1 commented Jun 12, 2024

The issue

I configed sslCiphers=DHE-RSA-CHACHA20-POLY1305 in server ini to disable AES.
However when starting server the following indicating it's not respected and clients still connecting with AES encryption.

2024-06-12 13:01:34.054 MetaParams: TLS cipher preference is "TLS_AES_256_GCM_SHA384:TLS_CHACHA20_POLY1305_SHA256:TLS_AES_128_GCM_SHA256:DHE-RSA-CHACHA20-POLY1305"

Mumble version

1.5.634

Mumble component

Server

OS

Linux

Additional information

No response

@eebssk1
Copy link
Author

eebssk1 commented Jun 13, 2024

I had to clear tls1.3 and unintended ciphers in openssl.cnf to make it work.

@Krzmbrzl
Copy link
Member

openssl.cnf as in the OpenSSL config file? The intended way is that this is not required 🤔

Does the Mumble config setting (without changed OpenSSL config) come into effect when you create a fresh server (that doesn't reuse the old's database)?

@eebssk1
Copy link
Author

eebssk1 commented Jun 13, 2024

openssl.cnf as in the OpenSSL config file? The intended way is that this is not required 🤔

Does the Mumble config setting (without changed OpenSSL config) come into effect when you create a fresh server (that doesn't reuse the old's database)?

So I digged a little.

It looks like official client does not support chacha20 in (EC)DHE mode. And it seems the sslciphers config options does not accept ciphersuit name.
Which means the only way for me is to remove the AES one from openssl.cnf ciphersuit so chacha20 one is the prefered.
Maybe it's better to seprate the config options for TLS1.3 and TLS1.2_and_older, as indicated by openssl that they are unrelated.

@github-actions github-actions bot added the stale-support This issue hasn't seen any activity in recent time and will probably be closed soon. label Jun 18, 2024
@mumble-voip mumble-voip deleted a comment from github-actions bot Jun 18, 2024
@Krzmbrzl Krzmbrzl removed the stale-support This issue hasn't seen any activity in recent time and will probably be closed soon. label Jun 18, 2024
@eebssk1
Copy link
Author

eebssk1 commented Jun 18, 2024

So for clafirication. I'm still using TLS 1.3 but with only chacha20 one. I removed AES ciphersuit from openssl.cnf since mumble config does not regconise ciphersuit name which means i can not set the preference there.

@github-actions github-actions bot added the stale-support This issue hasn't seen any activity in recent time and will probably be closed soon. label Jun 23, 2024
@mumble-voip mumble-voip deleted a comment from github-actions bot Jun 23, 2024
@Krzmbrzl Krzmbrzl removed the stale-support This issue hasn't seen any activity in recent time and will probably be closed soon. label Jun 23, 2024
@github-actions github-actions bot added the stale-support This issue hasn't seen any activity in recent time and will probably be closed soon. label Jun 28, 2024
@mumble-voip mumble-voip deleted a comment from github-actions bot Jun 28, 2024
@Krzmbrzl Krzmbrzl removed the stale-support This issue hasn't seen any activity in recent time and will probably be closed soon. label Jun 28, 2024
@github-actions github-actions bot added the stale-support This issue hasn't seen any activity in recent time and will probably be closed soon. label Jul 3, 2024
@mumble-voip mumble-voip deleted a comment from github-actions bot Jul 3, 2024
@Krzmbrzl Krzmbrzl removed the stale-support This issue hasn't seen any activity in recent time and will probably be closed soon. label Jul 3, 2024
@Krzmbrzl
Copy link
Member

Krzmbrzl commented Jul 6, 2024

When using the sslciphers config option, you should get a message like MetaParams: TLS cipher preference is ... when starting up the server. Does this message agree with what you have configured in the INI file?

'cause what is printed there should also be used for the actual connections if I read the code correctly 🤔

@eebssk1
Copy link
Author

eebssk1 commented Jul 6, 2024

When using the sslciphers config option, you should get a message like MetaParams: TLS cipher preference is ... when starting up the server. Does this message agree with what you have configured in the INI file?

'cause what is printed there should also be used for the actual connections if I read the code correctly 🤔

Server won't startup. The TLS1.3 cipher string is not recognized by the option.

@eebssk1
Copy link
Author

eebssk1 commented Jul 6, 2024

When using the sslciphers config option, you should get a message like MetaParams: TLS cipher preference is ... when starting up the server. Does this message agree with what you have configured in the INI file?
'cause what is printed there should also be used for the actual connections if I read the code correctly 🤔

Server won't startup. The TLS1.3 cipher string is not recognized by the option.

It's very weired that MetaParams automatically prepend tls1.3 ciphers,but the options does not recognize it when manually entered.

@Krzmbrzl Krzmbrzl added server bug A bug (error) in the software good first issue Good for first-time contributors and removed support labels Jul 6, 2024
@Krzmbrzl Krzmbrzl changed the title configed sslciphers not respected. configured sslciphers not respected. Jul 6, 2024
@Krzmbrzl
Copy link
Member

Krzmbrzl commented Jul 6, 2024

The appending of additional ciphers is actually done by OpenSSL, it seems. The cipher list entering

mumble/src/SSL.cpp

Lines 49 to 108 in 9f0b143

QList< QSslCipher > MumbleSSL::ciphersFromOpenSSLCipherString(QString cipherString) {
QList< QSslCipher > chosenCiphers;
SSL_CTX *ctx = nullptr;
SSL *ssl = nullptr;
const SSL_METHOD *meth = nullptr;
int i = 0;
QByteArray csbuf = cipherString.toLatin1();
const char *ciphers = csbuf.constData();
meth = SSLv23_server_method();
if (!meth) {
qWarning("MumbleSSL: unable to get SSL method");
goto out;
}
// We use const_cast to be compatible with OpenSSL 0.9.8.
ctx = SSL_CTX_new(const_cast< SSL_METHOD * >(meth));
if (!ctx) {
qWarning("MumbleSSL: unable to allocate SSL_CTX");
goto out;
}
if (!SSL_CTX_set_cipher_list(ctx, ciphers)) {
qWarning("MumbleSSL: error parsing OpenSSL cipher string in ciphersFromOpenSSLCipherString");
goto out;
}
ssl = SSL_new(ctx);
if (!ssl) {
qWarning("MumbleSSL: unable to create SSL object in ciphersFromOpenSSLCipherString");
goto out;
}
while (1) {
const char *name = SSL_get_cipher_list(ssl, i);
if (!name) {
break;
}
#if QT_VERSION >= 0x050300
QSslCipher c = QSslCipher(QString::fromLatin1(name));
if (!c.isNull()) {
chosenCiphers << c;
}
#else
foreach (const QSslCipher &c, QSslSocket::supportedCiphers()) {
if (c.name() == QString::fromLatin1(name)) {
chosenCiphers << c;
}
}
#endif
++i;
}
out:
SSL_CTX_free(ctx);
SSL_free(ssl);
return chosenCiphers;
}

is the one from the INI file but the one leaving this function contains the extra ciphers.

@eebssk1
Copy link
Author

eebssk1 commented Oct 10, 2024

According to https://manpages.debian.org/testing/libssl-doc/OPENSSL_config.3ssl.en.html
Maybe we could set OPENSSL_CONF to load the modded conf instead of loading/modifying the global one.
Since the function will always called by qt dependencies(it seems), this way might work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A bug (error) in the software good first issue Good for first-time contributors server
Projects
None yet
Development

No branches or pull requests

2 participants