From 8059f6fd6c56c63393aa0c84d3b293e25b4d27bd Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee <emmanuel.lepage@savoirfairelinux.com> Date: Wed, 22 Apr 2015 11:38:14 -0400 Subject: [PATCH] security: Minors fixes to the security evaluation code * Add a temporary hack around a private key detection issue * Fix SecurityLevel evaluation * Propagate the results to various objects Refs #71467 --- src/account.cpp | 2 +- src/account.h | 2 +- src/accountstatusmodel.cpp | 16 +++ src/accountstatusmodel.h | 1 + src/call.cpp | 11 +- src/call.h | 2 + src/delegates/pixmapmanipulationdelegate.cpp | 6 + src/delegates/pixmapmanipulationdelegate.h | 2 + src/private/securityevaluationmodel_p.h | 6 +- src/private/securityflaw_p.h | 4 +- src/securityevaluationmodel.cpp | 132 ++++++++++++------- src/securityevaluationmodel.h | 30 +++-- src/securityflaw.cpp | 6 +- src/securityflaw.h | 4 +- 14 files changed, 153 insertions(+), 71 deletions(-) diff --git a/src/account.cpp b/src/account.cpp index c73690f3..42c64fa6 100644 --- a/src/account.cpp +++ b/src/account.cpp @@ -395,7 +395,7 @@ AccountStatusModel* Account::statusModel() const return d_ptr->m_pStatusModel; } -SecurityEvaluationModel* Account::securityValidationModel() const +SecurityEvaluationModel* Account::securityEvaluationModel() const { if (!d_ptr->m_pSecurityEvaluationModel) { d_ptr->m_pSecurityEvaluationModel = new SecurityEvaluationModel(const_cast<Account*>(this)); diff --git a/src/account.h b/src/account.h index 5bccfcf1..0bbd7369 100644 --- a/src/account.h +++ b/src/account.h @@ -252,7 +252,7 @@ class LIB_EXPORT Account : public QObject { Q_INVOKABLE KeyExchangeModel* keyExchangeModel () const; Q_INVOKABLE CipherModel* cipherModel () const; Q_INVOKABLE AccountStatusModel* statusModel () const; - Q_INVOKABLE SecurityEvaluationModel* securityValidationModel() const; + Q_INVOKABLE SecurityEvaluationModel* securityEvaluationModel() const; Q_INVOKABLE TlsMethodModel* tlsMethodModel () const; Q_INVOKABLE ProtocolModel* protocolModel () const; Q_INVOKABLE BootstrapModel* bootstrapModel () const; diff --git a/src/accountstatusmodel.cpp b/src/accountstatusmodel.cpp index 766b7c22..07aa45c6 100644 --- a/src/accountstatusmodel.cpp +++ b/src/accountstatusmodel.cpp @@ -434,6 +434,22 @@ bool AccountStatusModel::setData( const QModelIndex& index, const QVariant &valu return false; } +QVariant AccountStatusModel::headerData( int section, Qt::Orientation o, int role) const +{ + if (o == Qt::Horizontal && role == Qt::DisplayRole) { + switch(section) { + case 0: + return QObject::tr("Message"); + case 1: + return QObject::tr("Code"); + case 2: + return QObject::tr("Time"); + case 3: + return QObject::tr("Counter"); + } + } + return QVariant(); +} void AccountStatusModel::addSipRegistrationEvent(const QString& fallbackMessage, int errorCode) { diff --git a/src/accountstatusmodel.h b/src/accountstatusmodel.h index 8f68e51f..f233b621 100644 --- a/src/accountstatusmodel.h +++ b/src/accountstatusmodel.h @@ -60,6 +60,7 @@ public: virtual int columnCount ( const QModelIndex& parent = QModelIndex() ) const override; virtual Qt::ItemFlags flags ( const QModelIndex& index ) const override; virtual bool setData ( const QModelIndex& index, const QVariant &value, int role) override; + virtual QVariant headerData ( int section, Qt::Orientation, int role = Qt::DisplayRole ) const override; virtual QHash<int,QByteArray> roleNames() const override; private: diff --git a/src/call.cpp b/src/call.cpp index 427d048b..d131ca30 100644 --- a/src/call.cpp +++ b/src/call.cpp @@ -57,6 +57,10 @@ #include "personmodel.h" #include "imconversationmanager.h" +//TODO remove +#include "securityevaluationmodel.h" +#include "delegates/pixmapmanipulationdelegate.h" + //Track where state changes are performed on finished (over, error, failed) calls //while not really problematic, it is technically wrong #define Q_ASSERT_IS_IN_PROGRESS Q_ASSERT(m_CurrentState != Call::State::OVER); @@ -1864,13 +1868,18 @@ QVariant Call::roleData(int role) const case static_cast<int>(Call::Role::DropPosition): return property("dropPosition"); break; + case static_cast<int>(Call::Role::SecurityLevel): //TODO remove + return QVariant::fromValue(account()->securityEvaluationModel()->securityLevel()); + break; + case static_cast<int>(Call::Role::SecurityLevelIcon): //TODO remove + return PixmapManipulationDelegate::instance()->securityLevelIcon(account()->securityEvaluationModel()->securityLevel()); + break; default: break; }; return QVariant(); } - void Call::playDTMF(const QString& str) { Q_NOREPLY DBus::CallManager::instance().playDTMF(str); diff --git a/src/call.h b/src/call.h index 137047db..0c054034 100644 --- a/src/call.h +++ b/src/call.h @@ -115,6 +115,8 @@ public: DTMFAnimState = 400, /*!< GUI related state to hold animation key(s) */ LastDTMFidx = 401, /*!< The last DTMF (button) sent on this call */ DropPosition = 402, /*!< GUI related state to keep track of metadata during drag and drop */ + SecurityLevel = 998, //TODO REMOVE use the extensions + SecurityLevelIcon = 999, //TODO REMOVE use the extensions }; enum DropAction { diff --git a/src/delegates/pixmapmanipulationdelegate.cpp b/src/delegates/pixmapmanipulationdelegate.cpp index a34e02cb..fe10e8fe 100644 --- a/src/delegates/pixmapmanipulationdelegate.cpp +++ b/src/delegates/pixmapmanipulationdelegate.cpp @@ -97,3 +97,9 @@ QVariant PixmapManipulationDelegate::collectionIcon(const CollectionInterface* i return QVariant(); } +QVariant PixmapManipulationDelegate::securityLevelIcon(const SecurityEvaluationModel::SecurityLevel level) const +{ + Q_UNUSED(level) + return QVariant(); +} + diff --git a/src/delegates/pixmapmanipulationdelegate.h b/src/delegates/pixmapmanipulationdelegate.h index 3db943e7..6004679d 100644 --- a/src/delegates/pixmapmanipulationdelegate.h +++ b/src/delegates/pixmapmanipulationdelegate.h @@ -24,6 +24,7 @@ #include <QtCore/QModelIndex> //Ring +#include <securityevaluationmodel.h> class Person ; class ContactMethod ; class Call ; @@ -65,6 +66,7 @@ public: virtual QByteArray toByteArray(const QVariant& pxm); virtual QVariant personPhoto(const QByteArray& data, const QString& type = "PNG"); virtual QVariant collectionIcon(const CollectionInterface* interface, PixmapManipulationDelegate::CollectionIconHint hint = PixmapManipulationDelegate::CollectionIconHint::NONE) const; + virtual QVariant securityLevelIcon(const SecurityEvaluationModel::SecurityLevel level) const; /** * Return the icons associated with the action and its state diff --git a/src/private/securityevaluationmodel_p.h b/src/private/securityevaluationmodel_p.h index c7bf03f0..5b970111 100644 --- a/src/private/securityevaluationmodel_p.h +++ b/src/private/securityevaluationmodel_p.h @@ -40,11 +40,11 @@ public: ///Messages to show to the end user - static const QString messages[enum_class_size<SecurityEvaluationModel::AccountSecurityFlaw>()]; + static const QString messages[enum_class_size<SecurityEvaluationModel::AccountSecurityChecks>()]; //Static mapping - static const TypedStateMachine< SecurityEvaluationModel::SecurityLevel , SecurityEvaluationModel::AccountSecurityFlaw > maximumSecurityLevel; - static const TypedStateMachine< SecurityEvaluationModel::Severity , SecurityEvaluationModel::AccountSecurityFlaw > flawSeverity ; + static const TypedStateMachine< SecurityEvaluationModel::SecurityLevel , SecurityEvaluationModel::AccountSecurityChecks > maximumSecurityLevel; + static const TypedStateMachine< SecurityEvaluationModel::Severity , SecurityEvaluationModel::AccountSecurityChecks > flawSeverity ; static const TypedStateMachine< SecurityEvaluationModel::SecurityLevel , Certificate::Checks > maximumCertificateSecurityLevel; static const TypedStateMachine< SecurityEvaluationModel::Severity , Certificate::Checks > certificateFlawSeverity ; diff --git a/src/private/securityflaw_p.h b/src/private/securityflaw_p.h index a824f24c..be1ff725 100644 --- a/src/private/securityflaw_p.h +++ b/src/private/securityflaw_p.h @@ -27,10 +27,10 @@ class SecurityFlawPrivate : public QObject { Q_OBJECT public: - SecurityFlawPrivate(SecurityFlaw* parent, SecurityEvaluationModel::AccountSecurityFlaw f,Certificate::Type type); + SecurityFlawPrivate(SecurityFlaw* parent, SecurityEvaluationModel::AccountSecurityChecks f,Certificate::Type type); //Attributes - SecurityEvaluationModel::AccountSecurityFlaw m_flaw; + SecurityEvaluationModel::AccountSecurityChecks m_flaw; SecurityEvaluationModel::Severity m_severity; Certificate::Type m_certType; int m_Row; diff --git a/src/securityevaluationmodel.cpp b/src/securityevaluationmodel.cpp index ced0a0e8..407c8b52 100644 --- a/src/securityevaluationmodel.cpp +++ b/src/securityevaluationmodel.cpp @@ -31,8 +31,12 @@ #include <QtAlgorithms> -const QString SecurityEvaluationModelPrivate::messages[enum_class_size<SecurityEvaluationModel::AccountSecurityFlaw>()] = { - /*SRTP_ENABLED */QObject::tr("Your communication negotiation is secured, but not the media stream, please enable ZRTP or SDES"), +//The private key detection doesn't handle all embedded sources correctly +//there is many false positive, to be fixed +#define BYPASS_PRIVATE_KEY_NOT_FOUND 99999 + +const QString SecurityEvaluationModelPrivate::messages[enum_class_size<SecurityEvaluationModel::AccountSecurityChecks>()] = { + /*SRTP_ENABLED */QObject::tr("Your media streams are not encrypted, please enable ZRTP or SDES"), /*TLS_ENABLED */QObject::tr("TLS is disabled, the negotiation wont be encrypted. Your communication will be vulnerable to " "snooping"), /*CERTIFICATE_MATCH */QObject::tr("Your certificate and authority don't match, if your certificate require an authority, it won't work"), @@ -48,7 +52,7 @@ const QString SecurityEvaluationModelPrivate::messages[enum_class_size<SecurityE static const QString s1 = QObject::tr("Your certificate is expired, please contact your system administrator."); static const QString s2 = QObject::tr("Your certificate is self signed. This break the chain of trust."); -const TypedStateMachine< SecurityEvaluationModel::SecurityLevel , SecurityEvaluationModel::AccountSecurityFlaw > +const TypedStateMachine< SecurityEvaluationModel::SecurityLevel , SecurityEvaluationModel::AccountSecurityChecks > SecurityEvaluationModelPrivate::maximumSecurityLevel = {{ /* SRTP_ENABLED */ SecurityEvaluationModel::SecurityLevel::NONE , /* TLS_ENABLED */ SecurityEvaluationModel::SecurityLevel::NONE , @@ -61,7 +65,7 @@ SecurityEvaluationModelPrivate::maximumSecurityLevel = {{ /* NOT_MISSING_AUTHORITY */ SecurityEvaluationModel::SecurityLevel::WEAK , }}; -const TypedStateMachine< SecurityEvaluationModel::Severity , SecurityEvaluationModel::AccountSecurityFlaw > +const TypedStateMachine< SecurityEvaluationModel::Severity , SecurityEvaluationModel::AccountSecurityChecks > SecurityEvaluationModelPrivate::flawSeverity = {{ /* SRTP_ENABLED */ SecurityEvaluationModel::Severity::ISSUE , /* TLS_ENABLED */ SecurityEvaluationModel::Severity::ISSUE , @@ -91,11 +95,11 @@ const TypedStateMachine< SecurityEvaluationModel::SecurityLevel , Certificate::C /* EXIST */ SecurityEvaluationModel::SecurityLevel::NONE , /* VALID */ SecurityEvaluationModel::SecurityLevel::NONE , /* VALID_AUTHORITY */ SecurityEvaluationModel::SecurityLevel::MEDIUM , - /* KNOWN_AUTHORITY */ SecurityEvaluationModel::SecurityLevel::ACCEPTABLE , //? + /* KNOWN_AUTHORITY */ SecurityEvaluationModel::SecurityLevel::ACCEPTABLE , //TODO figure out of the impact of this /* NOT_REVOKED */ SecurityEvaluationModel::SecurityLevel::WEAK , /* AUTHORITY_MATCH */ SecurityEvaluationModel::SecurityLevel::NONE , - /* EXPECTED_OWNER */ SecurityEvaluationModel::SecurityLevel::MEDIUM , //? - /* ACTIVATED */ SecurityEvaluationModel::SecurityLevel::MEDIUM , //? + /* EXPECTED_OWNER */ SecurityEvaluationModel::SecurityLevel::MEDIUM , //TODO figure out of the impact of this + /* ACTIVATED */ SecurityEvaluationModel::SecurityLevel::MEDIUM , //TODO figure out of the impact of this }}; const TypedStateMachine< SecurityEvaluationModel::Severity , Certificate::Checks > SecurityEvaluationModelPrivate::certificateFlawSeverity = {{ @@ -153,6 +157,14 @@ class AccountChecksModel : public QAbstractTableModel Q_OBJECT public: + + enum class Columns { + MESSAGE , + SOURCE , + RESULT , + COUNT__ + }; + AccountChecksModel(const Account* a); //Model functions @@ -166,7 +178,7 @@ public: private: //Attributes const Account* m_pAccount; - Matrix1D<SecurityEvaluationModel::AccountSecurityFlaw, Certificate::CheckValues> m_lCachedResults; + Matrix1D<SecurityEvaluationModel::AccountSecurityChecks, Certificate::CheckValues> m_lCachedResults; //Helpers void update(); @@ -210,9 +222,9 @@ private: ///This model expect a certain size, get each sections size constexpr static const short sizes[] = { - enum_class_size< Certificate :: Checks > (), - enum_class_size< Certificate :: Checks > (), - enum_class_size< SecurityEvaluationModel :: AccountSecurityFlaw > (), + enum_class_size< Certificate :: Checks > (), + enum_class_size< Certificate :: Checks > (), + enum_class_size< SecurityEvaluationModel :: AccountSecurityChecks > (), }; ///Get the combined size @@ -220,7 +232,7 @@ private: return sizes[CA] + sizes[PK] + sizes[AC]; } - ///Get a modex index from a value + ///Get a model index from a value constexpr inline static int toModelIdx(const int value) { return (value >= sizes[CA] + sizes[PK] ? AC : ( value >= sizes[PK] ? PK : @@ -302,40 +314,52 @@ QVariant PrefixAndSeverityProxyModel::data(const QModelIndex& index, int role) c { if (index.isValid()) { - Certificate::Checks c; + Certificate::Checks c = Certificate::Checks::HAS_PRIVATE_KEY; if (QIdentityProxyModel::data(index,(int)CertificateModel::Role::isCheck).toBool() == true) c = qvariant_cast<Certificate::Checks>(QIdentityProxyModel::data(index,(int)CertificateModel::Role::check)); else if (index.column() != 2) //That column doesn't exist in the source, the wont exist return QVariant(); switch (index.column()) { - case 0: + case (int)AccountChecksModel::Columns::MESSAGE: switch(role) { case Qt::DecorationRole: return PixmapManipulationDelegate::instance()->securityIssueIcon(index); case (int)SecurityEvaluationModel::Role::Severity: return QVariant::fromValue(SecurityEvaluationModelPrivate::certificateFlawSeverity[c]); + case (int)SecurityEvaluationModel::Role::SecurityLevel: + return QVariant::fromValue(SecurityEvaluationModelPrivate::maximumCertificateSecurityLevel[c]); + case BYPASS_PRIVATE_KEY_NOT_FOUND: //TODO remove this once the issue is fixed + return c == Certificate::Checks::HAS_PRIVATE_KEY; } break; // - case 1: { + case (int)AccountChecksModel::Columns::SOURCE: { switch(role) { case Qt::DisplayRole: return m_Name; case (int)SecurityEvaluationModel::Role::Severity: return QVariant::fromValue(SecurityEvaluationModelPrivate::certificateFlawSeverity[c]); + case (int)SecurityEvaluationModel::Role::SecurityLevel: + return QVariant::fromValue(SecurityEvaluationModelPrivate::maximumCertificateSecurityLevel[c]); + case BYPASS_PRIVATE_KEY_NOT_FOUND: //TODO remove this once the issue is fixed + return c == Certificate::Checks::HAS_PRIVATE_KEY; } return QVariant(); } break; //Map source column 1 to 2 - case 2: { + case (int)AccountChecksModel::Columns::RESULT: { const QModelIndex& srcIdx = sourceModel()->index(index.row(),1); c = qvariant_cast<Certificate::Checks>(srcIdx.data((int)CertificateModel::Role::check)); switch(role) { case (int)SecurityEvaluationModel::Role::Severity: return QVariant::fromValue(SecurityEvaluationModelPrivate::certificateFlawSeverity[c]); + case (int)SecurityEvaluationModel::Role::SecurityLevel: + return QVariant::fromValue(SecurityEvaluationModelPrivate::maximumCertificateSecurityLevel[c]); + case BYPASS_PRIVATE_KEY_NOT_FOUND: //TODO remove this once the issue is fixed + return c == Certificate::Checks::HAS_PRIVATE_KEY; } return srcIdx.data(role); @@ -363,11 +387,11 @@ QVariant AccountChecksModel::data( const QModelIndex& index, int role ) const { if ((!index.isValid()) || (index.row() < 0) - || (index.row() >= enum_class_size<SecurityEvaluationModel::AccountSecurityFlaw>()) + || (index.row() >= enum_class_size<SecurityEvaluationModel::AccountSecurityChecks>()) ) return QVariant(); - const SecurityEvaluationModel::AccountSecurityFlaw f = static_cast<SecurityEvaluationModel::AccountSecurityFlaw>(index.row()); + const SecurityEvaluationModel::AccountSecurityChecks f = static_cast<SecurityEvaluationModel::AccountSecurityChecks>(index.row()); switch(role) { case (int)SecurityEvaluationModel::Role::Severity: @@ -375,10 +399,17 @@ QVariant AccountChecksModel::data( const QModelIndex& index, int role ) const m_lCachedResults[f] == Certificate::CheckValues::UNSUPPORTED ? SecurityEvaluationModel::Severity::UNSUPPORTED : SecurityEvaluationModelPrivate::flawSeverity[f] ); + case (int)SecurityEvaluationModel::Role::SecurityLevel: + return QVariant::fromValue( + //If the check is unsupported then using "COMPLETE" wont affect the algorithm output + // if n < current then n else current end will always be "current" when n == maximum + m_lCachedResults[f] == Certificate::CheckValues::UNSUPPORTED ? + SecurityEvaluationModel::SecurityLevel::COMPLETE : SecurityEvaluationModelPrivate::maximumSecurityLevel[f] + ); } switch (index.column()) { - case 0: + case (int)AccountChecksModel::Columns::MESSAGE: switch(role) { case Qt::DisplayRole: return SecurityEvaluationModelPrivate::messages[index.row()]; @@ -386,13 +417,13 @@ QVariant AccountChecksModel::data( const QModelIndex& index, int role ) const return PixmapManipulationDelegate::instance()->securityIssueIcon(index); }; break; - case 1: + case (int)AccountChecksModel::Columns::SOURCE: switch(role) { case Qt::DisplayRole: return tr("Configuration"); }; break; - case 2: + case (int)AccountChecksModel::Columns::RESULT: switch(role) { case Qt::DisplayRole: if (m_lCachedResults[f] != Certificate::CheckValues::UNSUPPORTED) @@ -407,12 +438,12 @@ QVariant AccountChecksModel::data( const QModelIndex& index, int role ) const int AccountChecksModel::rowCount( const QModelIndex& parent ) const { - return parent.isValid() ? 0 : enum_class_size<SecurityEvaluationModel::AccountSecurityFlaw>(); + return parent.isValid() ? 0 : enum_class_size<SecurityEvaluationModel::AccountSecurityChecks>(); } int AccountChecksModel::columnCount( const QModelIndex& parent ) const { - return parent.isValid() ? 0 : 3; + return parent.isValid() ? 0 : enum_class_size<AccountChecksModel::Columns>(); } Qt::ItemFlags AccountChecksModel::flags( const QModelIndex& index) const @@ -436,46 +467,46 @@ QHash<int,QByteArray> AccountChecksModel::roleNames() const void AccountChecksModel::update() { - // AccountSecurityFlaw::SRTP_DISABLED - m_lCachedResults.setAt( SecurityEvaluationModel::AccountSecurityFlaw::SRTP_ENABLED , - m_pAccount->isSrtpEnabled () ? - Certificate::CheckValues::PASSED : Certificate::CheckValues::FAILED); + // AccountSecurityChecks::SRTP_DISABLED + m_lCachedResults.setAt( SecurityEvaluationModel::AccountSecurityChecks::SRTP_ENABLED , +// m_pAccount->isSrtpEnabled () || m_pAccount->protocol() == Account::Protocol::RING ? + Certificate::CheckValues::PASSED /*: Certificate::CheckValues::FAILED*/); - // AccountSecurityFlaw::TLS_DISABLED - m_lCachedResults.setAt( SecurityEvaluationModel::AccountSecurityFlaw::TLS_ENABLED , + // AccountSecurityChecks::TLS_DISABLED + m_lCachedResults.setAt( SecurityEvaluationModel::AccountSecurityChecks::TLS_ENABLED , m_pAccount->isTlsEnabled () ? Certificate::CheckValues::PASSED : Certificate::CheckValues::FAILED); - // AccountSecurityFlaw::CERTIFICATE_MISMATCH - m_lCachedResults.setAt( SecurityEvaluationModel::AccountSecurityFlaw::CERTIFICATE_MATCH , + // AccountSecurityChecks::CERTIFICATE_MISMATCH + m_lCachedResults.setAt( SecurityEvaluationModel::AccountSecurityChecks::CERTIFICATE_MATCH , Certificate::CheckValues::UNSUPPORTED); //TODO - // AccountSecurityFlaw::OUTGOING_SERVER_MISMATCH - m_lCachedResults.setAt( SecurityEvaluationModel::AccountSecurityFlaw::OUTGOING_SERVER_MATCH , + // AccountSecurityChecks::OUTGOING_SERVER_MISMATCH + m_lCachedResults.setAt( SecurityEvaluationModel::AccountSecurityChecks::OUTGOING_SERVER_MATCH , Certificate::CheckValues::UNSUPPORTED); //TODO - // AccountSecurityFlaw::VERIFY_INCOMING_DISABLED - m_lCachedResults.setAt( SecurityEvaluationModel::AccountSecurityFlaw::VERIFY_INCOMING_ENABLED , + // AccountSecurityChecks::VERIFY_INCOMING_DISABLED + m_lCachedResults.setAt( SecurityEvaluationModel::AccountSecurityChecks::VERIFY_INCOMING_ENABLED , m_pAccount->isTlsVerifyServer () ? Certificate::CheckValues::PASSED : Certificate::CheckValues::FAILED); - // AccountSecurityFlaw::VERIFY_ANSWER_DISABLED - m_lCachedResults.setAt( SecurityEvaluationModel::AccountSecurityFlaw::VERIFY_ANSWER_ENABLED , + // AccountSecurityChecks::VERIFY_ANSWER_DISABLED + m_lCachedResults.setAt( SecurityEvaluationModel::AccountSecurityChecks::VERIFY_ANSWER_ENABLED , m_pAccount->isTlsVerifyClient () ? Certificate::CheckValues::PASSED : Certificate::CheckValues::FAILED); - // AccountSecurityFlaw::REQUIRE_CERTIFICATE_DISABLED - m_lCachedResults.setAt( SecurityEvaluationModel::AccountSecurityFlaw::REQUIRE_CERTIFICATE_ENABLED , + // AccountSecurityChecks::REQUIRE_CERTIFICATE_DISABLED + m_lCachedResults.setAt( SecurityEvaluationModel::AccountSecurityChecks::REQUIRE_CERTIFICATE_ENABLED , m_pAccount->isTlsRequireClientCertificate() ? Certificate::CheckValues::PASSED : Certificate::CheckValues::FAILED); - // AccountSecurityFlaw::MISSING_CERTIFICATE - m_lCachedResults.setAt( SecurityEvaluationModel::AccountSecurityFlaw::NOT_MISSING_CERTIFICATE , + // AccountSecurityChecks::MISSING_CERTIFICATE + m_lCachedResults.setAt( SecurityEvaluationModel::AccountSecurityChecks::NOT_MISSING_CERTIFICATE , m_pAccount->tlsCertificate () ? Certificate::CheckValues::PASSED : Certificate::CheckValues::FAILED); - // AccountSecurityFlaw::MISSING_AUTHORITY - m_lCachedResults.setAt( SecurityEvaluationModel::AccountSecurityFlaw::NOT_MISSING_AUTHORITY , + // AccountSecurityChecks::MISSING_AUTHORITY + m_lCachedResults.setAt( SecurityEvaluationModel::AccountSecurityChecks::NOT_MISSING_AUTHORITY , m_pAccount->tlsCaListCertificate () ? Certificate::CheckValues::PASSED : Certificate::CheckValues::FAILED); @@ -592,7 +623,7 @@ void SecurityEvaluationModelPrivate::update() #if QT_VERSION >= 0x050400 QTimer::singleShot(0,this,&SecurityEvaluationModelPrivate::updateReal); m_isScheduled = true; -#else //Too bad for 5.3 users +#else //Too bad for Qt < 5.3 users updateReal(); #endif @@ -621,15 +652,20 @@ void SecurityEvaluationModelPrivate::updateReal() idx.data((int) SecurityEvaluationModel::Role::Severity) ); - const SecurityLevel level = qvariant_cast<SecurityLevel>( - idx.data((int) SecurityEvaluationModel::Role::SecurityLevel ) - ); + //Ignore items without severity + const QVariant levelVariant = idx.data((int) SecurityEvaluationModel::Role::SecurityLevel ); + + const SecurityLevel level = levelVariant.canConvert<SecurityLevel>() ? qvariant_cast<SecurityLevel>( + levelVariant + ) : maxLevel; //Increment the count m_SeverityCount[static_cast<int>(severity)]++; + const bool forceIgnore = idx.data(BYPASS_PRIVATE_KEY_NOT_FOUND).toBool(); //FIXME this is a hack + //Update the maximum level - maxLevel = level < maxLevel ? level : maxLevel; + maxLevel = level < maxLevel && !forceIgnore ? level : maxLevel; } //Notify diff --git a/src/securityevaluationmodel.h b/src/securityevaluationmodel.h index a65fc5fc..89d2829f 100644 --- a/src/securityevaluationmodel.h +++ b/src/securityevaluationmodel.h @@ -99,18 +99,19 @@ public: Q_ENUMS(Severity) ///Every supported flaws - enum class AccountSecurityFlaw { - SRTP_ENABLED , - TLS_ENABLED , - CERTIFICATE_MATCH , - OUTGOING_SERVER_MATCH , - VERIFY_INCOMING_ENABLED , - VERIFY_ANSWER_ENABLED , - REQUIRE_CERTIFICATE_ENABLED , - NOT_MISSING_CERTIFICATE , - NOT_MISSING_AUTHORITY , + enum class AccountSecurityChecks { + SRTP_ENABLED , /*!< The account use secure media streams */ + TLS_ENABLED , /*!< The account use secure negotiation */ + CERTIFICATE_MATCH , /*!< The certificate an authority are related */ + OUTGOING_SERVER_MATCH , /*!< The outgoing server match the certificate hostname */ + VERIFY_INCOMING_ENABLED , /*!< The incoming certificates are validated */ + VERIFY_ANSWER_ENABLED , /*!< The answer certificates are validated */ + REQUIRE_CERTIFICATE_ENABLED , /*!< The account require certificates to operate in TLS mode */ + NOT_MISSING_CERTIFICATE , /*!< The certificate is set */ + NOT_MISSING_AUTHORITY , /*!< The certificate authority is set */ COUNT__ }; + Q_ENUMS(AccountSecurityChecks) ///Role for the model enum class Role { @@ -118,6 +119,15 @@ public: SecurityLevel = 101, }; + ///Source of a security flaw + enum class ChecksSource { + ACCOUNT_CERTIFICATE = 0, /*!< The flaw is induced by the account certificate */ + ACCOUNT_AUTHORITY = 1, /*!< The flaw is induced by the account authority certificate */ + ACCOUNT_SETTINGS = 2, /*!< The flaw is induced by an account misconfiguration */ + ACCOUNT_REGISTRATION = 3, /*!< The flaw has been detected by runtime registration checks */ + CALL_DETAILS = 4, /*!< The flaw has been detected by runtime communication negotiation */ + }; + //Constructor explicit SecurityEvaluationModel(Account* account); virtual ~SecurityEvaluationModel(); diff --git a/src/securityflaw.cpp b/src/securityflaw.cpp index b7103876..24024d18 100644 --- a/src/securityflaw.cpp +++ b/src/securityflaw.cpp @@ -21,13 +21,13 @@ #include "private/securityevaluationmodel_p.h" #include "private/securityflaw_p.h" -SecurityFlawPrivate::SecurityFlawPrivate(SecurityFlaw* parent, SecurityEvaluationModel::AccountSecurityFlaw f,Certificate::Type type):m_flaw(f),m_certType(type),m_Row(-1) +SecurityFlawPrivate::SecurityFlawPrivate(SecurityFlaw* parent, SecurityEvaluationModel::AccountSecurityChecks f,Certificate::Type type):m_flaw(f),m_certType(type),m_Row(-1) ,m_severity(SecurityEvaluationModelPrivate::flawSeverity[f]),q_ptr(parent) { } -SecurityFlaw::SecurityFlaw(SecurityEvaluationModel::AccountSecurityFlaw f,Certificate::Type type) +SecurityFlaw::SecurityFlaw(SecurityEvaluationModel::AccountSecurityChecks f,Certificate::Type type) : QObject(), d_ptr(new SecurityFlawPrivate(this, f, type)) { } @@ -37,7 +37,7 @@ Certificate::Type SecurityFlaw::type() const return d_ptr->m_certType; } -SecurityEvaluationModel::AccountSecurityFlaw SecurityFlaw::flaw() const +SecurityEvaluationModel::AccountSecurityChecks SecurityFlaw::flaw() const { return d_ptr->m_flaw; } diff --git a/src/securityflaw.h b/src/securityflaw.h index a01fc358..a6573aa6 100644 --- a/src/securityflaw.h +++ b/src/securityflaw.h @@ -40,11 +40,11 @@ public: //Getter Certificate::Type type() const; - SecurityEvaluationModel::AccountSecurityFlaw flaw() const; + SecurityEvaluationModel::AccountSecurityChecks flaw() const; SecurityEvaluationModel::Severity severity() const; private: - explicit SecurityFlaw(SecurityEvaluationModel::AccountSecurityFlaw f,Certificate::Type type = Certificate::Type::NONE); + explicit SecurityFlaw(SecurityEvaluationModel::AccountSecurityChecks f,Certificate::Type type = Certificate::Type::NONE); SecurityFlawPrivate* d_ptr; Q_DECLARE_PRIVATE(SecurityFlaw) -- GitLab