Skip to content
Permalink
Browse files

Merge PR #3322: SelfSignedCertificate: fix automatic certificate gene…

…rator

Previously, the generator was choosing the certificate type (server or client) basing on the presence of the name and email, which broke it.

This PR also includes various other small fixes.

Fixes #3284.
  • Loading branch information...
mkrautz committed Jan 27, 2018
2 parents 1273ba9 + b28c30a commit 16810dd562d05c954bf1c3fff2f5b242662e00d5
@@ -28,7 +28,7 @@ static int add_ext(X509 * crt, int nid, char *value) {
return 1;
}

bool SelfSignedCertificate::generate(QString clientCertName, QString clientCertEmail, QSslCertificate &qscCert, QSslKey &qskKey) {
bool SelfSignedCertificate::generate(CertificateType certificateType, QString clientCertName, QString clientCertEmail, QSslCertificate &qscCert, QSslKey &qskKey) {
bool ok = true;
X509 *x509 = NULL;
EVP_PKEY *pkey = NULL;
@@ -39,7 +39,7 @@ bool SelfSignedCertificate::generate(QString clientCertName, QString clientCertE
ASN1_TIME *notBefore = NULL;
ASN1_TIME *notAfter = NULL;
QString commonName;
bool isServerCert = clientCertName.isEmpty() && clientCertEmail.isEmpty();
bool isServerCert = certificateType == CertificateTypeServerCertificate;

if (CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_ON) == -1) {
ok = false;
@@ -185,9 +185,11 @@ bool SelfSignedCertificate::generate(QString clientCertName, QString clientCertE
}

if (!isServerCert) {
if (add_ext(x509, NID_subject_alt_name, QString::fromLatin1("email:%1").arg(clientCertEmail).toUtf8().data()) == 0) {
ok = false;
goto out;
if (!clientCertEmail.trimmed().isEmpty()) {
if (add_ext(x509, NID_subject_alt_name, QString::fromLatin1("email:%1").arg(clientCertEmail).toUtf8().data()) == 0) {
ok = false;
goto out;
}
}
}

@@ -214,6 +216,7 @@ bool SelfSignedCertificate::generate(QString clientCertName, QString clientCertE
qscCert = QSslCertificate(crt, QSsl::Der);
if (qscCert.isNull()) {
ok = false;
goto out;
}
}

@@ -235,6 +238,7 @@ bool SelfSignedCertificate::generate(QString clientCertName, QString clientCertE
qskKey = QSslKey(key, QSsl::Rsa, QSsl::Der);
if (qskKey.isNull()) {
ok = false;
goto out;
}
}

@@ -262,19 +266,9 @@ bool SelfSignedCertificate::generate(QString clientCertName, QString clientCertE
}

bool SelfSignedCertificate::generateMumbleCertificate(QString name, QString email, QSslCertificate &qscCert, QSslKey &qskKey) {
if (name.trimmed().isEmpty()) {
qscCert = QSslCertificate();
qskKey = QSslKey();
return false;
}
if (email.trimmed().isEmpty()) {
qscCert = QSslCertificate();
qskKey = QSslKey();
return false;
}
return SelfSignedCertificate::generate(name, email, qscCert, qskKey);
return SelfSignedCertificate::generate(CertificateTypeClientCertificate, name, email, qscCert, qskKey);
}

bool SelfSignedCertificate::generateMurmurV2Certificate(QSslCertificate &qscCert, QSslKey &qskKey) {
return SelfSignedCertificate::generate(QString(), QString(), qscCert, qskKey);
return SelfSignedCertificate::generate(CertificateTypeServerCertificate, QString(), QString(), qscCert, qskKey);
}
@@ -10,9 +10,11 @@
#include <QtNetwork/QSslCertificate>
#include <QtNetwork/QSslKey>

enum CertificateType { CertificateTypeServerCertificate, CertificateTypeClientCertificate };

class SelfSignedCertificate {
private:
static bool generate(QString clientCertName, QString clientCertEmail, QSslCertificate &qscCert, QSslKey &qskKey);
static bool generate(CertificateType certificateType, QString clientCertName, QString clientCertEmail, QSslCertificate &qscCert, QSslKey &qskKey);

public:
static bool generateMumbleCertificate(QString name, QString email, QSslCertificate &qscCert, QSslKey &qskKey);
@@ -34,12 +34,27 @@ void TestSelfSignedCertificate::exerciseClientCert() {
bool ok = SelfSignedCertificate::generateMumbleCertificate(QLatin1String("Test"), QLatin1String("test@test.test"), cert, key);
QCOMPARE(ok, true);
QCOMPARE(cert.isNull(), false);
QCOMPARE(cert.isNull(), false);
QCOMPARE(key.isNull(), false);

// Test that certificates are auto-generated correctly
ok = SelfSignedCertificate::generateMumbleCertificate(QString(), QString(), cert, key);
QCOMPARE(ok, false);
QCOMPARE(cert.isNull(), true);
QCOMPARE(cert.isNull(), true);
QCOMPARE(ok, true);
QCOMPARE(cert.isNull(), false);
QCOMPARE(key.isNull(), false);

// Test that users can create certificates without an email
// address set.
ok = SelfSignedCertificate::generateMumbleCertificate(QLatin1String("John Doe"), QString(), cert, key);
QCOMPARE(ok, true);
QCOMPARE(cert.isNull(), false);
QCOMPARE(key.isNull(), false);

// Test that it's possible to create a client certificate with
// both a name and an email.
ok = SelfSignedCertificate::generateMumbleCertificate(QLatin1String("John Doe"), QLatin1String("john@doe.family"), cert, key);
QCOMPARE(ok, true);
QCOMPARE(cert.isNull(), false);
QCOMPARE(key.isNull(), false);
}

void TestSelfSignedCertificate::exerciseServerCert() {
@@ -49,7 +64,7 @@ void TestSelfSignedCertificate::exerciseServerCert() {
bool ok = SelfSignedCertificate::generateMurmurV2Certificate(cert, key);
QCOMPARE(ok, true);
QCOMPARE(cert.isNull(), false);
QCOMPARE(cert.isNull(), false);
QCOMPARE(key.isNull(), false);
}

QTEST_MAIN(TestSelfSignedCertificate)

0 comments on commit 16810dd

Please sign in to comment.
You can’t perform that action at this time.