Skip to content

Commit

Permalink
[auth][bugfix] Import pvt keys with unknown file extension
Browse files Browse the repository at this point in the history
This fixes an unreported bug that prevent imports of
private keys with wrong/unknown extension.

The old logic relied on the file extension, that is
not only weak but plain wrong because the same extension
can have different encodings.

The new implementation is 100% robust because completely
ignores the file extentions and try to load the key with
all supported encodings and algorithms before giving up.
  • Loading branch information
elpaso committed Nov 8, 2017
1 parent 940c4ed commit d09d704
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 48 deletions.
3 changes: 1 addition & 2 deletions python/core/auth/qgsauthcertutils.sip
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,10 @@ Map certificate sha1 to certificate as simple cache
%End


static QByteArray fileData( const QString &path, bool astext = false );
static QByteArray fileData( const QString &path );
%Docstring
Return data from a local file via a read-only operation
\param path Path to file to read
\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
Expand Down
60 changes: 38 additions & 22 deletions src/core/auth/qgsauthcertutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ QMap<QString, QList<QgsAuthConfigSslServer> > QgsAuthCertUtils::sslConfigsGroupe
return orgconfigs;
}

QByteArray QgsAuthCertUtils::fileData( const QString &path, bool astext )
QByteArray QgsAuthCertUtils::fileData( const QString &path )
{
QByteArray data;
QFile file( path );
Expand All @@ -112,8 +112,6 @@ QByteArray QgsAuthCertUtils::fileData( const QString &path, bool astext )
}
// 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 )
{
Expand All @@ -128,7 +126,7 @@ 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 ), pem ? QSsl::Pem : QSsl::Der );
certs = QSslCertificate::fromData( QgsAuthCertUtils::fileData( certspath ), pem ? QSsl::Pem : QSsl::Der );
if ( certs.isEmpty() )
{
QgsDebugMsg( QString( "Parsed cert(s) EMPTY for path: %1" ).arg( certspath ) );
Expand Down Expand Up @@ -191,37 +189,55 @@ QSslKey QgsAuthCertUtils::keyFromFile( const QString &keypath,
const QString &keypass,
QString *algtype )
{
bool pem = keypath.endsWith( QLatin1String( ".pem" ), Qt::CaseInsensitive );
QByteArray keydata( QgsAuthCertUtils::fileData( keypath, pem ) );
// The approach here is to try all possible encodings and algorithms
QByteArray keydata( QgsAuthCertUtils::fileData( keypath ) );

QSslKey clientkey;
clientkey = QSslKey( keydata,
QSsl::Rsa,
pem ? QSsl::Pem : QSsl::Der,
QSsl::Pem,
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,
pem ? QSsl::Pem : QSsl::Der,
QSsl::PrivateKey,
!keypass.isEmpty() ? keypass.toUtf8() : QByteArray() );
if ( clientkey.isNull() )
{
return QSslKey();
}
if ( ! clientkey.isNull() )
{
if ( algtype )
*algtype = QStringLiteral( "rsa" );
return clientkey;
}
clientkey = QSslKey( keydata,
QSsl::Dsa,
QSsl::Pem,
QSsl::PrivateKey,
!keypass.isEmpty() ? keypass.toUtf8() : QByteArray() );
if ( ! clientkey.isNull() )
{
if ( algtype )
*algtype = QStringLiteral( "dsa" );
return clientkey;
}
else
clientkey = QSslKey( keydata,
QSsl::Rsa,
QSsl::Der,
QSsl::PrivateKey,
!keypass.isEmpty() ? keypass.toUtf8() : QByteArray() );
if ( ! clientkey.isNull() )
{
if ( algtype )
*algtype = QStringLiteral( "rsa" );
return clientkey;
}

return clientkey;
clientkey = QSslKey( keydata,
QSsl::Dsa,
QSsl::Der,
QSsl::PrivateKey,
!keypass.isEmpty() ? keypass.toUtf8() : QByteArray() );
if ( ! clientkey.isNull() )
{
if ( algtype )
*algtype = QStringLiteral( "dsa" );
return clientkey;
}
return QSslKey();
}

QList<QSslCertificate> QgsAuthCertUtils::certsFromString( const QString &pemtext )
Expand Down
3 changes: 1 addition & 2 deletions src/core/auth/qgsauthcertutils.h
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,9 @@ class CORE_EXPORT QgsAuthCertUtils
/**
* Return data from a local file via a read-only operation
* \param path Path to file to read
* \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 );
static QByteArray fileData( const QString &path );

//! Return list of concatenated certs from a PEM or DER formatted file
static QList<QSslCertificate> certsFromFile( const QString &certspath );
Expand Down
23 changes: 2 additions & 21 deletions src/core/auth/qgsauthconfig.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -183,35 +183,16 @@ const QgsPkiBundle QgsPkiBundle::fromPemPaths( const QString &certPath,
if ( !certPath.isEmpty() && !keyPath.isEmpty()
&& ( certPath.endsWith( QLatin1String( ".pem" ), Qt::CaseInsensitive )
|| certPath.endsWith( QLatin1String( ".der" ), Qt::CaseInsensitive ) )
&& ( keyPath.endsWith( QLatin1String( ".pem" ), Qt::CaseInsensitive )
|| keyPath.endsWith( QLatin1String( ".der" ), Qt::CaseInsensitive ) )
&& QFile::exists( certPath ) && QFile::exists( keyPath )
)
{
// client cert
bool pem = certPath.endsWith( QLatin1String( ".pem" ), Qt::CaseInsensitive );
QSslCertificate clientcert( QgsAuthCertUtils::fileData( certPath, pem ), pem ? QSsl::Pem : QSsl::Der );
QSslCertificate clientcert( QgsAuthCertUtils::fileData( certPath ), pem ? QSsl::Pem : QSsl::Der );
pkibundle.setClientCert( clientcert );

// client key
bool pem_key = keyPath.endsWith( QLatin1String( ".pem" ), Qt::CaseInsensitive );
QByteArray keydata( QgsAuthCertUtils::fileData( keyPath, pem_key ) );

QSslKey clientkey;
clientkey = QSslKey( keydata,
QSsl::Rsa,
pem_key ? QSsl::Pem : QSsl::Der,
QSsl::PrivateKey,
!keyPass.isNull() ? keyPass.toUtf8() : QByteArray() );
if ( clientkey.isNull() )
{
// try DSA algorithm, since Qt can't seem to determine it otherwise
clientkey = QSslKey( keydata,
QSsl::Dsa,
pem_key ? QSsl::Pem : QSsl::Der,
QSsl::PrivateKey,
!keyPass.isNull() ? keyPass.toUtf8() : QByteArray() );
}
clientkey = QgsAuthCertUtils::keyFromFile( keyPath, keyPass );
pkibundle.setClientKey( clientkey );
if ( !caChain.isEmpty() )
{
Expand Down
2 changes: 1 addition & 1 deletion src/gui/auth/qgsauthimportidentitydialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,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( QgsAuthCertUtils::fileData( keypath, keypem ) );
QByteArray keydata( QgsAuthCertUtils::fileData( keypath ) );

QSslKey clientkey;
QString keypass = lePkiPathsKeyPass->text();
Expand Down

0 comments on commit d09d704

Please sign in to comment.