[auth] Move inline file reads to static func; add PKCS#8 sniffing func

python/core/auth/qgsauthcertutils.sip

@@ -81,6 +81,14 @@ Map certificate sha1 to certificate as simple cache
 %End
 
 
+    static QByteArray fileData( const QString &path, bool astext = false );
+%Docstring
+ Return data from a local file via a read-only operation
+ \param astext Whether to open the file as text, otherwise as binary
+ :return: All data contained in file or empty contents if file does not exist
+ :rtype: QByteArray
+%End
+
     static QList<QSslCertificate> certsFromFile( const QString &certspath );
 %Docstring
 Return list of concatenated certs from a PEM or DER formatted file
@@ -150,6 +158,16 @@ Return list of concatenated certs from a PEM Base64 text block
  :rtype: list of str
 %End
 
+    static bool pemIsPkcs8( const QString &keyPemTxt );
+%Docstring
+ Determine if the PEM-encoded text of a key is PKCS#8 format
+ \param keyPemTxt PEM-encoded text
+ :return: True if PKCS#8, otherwise false
+ :rtype: bool
+%End
+
+
+
     static QStringList pkcs12BundleToPem( const QString &bundlepath,
                                           const QString &bundlepass = QString(),
                                           bool reencrypt = true );

src/core/auth/qgsauthcertutils.cpp

@@ -94,22 +94,26 @@ QMap<QString, QList<QgsAuthConfigSslServer> > QgsAuthCertUtils::sslConfigsGroupe
   return orgconfigs;
 }
 
-static QByteArray fileData_( const QString &path, bool astext = false )
+QByteArray QgsAuthCertUtils::fileData( const QString &path, bool astext )
 {
   QByteArray data;
   QFile file( path );
-  if ( file.exists() )
+  if ( !file.exists() )
   {
-    QFile::OpenMode openflags( QIODevice::ReadOnly );
+    QgsDebugMsg( QStringLiteral( "Read file error, file not found: %1" ).arg( path ) );
-    if ( astext )
+    return data;
-      openflags |= QIODevice::Text;
-    bool ret = file.open( openflags );
-    if ( ret )
-    {
-      data = file.readAll();
-    }
-    file.close();
   }
+  // TODO: add checks for locked file, etc., to ensure it can be read
+  QFile::OpenMode openflags( QIODevice::ReadOnly );
+  if ( astext )
+    openflags |= QIODevice::Text;
+  bool ret = file.open( openflags );
+  if ( ret )
+  {
+    data = file.readAll();
+  }
+  file.close();
+
   return data;
 }
 
@@ -117,7 +121,7 @@ QList<QSslCertificate> QgsAuthCertUtils::certsFromFile( const QString &certspath
 {
   QList<QSslCertificate> certs;
   bool pem = certspath.endsWith( QLatin1String( ".pem" ), Qt::CaseInsensitive );
-  certs = QSslCertificate::fromData( fileData_( certspath, pem ), pem ? QSsl::Pem : QSsl::Der );
+  certs = QSslCertificate::fromData( QgsAuthCertUtils::fileData( certspath, pem ), pem ? QSsl::Pem : QSsl::Der );
   if ( certs.isEmpty() )
   {
     QgsDebugMsg( QString( "Parsed cert(s) EMPTY for path: %1" ).arg( certspath ) );
@@ -181,7 +185,7 @@ QSslKey QgsAuthCertUtils::keyFromFile( const QString &keypath,
                                        QString *algtype )
 {
   bool pem = keypath.endsWith( QLatin1String( ".pem" ), Qt::CaseInsensitive );
-  QByteArray keydata( fileData_( keypath, pem ) );
+  QByteArray keydata( QgsAuthCertUtils::fileData( keypath, pem ) );
 
   QSslKey clientkey;
   clientkey = QSslKey( keydata,
@@ -262,6 +266,13 @@ QStringList QgsAuthCertUtils::certKeyBundleToPem( const QString &certpath,
   return QStringList() << certpem << keypem << algtype;
 }
 
+bool QgsAuthCertUtils::pemIsPkcs8( const QString &keyPemTxt )
+{
+  QString pkcs8Header = QStringLiteral( "-----BEGIN PRIVATE KEY-----" );
+  QString pkcs8Footer = QStringLiteral( "-----END PRIVATE KEY-----" );
+  return keyPemTxt.contains( pkcs8Header ) && keyPemTxt.contains( pkcs8Footer );
+}
+
 QStringList QgsAuthCertUtils::pkcs12BundleToPem( const QString &bundlepath,
     const QString &bundlepass,
     bool reencrypt )

src/core/auth/qgsauthcertutils.h

@@ -104,6 +104,13 @@ class CORE_EXPORT QgsAuthCertUtils
      */
     static QMap< QString, QList<QgsAuthConfigSslServer> > sslConfigsGroupedByOrg( const QList<QgsAuthConfigSslServer> &configs ) SIP_SKIP;
 
+    /**
+     * Return data from a local file via a read-only operation
+     * \param astext Whether to open the file as text, otherwise as binary
+     * \returns All data contained in file or empty contents if file does not exist
+     */
+    static QByteArray fileData( const QString &path, bool astext = false );
+
     //! Return list of concatenated certs from a PEM or DER formatted file
     static QList<QSslCertificate> certsFromFile( const QString &certspath );
 
@@ -158,6 +165,12 @@ class CORE_EXPORT QgsAuthCertUtils
                                            bool reencrypt = true );
 
     /**
+     * Determine if the PEM-encoded text of a key is PKCS#8 format
+     * \param keyPemTxt PEM-encoded text
+     * \returns True if PKCS#8, otherwise false
+     */
+    static bool pemIsPkcs8( const QString &keyPemTxt );
+
      * Return list of certificate, private key and algorithm (as PEM text) for a PKCS#12 bundle
      * \param bundlepath File path to the PKCS bundle
      * \param bundlepass Passphrase for bundle

src/core/auth/qgsauthconfig.cpp

@@ -23,6 +23,8 @@
 #include <QCryptographicHash>
 #include <QUrl>
 
+#include "qgsauthcertutils.h"
+
 
 //////////////////////////////////////////////
 // QgsAuthMethodConfig
@@ -172,25 +174,6 @@ QgsPkiBundle::QgsPkiBundle( const QSslCertificate &clientCert,
   setClientKey( clientKey );
 }
 
-static QByteArray fileData_( const QString &path, bool astext = false )
-{
-  QByteArray data;
-  QFile file( path );
-  if ( file.exists() )
-  {
-    QFile::OpenMode openflags( QIODevice::ReadOnly );
-    if ( astext )
-      openflags |= QIODevice::Text;
-    bool ret = file.open( openflags );
-    if ( ret )
-    {
-      data = file.readAll();
-    }
-    file.close();
-  }
-  return data;
-}
-
 const QgsPkiBundle QgsPkiBundle::fromPemPaths( const QString &certPath,
     const QString &keyPath,
     const QString &keyPass,
@@ -207,12 +190,12 @@ const QgsPkiBundle QgsPkiBundle::fromPemPaths( const QString &certPath,
   {
     // client cert
     bool pem = certPath.endsWith( QLatin1String( ".pem" ), Qt::CaseInsensitive );
-    QSslCertificate clientcert( fileData_( certPath, pem ), pem ? QSsl::Pem : QSsl::Der );
+    QSslCertificate clientcert( QgsAuthCertUtils::fileData( certPath, pem ), pem ? QSsl::Pem : QSsl::Der );
     pkibundle.setClientCert( clientcert );
 
     // client key
     bool pem_key = keyPath.endsWith( QLatin1String( ".pem" ), Qt::CaseInsensitive );
-    QByteArray keydata( fileData_( keyPath, pem_key ) );
+    QByteArray keydata( QgsAuthCertUtils::fileData( keyPath, pem_key ) );
 
     QSslKey clientkey;
     clientkey = QSslKey( keydata,

src/gui/auth/qgsauthimportidentitydialog.cpp

@@ -29,26 +29,6 @@
 #include "qgslogger.h"
 
 
-static QByteArray fileData_( const QString &path, bool astext = false )
-{
-  QByteArray data;
-  QFile file( path );
-  if ( file.exists() )
-  {
-    QFile::OpenMode openflags( QIODevice::ReadOnly );
-    if ( astext )
-      openflags |= QIODevice::Text;
-    bool ret = file.open( openflags );
-    if ( ret )
-    {
-      data = file.readAll();
-    }
-    file.close();
-  }
-  return data;
-}
-
-
 QgsAuthImportIdentityDialog::QgsAuthImportIdentityDialog( QgsAuthImportIdentityDialog::IdentityType identitytype,
     QWidget *parent )
   : QDialog( parent )
@@ -306,7 +286,7 @@ bool QgsAuthImportIdentityDialog::validatePkiPaths()
 
   // check for valid private key and that any supplied password works
   bool keypem = keypath.endsWith( QLatin1String( ".pem" ), Qt::CaseInsensitive );
-  QByteArray keydata( fileData_( keypath, keypem ) );
+  QByteArray keydata( QgsAuthCertUtils::fileData( keypath, keypem ) );
 
   QSslKey clientkey;
   QString keypass = lePkiPathsKeyPass->text();

tests/src/core/CMakeLists.txt

@@ -74,6 +74,7 @@ SET(TESTS
  testqgsapplication.cpp
  testqgsatlascomposition.cpp
  testqgsauthcrypto.cpp
+ testqgsauthcertutils.cpp
  testqgsauthconfig.cpp
  testqgsauthmanager.cpp
  testqgsblendmodes.cpp

tests/src/core/testqgsauthcertutils.cpp

@@ -0,0 +1,78 @@
+/***************************************************************************
+     TestQgsAuthCertUtils.cpp
+     ----------------------
+    Date                 : October 2017
+    Copyright            : (C) 2017 by Boundless Spatial, Inc. USA
+    Author               : Larry Shaffer
+    Email                : lshaffer at boundlessgeo dot com
+ ***************************************************************************
+ *                                                                         *
+ *   This program is free software; you can redistribute it and/or modify  *
+ *   it under the terms of the GNU General Public License as published by  *
+ *   the Free Software Foundation; either version 2 of the License, or     *
+ *   (at your option) any later version.                                   *
+ *                                                                         *
+ ***************************************************************************/
+
+#include "qgstest.h"
+#include <QObject>
+#include <QSslKey>
+#include <QString>
+#include <QStringList>
+
+#include "qgsapplication.h"
+#include "qgsauthcrypto.h"
+#include "qgsauthcertutils.h"
+#include "qgslogger.h"
+
+/**
+ * \ingroup UnitTests
+ * Unit tests for QgsAuthCertUtils static functions
+ */
+class TestQgsAuthCertUtils: public QObject
+{
+    Q_OBJECT
+
+  private slots:
+    void initTestCase();
+    void cleanupTestCase();
+    void init() {}
+    void cleanup() {}
+
+    void testPkcsUtils();
+
+  private:
+    static QString sPkiData;
+};
+
+QString TestQgsAuthCertUtils::sPkiData = QStringLiteral( TEST_DATA_DIR ) + "/auth_system/certs_keys";
+
+void TestQgsAuthCertUtils::initTestCase()
+{
+  QgsApplication::init();
+  QgsApplication::initQgis();
+  if ( QgsAuthCrypto::isDisabled() )
+    QSKIP( "QCA's qca-ossl plugin is missing, skipping test case", SkipAll );
+}
+
+void TestQgsAuthCertUtils::cleanupTestCase()
+{
+  QgsApplication::exitQgis();
+}
+
+void TestQgsAuthCertUtils::testPkcsUtils()
+{
+  QByteArray pkcs;
+
+  pkcs = QgsAuthCertUtils::fileData( sPkiData + "/gerardus_key.pem", false );
+  QVERIFY( !pkcs.isEmpty() );
+  QVERIFY( !QgsAuthCertUtils::pemIsPkcs8( QString( pkcs ) ) );
+
+  pkcs.clear();
+  pkcs = QgsAuthCertUtils::fileData( sPkiData + "/gerardus_key-pkcs8-rsa.pem", false );
+  QVERIFY( !pkcs.isEmpty() );
+  QVERIFY( QgsAuthCertUtils::pemIsPkcs8( QString( pkcs ) ) );
+}
+
+QGSTEST_MAIN( TestQgsAuthCertUtils )
+#include "testqgsauthcertutils.moc"