From 0c575ed8c9d2eafad112c3f8ae246b74dc3d6b4c Mon Sep 17 00:00:00 2001 From: Maurice ONeal Date: Sun, 5 Apr 2020 19:07:08 -0400 Subject: [PATCH] Updated SSL Handling - The client will now display all ssl errors that occur during the handshake process properly. - New versions of MRCI host will longer support multiple certs so the the common name designator was removed from the client header. It is now just 272bytes of padding reserved for future expansion. - Fixed the defined server header len from 35 to 37. - The client will now respond to only reply 1 or 2 on the host header. - HOST_CERT was removed as a data type and the host will longer use it. --- docs/README.md | 19 +++------ docs/type_ids.md | 16 +++---- src/cmd_objs/exec.cpp | 4 -- src/common.h | 3 +- src/session.cpp | 99 +++++++++++++++++++++++-------------------- src/session.h | 3 +- 6 files changed, 68 insertions(+), 76 deletions(-) diff --git a/docs/README.md b/docs/README.md index d4d114e..7913457 100644 --- a/docs/README.md +++ b/docs/README.md @@ -44,20 +44,18 @@ notes: ### Client Header (This Application) ### ``` -[tag][appName][coName] +[tag][appName][padding] tag - 4bytes - 0x4D, 0x52, 0x43, 0x49 (MRCI) appName - 134bytes - UTF16LE string (padded with 0x00) -coName - 272bytes - UTF16LE string (padded with 0x00) +padding - 272bytes - padding of 0x00 bytes reserved for future expansion ``` notes: * The **tag** is just a fixed ascii string "MRCI" that indicates to the host that the client is indeed attempting to use the MRCI protocol. -* The **appName** is the name of the client application that is connected to the host. It can also contain the client's app version if needed because it doesn't follow any particular standard. - -* The **coName** is the common name of a SSL certificate that is currently installed in the host. Depending on how the host is configured, it can contain more than one installed SSL cert so coName can be used by clients as a way to request which one of the SSL certs to use during the SSL handshake. If the client doesn't know which cert to request, it is good practice to use the address that was used to connect to the host. +* The **appName** is the name of the client application that is connected to the host. It can also contain the client's app version if needed because it doesn't follow any particular standard. This string is accessable to all modules so the commands themselves can be made aware of what app the user is currently using. ### Host Header ### @@ -76,15 +74,12 @@ sesId - 28bytes - 224bit sha3 hash notes: -* **reply** is a numeric value that the host returns in it's header to communicate to the client the result of it's evaluation of the client's header. +* **reply** is a numeric value that the host returns in it's header to communicate to the client if SSL need to initated or not. - * reply = 1, means the client is acceptable and it does not need to take any further action. - * reply = 2, means the client is acceptable but the host will now send it's Pem formatted SSL cert data in a ```HOST_CERT``` mrci frame just after sending it's header. After receiving the cert, the client will then need to send a STARTTLS signal using this cert. - * reply = 4, means the host was unable to find the SSL cert associated with the common name sent by the client. The session will auto close at this point. + * reply = 1, means SSL is not required so the client doesn't need to take any further action. + * reply = 2, means SSL is required to continue so the client needs to send a STARTLS signal. -* **major**, **minor**, **tcp_rev**, **mod_rev** these 4 numeric values are the host version number that uses a 4 number versioning system. This can be used by the client to setup backwards compatibility or determine if it supports the host at all. If not supported, the client can simply disconnect from the host and display an error to the user. - -* **sesId** is the session id. It is a unique 224bit sha3 hash that can be used by the host and client to uniquely identify the current session or past sessions. +* **sesId** is the session id. It is a unique 224bit sha3 hash generated against the current date and time of session creation (down to the msec) and the machine id of the host. This can be used by the host or client to uniquely identify the current session or past sessions. ### Async Commands ### diff --git a/docs/type_ids.md b/docs/type_ids.md index c4ff2d7..cee2ed3 100644 --- a/docs/type_ids.md +++ b/docs/type_ids.md @@ -10,7 +10,6 @@ enum TypeID : quint8 ERR = 3, PRIV_TEXT = 4, IDLE = 5, - HOST_CERT = 6, FILE_INFO = 7, PEER_INFO = 8, MY_INFO = 9, @@ -113,10 +112,7 @@ This works similarly to TERM_CMD except it will also terminate the module proces This type id doesn't carry any actual data, instead can be used to tell the host to pause/yield the command id and branch id that was used to send it. All modules are not obligated to support this feature but it is highly recommended to do so. ```RESUME_CMD``` -This is the other half of YIELD_CMD that tells the host to resume the command that was running. - -```HOST_CERT``` -Just as the name implies, this data type is used by the host to send the host SSL certificate while setting up an SSL connection. +This is the other half of YIELD_CMD that tells the host to resume the command that was running. ```HOST_VER``` This data structure carries 4 numeric values that represent the host version as described in section [1.3](protocol.md). @@ -176,8 +172,8 @@ This carry some user account and session information about a peer client connect 1. bytes[0-27] 28bytes - session id (224bit hash) 2. bytes[28-59] 32bytes - user id (256bit hash) 3. bytes[60-107] 48bytes - user name (TEXT - padded with 0x00) - 4. bytes[108-235] 128bytes - app name (TEXT - padded with 0x00) - 5. bytes[236-299] 64bytes - disp name (TEXT - padded with 0x00) + 4. bytes[108-241] 134bytes - app name (TEXT - padded with 0x00) + 5. bytes[242-305] 64bytes - disp name (TEXT - padded with 0x00) notes: 1. the session id is unique to the peer's session connection only. it @@ -201,9 +197,9 @@ This contains all of the information found in ```PEER_INFO``` for the local sess ``` format: - 1. bytes[300-427] 128bytes - email (TEXT - padded with 0x00) - 2. bytes[428-431] 4bytes - host rank (32bit unsigned int) - 3. bytes[432] 1byte - is email confirmed? (0x00 false, 0x01 true) + 1. bytes[306-433] 128bytes - email (TEXT - padded with 0x00) + 2. bytes[434-437] 4bytes - host rank (32bit unsigned int) + 3. bytes[438] 1byte - is email confirmed? (0x00 false, 0x01 true) ``` ```NEW_CMD``` diff --git a/src/cmd_objs/exec.cpp b/src/cmd_objs/exec.cpp index dda9105..ee70ba0 100644 --- a/src/cmd_objs/exec.cpp +++ b/src/cmd_objs/exec.cpp @@ -130,10 +130,6 @@ void Connect::dataIn(const QString &argsLine) { cacheTxt(ERR, "err: Host address is empty.\n"); } - else if (QHostAddress(*Shared::hostAddress).isNull()) - { - cacheTxt(ERR, "err: '" + *Shared::hostAddress + "' is not a valid address.\n"); - } else { emit connectToHost(); diff --git a/src/common.h b/src/common.h index cb6e33a..8df54f8 100644 --- a/src/common.h +++ b/src/common.h @@ -76,7 +76,7 @@ #define CONFIG_FILENAME "config_v3.json" #define APP_NAME "Cmdr" #define APP_TARGET "cmdr" -#define APP_VERSION "3.2" +#define APP_VERSION "3.3" enum TypeID : quint8 { @@ -85,7 +85,6 @@ enum TypeID : quint8 ERR = 3, PRIV_TEXT = 4, IDLE = 5, - HOST_CERT = 6, FILE_INFO = 7, PEER_INFO = 8, MY_INFO = 9, diff --git a/src/session.cpp b/src/session.cpp index 4de2d2b..b14a4b4 100644 --- a/src/session.cpp +++ b/src/session.cpp @@ -36,6 +36,7 @@ Session::Session(QObject *parent) : QSslSocket(nullptr) connect(this, &Session::loopDataIn, this, &Session::dataIn); connect(this, SIGNAL(error(QAbstractSocket::SocketError)), this, SLOT(sockerr(QAbstractSocket::SocketError))); + connect(this, SIGNAL(sslErrors(QList)), this, SLOT(sslErrs(QList))); progResetDelay->setInterval(200); progResetDelay->setSingleShot(true); @@ -82,7 +83,14 @@ void Session::sockerr(QAbstractSocket::SocketError err) } else { - cacheTxt(ERR, "\nerr: " + errorString() + "\n"); + if (sslErrors().isEmpty()) + { + // a non-empty sslErrors() can assume the errors were already displayed + // by the sslErrs() slot so this conditional block is here to prevent + // the same error from getting displayed twice. + + cacheTxt(ERR, "\nerr: " + errorString() + "\n"); + } if (state() == QAbstractSocket::UnconnectedState) { @@ -91,18 +99,40 @@ void Session::sockerr(QAbstractSocket::SocketError err) } } +void Session::sslErrs(const QList &errors) +{ + cacheTxt(ERR, "\n"); + + auto canIgnore = true; + + for (auto err : errors) + { + if ((err.error() == QSslError::SelfSignedCertificate) || + (err.error() == QSslError::SelfSignedCertificateInChain)) + { + cacheTxt(TEXT, "WARNING: the host cert is self signed.\n\n"); + } + else + { + canIgnore = false; + + cacheTxt(ERR, "err: " + err.errorString() + "\n"); + } + } + + if (canIgnore) + { + ignoreSslErrors(); + } +} + void Session::isConnected() { - // client header format: [4bytes(tag)][134bytes(appName)][272bytes(coName)] + // client header format: [4bytes(tag)][134bytes(appName)][272bytes(padding)] // tag = 0x4D, 0x52, 0x43, 0x49 (MRCI) // appName = UTF16LE string (padded with 0x00) - // coName = UTF16LE string (padded with 0x00) - - // for MRCI host, if the client doesn't need to request a special SSL - // cert from the host, it is good practice to fallback to using the - // address used to connect to the host as the common name (coName). - // in this case, it is used all of the time. + // padding = just a string of 0x00 (reserved for future expansion) cacheTxt(TEXT, "Connected.\n"); @@ -112,7 +142,7 @@ void Session::isConnected() header.append(SERVER_HEADER_TAG); header.append(fixedToTEXT(appName, 134)); - header.append(fixedToTEXT(*Shared::hostAddress, 272)); + header.append(QByteArray(272, 0)); if (header.size() == CLIENT_HEADER_LEN) { @@ -120,7 +150,7 @@ void Session::isConnected() } else { - cacheTxt(ERR, "\nerr: client bug! - header len not equal to " + QString::number(CLIENT_HEADER_LEN) + "\n"); + cacheTxt(ERR, "\nerr: client bug! - header len not equal to " + QString::number(CLIENT_HEADER_LEN) + " bytes.\n"); disconnectFromHost(); } } @@ -264,25 +294,6 @@ void Session::procAsync(const QByteArray &data) hook = 0; } - else if (cmdId == ASYNC_SYS_MSG) - { - if (dType == HOST_CERT) - { - QSslCertificate cert(data, QSsl::Pem); - QSslError selfSigned(QSslError::SelfSignedCertificate, cert); - - cacheTxt(TEXT, "SSL cert received.\n\n"); - - if (cert.isSelfSigned()) - { - cacheTxt(TEXT, "WARNING: the host cert is self signed.\n\n"); - } - - ignoreSslErrors(QList() << selfSigned); - addCaCertificate(cert); - startClientEncryption(); - } - } else if (cmdId == ASYNC_ADD_CMD) { if (dType == NEW_CMD) @@ -435,32 +446,29 @@ void Session::dataIn() { // host header format: [1byte(reply)][2bytes(major)][2bytes(minor)][2bytes(tcp_rev)][2bytes(mod_rev)][28bytes(sesId)] - // reply is an 8bit little endian int the indicates the result of the client header - // sent to the host from the isConnected() function. - // sesId = 224bit sha3 hash // major = 16bit little endian uint (host ver major) // minor = 16bit little endian uint (host ver minor) // patch = 16bit little endian uint (host ver patch) - // reply 1: client header acceptable, SSL handshake not needed. - // reply 2: client header acceptable, SSL handshake needed (STARTTLS). - // reply 4: the common name sent by the client header was not found. + // reply [1]: SSL handshake not needed. + // reply [2]: SSL handshake needed (STARTTLS). - auto reply = static_cast(rdInt(read(1))); + auto servHeader = read(SERVER_HEADER_LEN); + auto reply = static_cast(servHeader[0]); if ((reply == 1) || (reply == 2)) { - *Shared::servMajor = static_cast(rdInt(read(2))); - *Shared::servMinor = static_cast(rdInt(read(2))); - *Shared::tcpRev = static_cast(rdInt(read(2))); - *Shared::modRev = static_cast(rdInt(read(2))); + *Shared::servMajor = static_cast(rdInt(servHeader.mid(1, 2))); + *Shared::servMinor = static_cast(rdInt(servHeader.mid(3, 2))); + *Shared::tcpRev = static_cast(rdInt(servHeader.mid(5, 2))); + *Shared::modRev = static_cast(rdInt(servHeader.mid(7, 2))); cacheTxt(TEXT, "Detected host version: " + verText() + "\n"); - if (*Shared::tcpRev == 0) + if (*Shared::tcpRev == 1) { - *Shared::sessionId = read(28); + *Shared::sessionId = servHeader.mid(9, 28); *Shared::connectedToHost = true; flags |= VER_OK; @@ -469,7 +477,8 @@ void Session::dataIn() if (reply == 2) { - cacheTxt(TEXT, "Awaiting SSL cert from the host.\n"); + cacheTxt(TEXT, "Starting SSL handshake.\n"); + startClientEncryption(); } else { @@ -482,10 +491,6 @@ void Session::dataIn() disconnectFromHost(); } } - else if (reply == 4) - { - cacheTxt(ERR, "err: The host was unable to find a SSL cert for common name: " + *Shared::hostAddress + ".\n"); - } else { cacheTxt(ERR, "err: Invalid reply id from the host: " + QString::number(reply) + ".\n"); diff --git a/src/session.h b/src/session.h index f7c1e64..93c7c4f 100644 --- a/src/session.h +++ b/src/session.h @@ -42,7 +42,7 @@ #define SERVER_HEADER_TAG "MRCI" #define CLIENT_HEADER_LEN 410 -#define SERVER_HEADER_LEN 35 +#define SERVER_HEADER_LEN 37 #define FRAME_HEADER_LEN 8 class Session : public QSslSocket @@ -76,6 +76,7 @@ private slots: void resetProg(); void startProg(); void sockerr(QAbstractSocket::SocketError err); + void sslErrs(const QList &errors); public: