From 356cab34016ff1dd998ca87b8da964d88ff1546a Mon Sep 17 00:00:00 2001 From: Christian Hohnstaedt Date: Tue, 3 Mar 2009 21:45:00 +0100 Subject: [PATCH] check for duplicate x509 v3 extensions - while taking extensions from the request, the advanced tab and the other tabs, extensions may be duplicated. They will be now diplayed in detail and duplicates are found and shown clearly. Warning message will allow for modifications. - Fixes [ 1881482 ] Copy extensions from request seems to fail [ 1998815 ] xca adds basic constraint "CA" twice resulting in invalid CA --- lib/db_x509.cpp | 2 -- lib/x509v3ext.cpp | 10 ++++++++ lib/x509v3ext.h | 1 + widgets/NewX509.cpp | 21 ++++++++++++++- widgets/NewX509.h | 3 ++- widgets/NewX509_ext.cpp | 57 +++++++++++++++++++++++++++++++---------- 6 files changed, 76 insertions(+), 18 deletions(-) diff --git a/lib/db_x509.cpp b/lib/db_x509.cpp index 8560ceab..c6921375 100644 --- a/lib/db_x509.cpp +++ b/lib/db_x509.cpp @@ -486,7 +486,6 @@ void db_x509::newCert(NewX509 *dlg) throw errorEx(""); } - // STEP 4 handle extensions if (dlg->copyReqExtCB->isChecked() && dlg->fromReqCB->isChecked()) { extList el = req->getV3ext(); @@ -497,7 +496,6 @@ void db_x509::newCert(NewX509 *dlg) // apply all extensions to the subject cert in the context dlg->getAllExt(); - dlg->checkExtDuplicates(); const EVP_MD *hashAlgo = dlg->hashAlgo->currentHash(); #ifdef WG_QA_SERIAL diff --git a/lib/x509v3ext.cpp b/lib/x509v3ext.cpp index 9a29adc2..11218a7d 100644 --- a/lib/x509v3ext.cpp +++ b/lib/x509v3ext.cpp @@ -177,6 +177,16 @@ int extList::delByNid(int nid) return removed; } +int extList::idxByNid(int nid) +{ + for(int i = 0; i< size(); i++) { + if (at(i).nid() == nid) { + return i; + } + } + return -1; +} + int extList::delInvalid(void) { int removed=0; diff --git a/lib/x509v3ext.h b/lib/x509v3ext.h index 4e5fecfd..f776c597 100644 --- a/lib/x509v3ext.h +++ b/lib/x509v3ext.h @@ -45,5 +45,6 @@ class extList : public QList QString getHtml(const QString &sep); int delByNid(int nid); int delInvalid(); + int idxByNid(int nid); }; #endif diff --git a/widgets/NewX509.cpp b/widgets/NewX509.cpp index 66ab5938..b6332738 100644 --- a/widgets/NewX509.cpp +++ b/widgets/NewX509.cpp @@ -163,6 +163,7 @@ void NewX509::setRequest() signerBox->setEnabled(false); timewidget->setEnabled(false); capt->setText(tr("Create Certificate signing request")); + authKey->setEnabled(false); setImage(MainWindow::csrImg); pt = x509_req; } @@ -651,7 +652,7 @@ void NewX509::on_adv_validate_clicked() QString result; setupTmpCtx(); v3ext_backup = nconf_data->toPlainText(); - if (fromReqCB->isChecked()) { + if (fromReqCB->isChecked() && copyReqExtCB->isChecked()) { el = getSelectedReq()->getV3ext(); } if (el.size() > 0) { @@ -689,11 +690,13 @@ void NewX509::on_adv_validate_clicked() nconf_data->setReadOnly(true); adv_validate->setText(tr("Edit")); + valid_htmltext = result; checkExtDuplicates(); } else { nconf_data->document()->setPlainText(v3ext_backup); nconf_data->setReadOnly(false); adv_validate->setText(tr("Validate")); + valid_htmltext = ""; } pki_base::ign_openssl_error(); } @@ -843,5 +846,21 @@ void NewX509::on_okButton_clicked() return; } } + on_adv_validate_clicked(); + if (checkExtDuplicates()) { + switch (QMessageBox::warning(this, tr(XCA_TITLE), + tr("The certificate contains duplicated extensions. " + "Check the validation on the advanced tab."), + tr("Ok"), tr("Abort rollout"), tr("Continue rollout"))) + { + case -1: + case 0: + return; + case 1: + reject(); + return; + } + } + accept(); } diff --git a/widgets/NewX509.h b/widgets/NewX509.h index 2b1d799f..b3fe76a0 100644 --- a/widgets/NewX509.h +++ b/widgets/NewX509.h @@ -47,6 +47,7 @@ class NewX509: public QDialog, public Ui::NewX509 QStringList private_keys, private_keys0; pki_x509 *ctx_cert; QString v3ext_backup; + QString valid_htmltext; public: QRadioButton *selfQASignRB; NewX509(QWidget *parent); @@ -90,7 +91,7 @@ class NewX509: public QDialog, public Ui::NewX509 void setExt(const x509v3ext &ext); void switchHashAlgo(); void addReqAttributes(pki_x509req *req); - void checkExtDuplicates(); + int checkExtDuplicates(); public slots: void on_fromReqCB_clicked(); void on_keyList_currentIndexChanged(const QString &); diff --git a/widgets/NewX509_ext.cpp b/widgets/NewX509_ext.cpp index 1a2f5239..7cb0ab4a 100644 --- a/widgets/NewX509_ext.cpp +++ b/widgets/NewX509_ext.cpp @@ -207,6 +207,9 @@ extList NewX509::getAdvanced() char ext_name[] = "ext"; int ret, i, start; + if (nconf_data->isReadOnly()) { + on_adv_validate_clicked(); + } conf_str = nconf_data->toPlainText(); if (conf_str.isEmpty()) return elist; @@ -215,7 +218,10 @@ extList NewX509::getAdvanced() conf_str = QString("[") + ext_name + "]\n"; for (i=0; i< list.count(); i++) { - conf_str += list[i].trimmed() + "\n"; + QString s = list[i].trimmed(); + if (!s.isEmpty()){ + conf_str += s + "\n"; + } } bio = BIO_new_mem_buf((void*)CCHAR(conf_str), conf_str.length()); if (!bio) @@ -319,32 +325,55 @@ void NewX509::initCtx(pki_x509 *subj, pki_x509 *iss, pki_x509req *req) X509V3_set_ctx(&ext_ctx, s, s1, r, NULL, 0); } -void NewX509::checkExtDuplicates() +int NewX509::checkExtDuplicates() { int i, start, cnt, n1, n; - X509_EXTENSION *e, *e1; + x509v3ext e; STACK_OF(X509_EXTENSION) *sk; + extList el_dup, el; + QString olist; if (ext_ctx.subject_cert) { sk = ext_ctx.subject_cert->cert_info->extensions; } else - return; + return 0; - cnt = sk_X509_EXTENSION_num(sk); - for (start=0; startisChecked() && copyReqExtCB->isChecked()) { + el += getSelectedReq()->getV3ext(); + } + + cnt = el.size(); + for (start=0; start < cnt; start++) { + n1 = el[start].nid(); + for (i = start+1; isetCurrentIndex(tabWidget->count() -1); + if (!nconf_data->isReadOnly()) { + on_adv_validate_clicked(); + } + + olist = "

Error: " + "duplicate extensions:

    \n"; + for(int i = 0; i< el_dup.size(); i++) { + olist += "
  • " + el_dup[i].getObject() + "
    • \n"; + } + olist += "
\n
\n"; + olist += valid_htmltext; + nconf_data->document()->setHtml(olist); + return el_dup.size(); } void NewX509::setExt(const x509v3ext &ext)