DarkSingularity/orbithub
Internal
Public Access
0
0
Fork 0
Code Issues 1 Pull Requests Packages Projects Releases 7 Wiki Activity

Fix Apple DH auth wire-order bug, add type 33 fallback, fix missing VNC password prompt

Browse Source 
Live testing against a real macOS Screen Sharing server surfaced two
real bugs, independent of each other:

1. SessionTab::requestConnectOptions() never prompted for a password
   on VNC profiles (only SSH/RDP) -- every VNC connection went out
   with an empty password regardless of what the server needed. VNC
   now gets its own prompt; an empty password is allowed through
   (unlike RDP's hard requirement) since no-auth VNC servers exist and
   there's no way to know client-side before the security-type
   negotiation happens.

2. VncSessionBackend's Apple DH (type 30) response sent the client's
   public key before the encrypted credentials. Cross-checking against
   neatvnc's rfb-proto.h (an independent, authoritative reference: both
   the wire struct definitions and the full server-side verification
   code, matched field-by-field against this implementation) showed
   the correct order is credentials first, then public key -- exactly
   backwards from what was implemented. Fixed, with a new regression
   test that decrypts the credentials back out using the trailing
   public-key bytes to derive the shared secret, which would fail if
   the fields were swapped again.

Also adds security type 33 (RSA + AES, src/vnc_apple_rsa_auth.h) as a
fallback Apple auth scheme, sourced from the `asyncvnc` PyPI package.
Preference when multiple are offered: None > AppleDH(30) >
AppleRSA(33) > VNCAuth(2).

Neither scheme has been gotten working live yet against the specific
macOS Tahoe (26.6.2) server available for testing -- type 30's wire
format is now verified correct byte-for-byte against the independent
reference above, but the server still rejects it with a generic
"Authentication or authorization failure"; type 33 is rejected even
earlier, right after the initial host-key request. macOS Tahoe was
released after this assistant's knowledge cutoff, so there may be a
protocol or permission-model change specific to it that isn't
reflected in either reference. Documented as an open issue in
docs/PROGRESS.md rather than claimed as working.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
ksmith
2026-09-16 03:49:06 -06:00
co-authored by Claude Sonnet 5
parent df2b1a8d50
commit e80fe7d634
9 changed files with 511 additions and 76 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/CMakeLists.txt
+1
View File
@@ -31,6 +31,7 @@ add_executable(test_vnc_session_backend
${CMAKE_SOURCE_DIR}/src/vnc_session_backend.cpp
${CMAKE_SOURCE_DIR}/src/vnc_pixel_codecs.cpp
${CMAKE_SOURCE_DIR}/src/vnc_apple_dh_auth.cpp
${CMAKE_SOURCE_DIR}/src/vnc_apple_rsa_auth.cpp
${CMAKE_SOURCE_DIR}/src/session_backend.h
)
target_include_directories(test_vnc_session_backend PRIVATE ${CMAKE_SOURCE_DIR}/src)
tests/test_vnc_session_backend.cpp
+123 -7
View File
@@ -334,7 +334,9 @@ ToyDhKeypair generateToyDhKeypair()
const int primeLen = BN_num_bytes(p);
result.primeBytes = QByteArray(primeLen, char(0));
BN_bn2binpad(p, reinterpret_cast<unsigned char*>(result.primeBytes.data()), primeLen);
result.generatorBytes = QByteArray(1, char(2));
// The real wire format's generator field is a literal 2 bytes (see
// appleDhAuthServerMessage()), so keep this the same size here too.
result.generatorBytes = QByteArray::fromHex("0002");
result.publicKeyBytes = QByteArray(primeLen, char(0));
BN_bn2binpad(pub, reinterpret_cast<unsigned char*>(result.publicKeyBytes.data()), primeLen);
result.privateExponent = priv;
@@ -346,14 +348,17 @@ ToyDhKeypair generateToyDhKeypair()
return result;
}
// A FramebufferUpdate-adjacent helper isn't needed here -- these bytes are
// sent immediately after the client selects security type 30, before
// ClientInit/ServerInit even happen.
// The bytes a real Apple Screen Sharing server sends immediately after the
// client selects security type 30, before ClientInit/ServerInit even
// happen. Format confirmed empirically against a real macOS server: a
// literal 2-byte generator (NOT length-prefixed -- there is no separate
// generator-length field, unlike a first guess at this scheme might
// assume), then a 2-byte key length applying to both the prime and the
// server's public key that follow.
QByteArray appleDhAuthServerMessage(const QByteArray& generator, const QByteArray& prime,
const QByteArray& serverPublicKey)
{
QByteArray msg;
appendU16(msg, static_cast<quint16>(generator.size()));
msg += generator;
appendU16(msg, static_cast<quint16>(prime.size()));
msg += prime;
@@ -414,6 +419,7 @@ private slots:
void appleDhAuthRoundTripDecryptsToOriginalCredentials();
void connectsWithAppleDhAuthenticationRfb38();
void appleDhAuthIsPreferredOverVncAuthWhenBothOffered();
void appleDhAuthAcceptsRealCapturedMacOsServerParameters();
private:
std::unique_ptr<FakeVncServer> m_server;
@@ -1995,6 +2001,8 @@ void TestVncSessionBackend::connectsWithAppleDhAuthenticationRfb38()
ToyDhKeypair serverKeypair = generateToyDhKeypair();
const QByteArray authMessage = appleDhAuthServerMessage(
serverKeypair.generatorBytes, serverKeypair.primeBytes, serverKeypair.publicKeyBytes);
const int keyLength = serverKeypair.primeBytes.size();
const QString password = QStringLiteral("s3cret-pass");
connect(m_server.get(), &FakeVncServer::clientConnected, this, [this]() {
m_server->sendWhenConnected(QByteArray("RFB 003.008\n"));
@@ -2011,7 +2019,7 @@ void TestVncSessionBackend::connectsWithAppleDhAuthenticationRfb38()
case 1: // security-type selection (byte value 30)
m_server->sendWhenConnected(authMessage);
break;
case 2: // client's DH public key + encrypted credentials -- accept unconditionally
case 2: // client's response -- accept unconditionally, checked below
m_server->sendWhenConnected(QByteArray(4, char(0))); // SecurityResult: OK
break;
case 3: { // ClientInit
@@ -2028,9 +2036,62 @@ void TestVncSessionBackend::connectsWithAppleDhAuthenticationRfb38()
}
});
m_backend->connectSession(makeOptions(QStringLiteral("irrelevant-password")));
m_backend->connectSession(makeOptions(password));
QTRY_COMPARE(m_lastState, SessionState::Connected);
// Verify wire order: encrypted credentials (128 bytes) MUST come
// before the client's public key, per neatvnc's authoritative
// rfb_apple_dh_client_msg struct layout (this is exactly the ordering
// bug that made real-world testing fail before it was found and
// fixed). Proven by actually deriving the shared secret from the
// trailing public-key bytes -- as the server would -- and decrypting
// the leading 128 bytes with it; a swap would make this decrypt to
// garbage instead of the real credentials.
// 12 bytes for the client's version-reply line, 1 byte for its
// security-type selection, precede the response itself; more bytes
// (ClientInit, SetPixelFormat, ...) may already follow it by the time
// Connected is observed, so slice by offset rather than assuming
// total size.
const int responseOffset = 13;
QVERIFY(m_server->received.size() >= responseOffset + 128 + keyLength);
const QByteArray encryptedCredentials = m_server->received.mid(responseOffset, 128);
const QByteArray clientPublicKeyBytes =
m_server->received.mid(responseOffset + 128, keyLength);
BIGNUM* clientPub = BN_bin2bn(
reinterpret_cast<const unsigned char*>(clientPublicKeyBytes.constData()),
clientPublicKeyBytes.size(), nullptr);
BN_CTX* ctx = BN_CTX_new();
BIGNUM* sharedSecret = BN_new();
BN_mod_exp(sharedSecret, clientPub, serverKeypair.privateExponent, serverKeypair.prime, ctx);
QByteArray secretBytes(keyLength, char(0));
BN_bn2binpad(sharedSecret, reinterpret_cast<unsigned char*>(secretBytes.data()), keyLength);
unsigned char aesKey[16];
EVP_Digest(secretBytes.constData(), static_cast<size_t>(secretBytes.size()), aesKey, nullptr,
EVP_md5(), nullptr);
EVP_CIPHER_CTX* decCtx = EVP_CIPHER_CTX_new();
EVP_DecryptInit_ex(decCtx, EVP_aes_128_ecb(), nullptr, aesKey, nullptr);
EVP_CIPHER_CTX_set_padding(decCtx, 0);
QByteArray plain(encryptedCredentials.size() + 16, char(0));
int outLen1 = 0;
EVP_DecryptUpdate(decCtx, reinterpret_cast<unsigned char*>(plain.data()), &outLen1,
reinterpret_cast<const unsigned char*>(encryptedCredentials.constData()),
encryptedCredentials.size());
int outLen2 = 0;
EVP_DecryptFinal_ex(decCtx, reinterpret_cast<unsigned char*>(plain.data()) + outLen1, &outLen2);
plain.resize(outLen1 + outLen2);
EVP_CIPHER_CTX_free(decCtx);
QByteArray expected(128, char(0));
const QByteArray passBytes = password.toLatin1().left(64);
std::memcpy(expected.data() + 64, passBytes.constData(), static_cast<size_t>(passBytes.size()));
QCOMPARE(plain, expected);
BN_free(clientPub);
BN_free(sharedSecret);
BN_CTX_free(ctx);
BN_free(serverKeypair.privateExponent);
BN_free(serverKeypair.prime);
}
@@ -2056,5 +2117,60 @@ void TestVncSessionBackend::appleDhAuthIsPreferredOverVncAuthWhenBothOffered()
QCOMPARE(static_cast<quint8>(m_server->received.at(0)), quint8(30));
}
void TestVncSessionBackend::appleDhAuthAcceptsRealCapturedMacOsServerParameters()
{
// The exact generator/prime/server-public-key bytes captured from a
// real macOS Screen Sharing server's security-type-30 auth message
// (generator=5, a 512-byte/4096-bit RFC 3526 Group 16-family prime,
// and its server public key) -- this is what caught the original
// implementation's wrong assumption that the generator was itself
// length-prefixed like the prime is; it wasn't (it's a literal 2
// bytes), and this pins that fix against the real bytes that exposed
// the bug rather than only against freshly-generated toy data.
const QByteArray captured = QByteArray::fromHex(
"00050200"
"ffffffffffffffffc90fdaa22168c234c4c6628b80dc1cd129024e088a67cc74020bbea63b139b22514a0"
"8798e3404ddef9519b3cd3a431b302b0a6df25f14374fe1356d6d51c245e485b576625e7ec6f44c42e9a6"
"37ed6b0bff5cb6f406b7edee386bfb5a899fa5ae9f24117c4b1fe649286651ece45b3dc2007cb8a163bf0"
"598da48361c55d39a69163fa8fd24cf5f83655d23dca3ad961c62f356208552bb9ed529077096966d670c"
"354e4abc9804f1746c08ca18217c32905e462e36ce3be39e772c180e86039b2783a2ec07a28fb5c55df06"
"f4c52c9de2bcbf6955817183995497cea956ae515d2261898fa051015728e5a8aaac42dad33170d04507a"
"33a85521abdf1cba64ecfb850458dbef0a8aea71575d060c7db3970f85a6e1e4c7abf5ae8cdb0933d71e8"
"c94e04a25619dcee3d2261ad2ee6bf12ffa06d98a0864d87602733ec86a64521f2b18177b200cbbe11757"
"7a615d6c770988c0bad946e208e24fa074e5ab3143db5bfce0fd108e4b82d120a92108011a723c12a787e"
"6d788719a10bdba5b2699c327186af4e23c1a946834b6150bda2583e9ca2ad44ce8dbbbc2db04de8ef92e"
"8efc141fbecaa6287c59474e6bc05d99b2964fa090c3a2233ba186515be7ed1f612970cee2d7afb81bdd7"
"62170481cd0069127d5b05aa993b4ea988d8fddc186ffb7dc90a6c08f4df435c934063199"
"ffffffffffffffff"
"91c11c23a5b54389a05127d7fa94e681b7884667d770cd646fa46583d73e0a5ad09e361c30a0f9a9b765a"
"b291e2b8a13ba0088ba4bdb4a838120e649d49ed78db7fdb3397b907d1a42c0531d16796229d7b3a5a369"
"bfc639771ab1dea093ca7756b2b4a4a98f296478b331538533faead12342d6f1fe92817690ae8e2c51cbf"
"dc96c819e480c2170e6be12149691cb2bab218528c40cc5729b8a303bbe95d77eba288f3c4137d316c5b1"
"d9004c8fe3d034b1284d39e57133972700d1933465b4cc656d2dcbef67dc7eeb464f197bd5187abce304b"
"c972768dcaaa703e287f4f1e375c287b474fe0e3df9897a24bdaf06466976becbc8908fd81211537d1a44"
"e63e705d69b5c90f375fa717c978b27a21384769da67cd97e5134f71aef420dd4b36a2c4e05f80cee043f"
"ab873b478804dc126495d35f0a1df041c4b2bf0926e35ae033f6f6808bdc8a5792b7d380e7c7fc95fc80c"
"fea44c92842ed75757230f590b7dd1d9beadbabb78a96c47f21e65f6ea8566820ff9c59d078870fa1d367"
"2ff0b7f9a3d600f3479232e533bc8b64a3909fd61e070b19ac98d50a7c2a33c885294183e82a4e7777f2a"
"423002219a6094fdf51651d48550f771ede87eb95592eda604b85058cd0d8db9a92670e6e0d74400f7cd4"
"b48c8dfbace5c92c39775a60bb4e59d70878a50b609b15ae4a930ee5eb0494902d44869089349fa2ab343"
"028e");
QCOMPARE(captured.size(), 1028);
const QByteArray generator = captured.left(2);
const int keyLength = (static_cast<quint8>(captured.at(2)) << 8) | static_cast<quint8>(captured.at(3));
QCOMPARE(keyLength, 512);
const QByteArray prime = captured.mid(4, keyLength);
const QByteArray serverPublicKey = captured.mid(4 + keyLength, keyLength);
QCOMPARE(prime.size(), 512);
QCOMPARE(serverPublicKey.size(), 512);
const VncAppleDhAuth::Response response = VncAppleDhAuth::computeResponse(
generator, prime, serverPublicKey, QStringLiteral("tester"), QStringLiteral("s3cret"));
QVERIFY(!response.clientPublicKey.isEmpty());
QCOMPARE(response.clientPublicKey.size(), 512);
QCOMPARE(response.encryptedCredentials.size(), 128);
}
QTEST_GUILESS_MAIN(TestVncSessionBackend)
#include "test_vnc_session_backend.moc"