Merge pull request #5631 from boundlessgeo/bd-2437-certs-format-sniffing

[auth] Moved the PEM/DER sniffing to a common private function
qgis · Nov 16, 2017 · dfc0305 · dfc0305
2 parents 8dd70c1 + 2a8ea5d
commit dfc0305
Show file tree

Hide file tree

Showing 6 changed files with 21 additions and 59 deletions.
diff --git a/src/auth/pkipaths/qgsauthpkipathsedit.cpp b/src/auth/pkipaths/qgsauthpkipathsedit.cpp
@@ -63,34 +63,7 @@ bool QgsAuthPkiPathsEdit::validateConfig()
   }
 
   // check for issue date validity, then notify status
-  QSslCertificate cert;
-  QFile file( certpath );
-  QFileInfo fileinfo( file );
-  QString ext( fileinfo.fileName().remove( fileinfo.completeBaseName() ).toLower() );
-  if ( ext.isEmpty() )
-  {
-    writePkiMessage( lePkiPathsMsg, tr( "Certificate file has no extension" ), Invalid );
-    return validityChange( false );
-  }
-
-  QFile::OpenMode openflags( QIODevice::ReadOnly );
-  QSsl::EncodingFormat encformat( QSsl::Der );
-  if ( ext == QLatin1String( ".pem" ) )
-  {
-    openflags |= QIODevice::Text;
-    encformat = QSsl::Pem;
-  }
-
-  if ( file.open( openflags ) )
-  {
-    cert = QSslCertificate( file.readAll(), encformat );
-    file.close();
-  }
-  else
-  {
-    writePkiMessage( lePkiPathsMsg, tr( "Failed to read certificate file" ), Invalid );
-    return validityChange( false );
-  }
+  QSslCertificate cert( QgsAuthCertUtils::certFromFile( certpath ) );
 
   if ( cert.isNull() )
   {
@@ -212,7 +185,7 @@ void QgsAuthPkiPathsEdit::chkPkiPathsPassShow_stateChanged( int state )
 void QgsAuthPkiPathsEdit::btnPkiPathsCert_clicked()
 {
   const QString &fn = QgsAuthGuiUtils::getOpenFileName( this, tr( "Open Client Certificate File" ),
-                      tr( "PEM (*.pem);;DER (*.der)" ) );
+                      tr( "All files (*.*);;PEM (*.pem);;DER (*.der)" ) );
   if ( !fn.isEmpty() )
   {
     lePkiPathsCert->setText( fn );
@@ -223,7 +196,7 @@ void QgsAuthPkiPathsEdit::btnPkiPathsCert_clicked()
 void QgsAuthPkiPathsEdit::btnPkiPathsKey_clicked()
 {
   const QString &fn = QgsAuthGuiUtils::getOpenFileName( this, tr( "Open Private Key File" ),
-                      tr( "PEM (*.pem);;DER (*.der)" ) );
+                      tr( "All files (*.*);;PEM (*.pem);;DER (*.der)" ) );
   if ( !fn.isEmpty() )
   {
     lePkiPathsKey->setText( fn );

diff --git a/src/core/auth/qgsauthcertutils.cpp b/src/core/auth/qgsauthcertutils.cpp
@@ -125,8 +125,8 @@ QByteArray QgsAuthCertUtils::fileData( const QString &path )
 QList<QSslCertificate> QgsAuthCertUtils::certsFromFile( const QString &certspath )
 {
   QList<QSslCertificate> certs;
-  bool pem = certspath.endsWith( QLatin1String( ".pem" ), Qt::CaseInsensitive );
-  certs = QSslCertificate::fromData( QgsAuthCertUtils::fileData( certspath ), pem ? QSsl::Pem : QSsl::Der );
+  const QByteArray payload( QgsAuthCertUtils::fileData( certspath ) );
+  certs = QSslCertificate::fromData( payload, sniffEncoding( payload ) );
   if ( certs.isEmpty() )
   {
     QgsDebugMsg( QString( "Parsed cert(s) EMPTY for path: %1" ).arg( certspath ) );
@@ -193,9 +193,7 @@ QSslKey QgsAuthCertUtils::keyFromFile( const QString &keypath,
   QByteArray keydata( QgsAuthCertUtils::fileData( keypath ) );
   QSslKey clientkey;
 
-  QSsl::EncodingFormat keyEncoding( keydata.contains( QByteArrayLiteral( "-----BEGIN " ) ) ?
-                                    QSsl::Pem :
-                                    QSsl::Der );
+  QSsl::EncodingFormat keyEncoding( sniffEncoding( keydata ) );
 
   const std::vector<QSsl::KeyAlgorithm> algs
   {
@@ -652,6 +650,13 @@ void QgsAuthCertUtils::appendDirSegment_( QStringList &dirname,
   }
 }
 
+QSsl::EncodingFormat QgsAuthCertUtils::sniffEncoding( const QByteArray &payload )
+{
+  return payload.contains( QByteArrayLiteral( "-----BEGIN " ) ) ?
+         QSsl::Pem :
+         QSsl::Der;
+}
+
 QString QgsAuthCertUtils::getCertDistinguishedName( const QSslCertificate &qcert,
     const QCA::Certificate &acert,
     bool issuer )

diff --git a/src/core/auth/qgsauthcertutils.h b/src/core/auth/qgsauthcertutils.h
@@ -373,6 +373,8 @@ class CORE_EXPORT QgsAuthCertUtils
 
   private:
     static void appendDirSegment_( QStringList &dirname, const QString &segment, QString value );
+
+    static QSsl::EncodingFormat sniffEncoding( const QByteArray &payload );
 };
 
 #endif // QGSAUTHCERTUTILS_H
diff --git a/src/gui/auth/qgsauthimportcertdialog.cpp b/src/gui/auth/qgsauthimportcertdialog.cpp
@@ -224,7 +224,7 @@ void QgsAuthImportCertDialog::validateCertificates()
 
 void QgsAuthImportCertDialog::btnImportFile_clicked()
 {
-  const QString &fn = getOpenFileName( tr( "Open Certificate File" ),  tr( "PEM (*.pem);;DER (*.der)" ) );
+  const QString &fn = getOpenFileName( tr( "Open Certificate File" ),  tr( "All files (*.*);;PEM (*.pem);;DER (*.der)" ) );
   if ( !fn.isEmpty() )
   {
     leImportFile->setText( fn );

diff --git a/src/gui/auth/qgsauthimportidentitydialog.cpp b/src/gui/auth/qgsauthimportidentitydialog.cpp
@@ -192,7 +192,7 @@ void QgsAuthImportIdentityDialog::chkPkiPathsPassShow_stateChanged( int state )
 
 void QgsAuthImportIdentityDialog::btnPkiPathsCert_clicked()
 {
-  const QString &fn = getOpenFileName( tr( "Open Client Certificate File" ),  tr( "PEM (*.pem);;DER (*.der)" ) );
+  const QString &fn = getOpenFileName( tr( "Open Client Certificate File" ),  tr( "All files (*.*);;PEM (*.pem);;DER (*.der)" ) );
   if ( !fn.isEmpty() )
   {
     lePkiPathsCert->setText( fn );
@@ -202,7 +202,7 @@ void QgsAuthImportIdentityDialog::btnPkiPathsCert_clicked()
 
 void QgsAuthImportIdentityDialog::btnPkiPathsKey_clicked()
 {
-  const QString &fn = getOpenFileName( tr( "Open Private Key File" ),  tr( "PEM (*.pem);;DER (*.der)" ) );
+  const QString &fn = getOpenFileName( tr( "Open Private Key File" ),  tr( "All files (*.*);;PEM (*.pem);;DER (*.der)" ) );
   if ( !fn.isEmpty() )
   {
     lePkiPathsKey->setText( fn );
@@ -287,26 +287,8 @@ bool QgsAuthImportIdentityDialog::validatePkiPaths()
   //TODO: set enabled on cert info button, relative to cert validity
 
   // check for valid private key and that any supplied password works
-  bool keypem = keypath.endsWith( QLatin1String( ".pem" ), Qt::CaseInsensitive );
-  QByteArray keydata( QgsAuthCertUtils::fileData( keypath ) );
-
-  QSslKey clientkey;
-  QString keypass = lePkiPathsKeyPass->text();
-  clientkey = QSslKey( keydata,
-                       QSsl::Rsa,
-                       keypem ? QSsl::Pem : QSsl::Der,
-                       QSsl::PrivateKey,
-                       !keypass.isEmpty() ? keypass.toUtf8() : QByteArray() );
-  if ( clientkey.isNull() )
-  {
-    // try DSA algorithm, since Qt can't seem to determine it otherwise
-    clientkey = QSslKey( keydata,
-                         QSsl::Dsa,
-                         keypem ? QSsl::Pem : QSsl::Der,
-                         QSsl::PrivateKey,
-                         !keypass.isEmpty() ? keypass.toUtf8() : QByteArray() );
-  }
-
+  QString keypass( lePkiPathsKeyPass->text() );
+  QSslKey clientkey( QgsAuthCertUtils::keyFromFile( keypath, keypass ) );
   if ( clientkey.isNull() )
   {
     writeValidation( tr( "Failed to load client private key from file" ), Invalid, true );

diff --git a/src/gui/auth/qgsauthsslimportdialog.cpp b/src/gui/auth/qgsauthsslimportdialog.cpp
@@ -369,7 +369,7 @@ void QgsAuthSslImportDialog::radioFileImportToggled( bool checked )
 
 void QgsAuthSslImportDialog::btnCertPath_clicked()
 {
-  const QString &fn = getOpenFileName( tr( "Open Server Certificate File" ),  tr( "PEM (*.pem);;DER (*.der)" ) );
+  const QString &fn = getOpenFileName( tr( "Open Server Certificate File" ),  tr( "All files (*.*);;PEM (*.pem);;DER (*.der)" ) );
   if ( !fn.isEmpty() )
   {
     leCertPath->setText( fn );