D17790: Port away from QCA

[Pino Toscano]

pino added inline comments.

INLINE COMMENTS

> kdeconnectconfig.cpp:198
>  
> -        // FIXME: We only use QCA here to generate the cert and key, would be nice to get rid of it completely.
> -        // The same thing we are doing with QCA could be done invoking openssl (although it's potentially less portable):
> -        // openssl req -new -x509 -sha256 -newkey rsa:2048 -nodes -keyout privateKey.pem -days 3650 -out certificate.pem -subj "/O=KDE/OU=KDE Connect/CN=_e6e29ad4_2b31_4b6d_8f7a_9872dbaa9095_"
> -
> -        QCA::CertificateOptions certificateOptions = QCA::CertificateOptions();
> -        QDateTime startTime = QDateTime::currentDateTime().addYears(-1);
> -        QDateTime endTime = startTime.addYears(10);
> -        QCA::CertificateInfo certificateInfo;
> -        certificateInfo.insert(QCA::CommonName, uuid);
> -        certificateInfo.insert(QCA::Organization,QStringLiteral("KDE"));
> -        certificateInfo.insert(QCA::OrganizationalUnit,QStringLiteral("Kde connect"));
> -        certificateOptions.setInfo(certificateInfo);
> -        certificateOptions.setFormat(QCA::PKCS10);
> -        certificateOptions.setSerialNumber(QCA::BigInteger(10));
> -        certificateOptions.setValidityPeriod(startTime, endTime);
> -
> -        d->m_certificate = QSslCertificate(QCA::Certificate(certificateOptions, d->m_privateKey).toPEM().toLatin1());
> -
> -        if (!cert.open(QIODevice::ReadWrite | QIODevice::Truncate))  {
> -            Daemon::instance()->reportError(QStringLiteral("KDE Connect"), i18n("Could not store certificate file: %1", certPath));
> -        } else {
> -            cert.setPermissions(strict);
> -            cert.write(d->m_certificate.toPem());
> -        }
> +        QProcess::execute("openssl", QStringLiteral("req -new -x509 -sha256 -newkey rsa:2048 -nodes -keyout %1 -days 3650 -out %2 -subj /O=KDE/OU=KDEConnect/CN=%3").arg(privateKeyPath(), certificatePath(), uuid).split(" "));
> +    }

- missing handling of the return value of the command (what if fails?)
- please do not split the arguments from a single string, but directly build a string list with the arguments (in case the paths contain spaces, for example)

> kdeconnectconfig.cpp:201
> +
> +    d->m_certificate = QSslCertificate::fromPath(certificatePath()).at(0);
> +

if QSslCertificate::fromPath fails, the return is an empty list, and this will misbehave

> kdeconnectconfig.cpp:206
> +    if (QFile::permissions(privateKeyPath()) != strict) {
> +        qCWarning(KDECONNECT_CORE) << "Warning: KDE Connect private key file has too open permissions " << privateKeyPath();
>      }

hint: output the wrong permissions too, so at least the warning gives more clues

> lanlinkprovidertest.cpp:317
> +    QTemporaryFile cert;
> +    cert.open();
> +    m_privateKeyFile.open();

this can fail, needs error handling

> lanlinkprovidertest.cpp:318
> +    cert.open();
> +    m_privateKeyFile.open();
> +

this can fail, needs error handling

> lanlinkprovidertest.cpp:320
> +
> +    QProcess::execute("openssl", QStringLiteral("req -new -x509 -sha256 -newkey rsa:2048 -nodes -keyout %1 -days 3650 -out %2 -subj /O=KDE/OU=KDEConnect/CN=%3").arg(m_privateKeyFile.fileName(), cert.fileName(), commonName).split(" "));
> +

- missing handling of the return value of the command (what if fails?)
- please do not split the arguments from a single string, but directly build a string list with the arguments (in case the paths contain spaces, for example)

> lanlinkprovidertest.cpp:322
> +
> +    QSslCertificate certificate = QSslCertificate::fromPath(cert.fileName()).at(0);
>      return certificate;

if QSslCertificate::fromPath fails, the return is an empty list, and this will misbehave

> testsslsocketlinereader.cpp:287
> +    QTemporaryFile cert;
> +    cert.open();
> +    QTemporaryFile priv;

this can fail, needs error handling

> testsslsocketlinereader.cpp:289
> +    QTemporaryFile priv;
> +    priv.open();
>  

this can fail, needs error handling

> testsslsocketlinereader.cpp:291
>  
> -    QCA::CertificateOptions certificateOptions(QCA::PKCS10);
> -    certificateOptions.setSerialNumber(10);
> -    certificateOptions.setInfo(certificateInfo);
> -    certificateOptions.setValidityPeriod(startTime, endTime);
> -    certificateOptions.setFormat(QCA::PKCS10);
> +    QProcess::execute("openssl", QStringLiteral("req -new -x509 -sha256 -newkey rsa:2048 -nodes -keyout %1 -days 3650 -out %2 -subj /O=KDE/OU=KDEConnect/CN=%3").arg(priv.fileName(), cert.fileName(), deviceName).split(" "));
>  

- missing handling of the return value of the command (what if fails?)
- please do not split the arguments from a single string, but directly build a string list with the arguments (in case the paths contain spaces, for example)

> testsslsocketlinereader.cpp:293
>  
> -    QCA::PrivateKey privKey = QCA::KeyGenerator().createRSA(2048);
> -    QSslCertificate certificate = QSslCertificate(QCA::Certificate(certificateOptions, privKey).toPEM().toLatin1());
> +    QSslCertificate certificate = QSslCertificate::fromPath(cert.fileName()).at(0);
>  

if QSslCertificate::fromPath fails, the return is an empty list, and this will misbehave

REVISION DETAIL
  https://phabricator.kde.org/D17790

To: nicolasfella, #kde_connect
Cc: pino, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdeconnect/attachments/20181225/77339b6e/attachment-0001.html>