Certificate chain in OCSP is assumed to be valid

[Issue#] N/A
[Feature/Bug] N/A
[Problem] Not all certificate chains can be properly verified with OCSP
[Cause] Certificate chain in OCSP verification may contain custom trusted root
CAs as well as untrusted certificates which are not taken into account by
current implementation of OCSP.
[Solution] Chain submitted for verification is treated as valid (i.e. created
with get_certificate_chain() API) and therefore all issuers preceeding a
certificate being currently verified with OCSP are treated as trusted and are
used for OCSP response verification.

[Verification] Run ocsp tests

Change-Id: Ia96e6ba830abfd121f9adc041c55789cbf919cbc

diff --git a/src/include/ckmc/ckmc-manager.h b/src/include/ckmc/ckmc-manager.h
index 3703495..749c7a3 100644 (file)
--- a/src/include/ckmc/ckmc-manager.h
+++ b/src/include/ckmc/ckmc-manager.h
@@ -969,7 +969,7 @@ int ckmc_get_certificate_chain_with_alias(const ckmc_cert_s *cert,
  * @privlevel public
  * @privilege %http://tizen.org/privilege/keymanager
  *
- * @param[in] pcert_chain_list   Certificate chain to perform OCSP check
+ * @param[in] pcert_chain_list   Valid certificate chain to perform OCSP check
  * @param[out] ocsp_status       The pointer to status result of OCSP check
  *
  * @return @c 0 on success, otherwise a negative error value
@@ -979,6 +979,8 @@ int ckmc_get_certificate_chain_with_alias(const ckmc_cert_s *cert,
  * @retval #CKMC_ERROR_PERMISSION_DENIED    Failed to access key manager
  *
  * @pre User is already logged in and the user key is already loaded into memory in plain text form.
+ * @pre @a pcert_chain_list is created with ckmc_get_certificate_chain() or
+ *      ckmc_get_certificate_chain_with_alias()
  *
  * @see ckmc_get_cert_chain())
  * @see ckmc_cert_list_all_free()

diff --git a/src/manager/common/certificate-store.cpp b/src/manager/common/certificate-store.cpp
index 33df66c..565f4fd 100644 (file)
--- a/src/manager/common/certificate-store.cpp
+++ b/src/manager/common/certificate-store.cpp
@@ -28,14 +28,10 @@
 #include <certificate-config.h>
 #include <ckm/ckm-error.h>
 #include <ckm/ckm-type.h>
+#include <openssl_utils.h>
 
 namespace CKM {
 
-namespace {
-typedef std::unique_ptr<X509_STORE_CTX, void(*)(X509_STORE_CTX*)> X509_STORE_CTX_PTR;
-typedef std::unique_ptr<STACK_OF(X509), void(*)(STACK_OF(X509)*)> X509_STACK_PTR;
-}
-
 CertificateStore::CertificateStore() : m_store(X509_STORE_new())
 {
     if (!m_store) {
@@ -63,7 +59,7 @@ int CertificateStore::verifyCertificate(
              trustedVector.size() << "trusted certificates" << " and system certificates set to: "
              << useTrustedSystemCertificates);
 
-    X509_STORE_CTX_PTR csc(X509_STORE_CTX_new(),X509_STORE_CTX_free);
+    X509_STORE_CTX_PTR csc= create_x509_store_ctx();
     if (!csc) {
         LogError("failed to create csc");
         return CKM_API_ERROR_UNKNOWN;
@@ -84,7 +80,7 @@ int CertificateStore::verifyCertificate(
         return ret;
 
     // create stack of untrusted certificates
-    X509_STACK_PTR untrusted(sk_X509_new_null(), [](STACK_OF(X509)* stack) { sk_X509_free(stack); });
+    X509_STACK_PTR untrusted = create_x509_stack();
     if (!untrustedVector.empty()) {
         for (auto &e : untrustedVector) {
             // we don't want to free certificates because we wont create copies

diff --git a/src/manager/common/openssl_utils.h b/src/manager/common/openssl_utils.h
new file mode 100644 (file)
index 0000000..47b966b
--- /dev/null
+++ b/src/manager/common/openssl_utils.h
@@ -0,0 +1,43 @@
+/*
+ *  Copyright (c) 2000 - 2015 Samsung Electronics Co., Ltd All Rights Reserved
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License
+ */
+/*
+ * @file       openssl_utils.h
+ * @author     Krzysztof Jackiewicz (k.jackiewicz@samsung.com)
+ * @version    1.0
+ */
+
+#pragma once
+
+#include <openssl/x509.h>
+
+#include <memory>
+
+namespace CKM
+{
+
+typedef std::unique_ptr<X509_STORE_CTX, void(*)(X509_STORE_CTX*)> X509_STORE_CTX_PTR;
+typedef std::unique_ptr<STACK_OF(X509), void(*)(STACK_OF(X509)*)> X509_STACK_PTR;
+
+inline X509_STACK_PTR create_x509_stack() {
+    return X509_STACK_PTR(sk_X509_new_null(), [](STACK_OF(X509)* stack) { sk_X509_free(stack); });
+}
+inline X509_STORE_CTX_PTR create_x509_store_ctx() {
+    return X509_STORE_CTX_PTR(X509_STORE_CTX_new(),X509_STORE_CTX_free);
+}
+
+} // namespace CKM
+
+

diff --git a/src/manager/service/ocsp.cpp b/src/manager/service/ocsp.cpp
index 3422d33..711883c 100644 (file)
--- a/src/manager/service/ocsp.cpp
+++ b/src/manager/service/ocsp.cpp
@@ -31,7 +31,7 @@
 #include <key-manager-util.h>
 #include <dpl/log/log.h>
 #include <certificate-impl.h>
-
+#include <openssl_utils.h>
 #include <ckm/ckm-error.h>
 
 /* Maximum leeway in validity period: default 5 minutes */
@@ -69,12 +69,20 @@ OCSPModule::~OCSPModule(){
 int OCSPModule::verify(const CertificateImplVector &certificateChain) {
     bool unsupported = false; // ocsp is unsupported in certificate in chain (except root CA)
 
-    if((systemCerts = loadSystemCerts(CKM_SYSTEM_CERTS_PATH)) == NULL) {
-        LogError("Error in loadSystemCerts function");
-        return CKM_API_OCSP_STATUS_INTERNAL_ERROR;
+    // create trusted store
+    X509_STACK_PTR trustedCerts = create_x509_stack();
+
+    // skip first 2 certificates
+    for (auto it=certificateChain.cbegin()+2; it != certificateChain.cend(); it++)
+    {
+        if (it->empty()) {
+            LogError("Error. Broken certificate chain.");
+            return CKM_API_OCSP_STATUS_INTERNAL_ERROR;
+        }
+        sk_X509_push(trustedCerts.get(), it->getX509());
     }
 
-    for(unsigned int i=0; i < certificateChain.size() -1; i++) {// except root certificate
+    for (unsigned int i=0; i < certificateChain.size() -1; i++) {// except root certificate
         if (certificateChain[i].empty() || certificateChain[i+1].empty()) {
             LogError("Error. Broken certificate chain.");
             return CKM_API_OCSP_STATUS_INTERNAL_ERROR;
@@ -82,6 +90,7 @@ int OCSPModule::verify(const CertificateImplVector &certificateChain) {
 
         X509 *cert   = certificateChain[i].getX509();
         X509 *issuer = certificateChain[i+1].getX509();
+
         std::string url = certificateChain[i].getOCSPURL();
 
         if (url.empty()) {
@@ -90,7 +99,9 @@ int OCSPModule::verify(const CertificateImplVector &certificateChain) {
             continue;
         }
 
-        int result = ocsp_verify(cert, issuer, systemCerts, url);
+        int result = ocsp_verify(cert, issuer, trustedCerts.get(), url);
+        // remove first element from trustedCerts store
+        sk_X509_delete(trustedCerts.get(), 0);
 
         if(result != CKM_API_OCSP_STATUS_GOOD) {
             LogError("Fail to OCSP certification check. Errorcode=[" << result <<
@@ -105,7 +116,7 @@ int OCSPModule::verify(const CertificateImplVector &certificateChain) {
     return CKM_API_OCSP_STATUS_GOOD;
 }
 
-int OCSPModule::ocsp_verify(X509 *cert, X509 *issuer, STACK_OF(X509) *systemCerts, const std::string &constUrl) {
+int OCSPModule::ocsp_verify(X509 *cert, X509 *issuer, STACK_OF(X509) *trustedCerts, const std::string &constUrl) {
     OCSP_REQUEST *req = NULL;
     OCSP_RESPONSE *resp = NULL;
     OCSP_BASICRESP *bs = NULL;
@@ -278,10 +289,10 @@ int OCSPModule::ocsp_verify(X509 *cert, X509 *issuer, STACK_OF(X509) *systemCert
         return CKM_API_OCSP_STATUS_INVALID_RESPONSE;
     }
 
-    if(systemCerts != NULL) {
+    if(trustedCerts != NULL) {
         trustedStore = X509_STORE_new();
-        for(tmpIdx=0; tmpIdx<sk_X509_num(systemCerts); tmpIdx++) {
-            X509_STORE_add_cert(trustedStore, sk_X509_value(systemCerts, tmpIdx));
+        for(tmpIdx=0; tmpIdx<sk_X509_num(trustedCerts); tmpIdx++) {
+            X509_STORE_add_cert(trustedStore, sk_X509_value(trustedCerts, tmpIdx));
         }
         X509_STORE_add_cert(trustedStore, issuer);
     }

diff --git a/src/manager/service/ocsp.h b/src/manager/service/ocsp.h
index 7db5f2d..c6db766 100644 (file)
--- a/src/manager/service/ocsp.h
+++ b/src/manager/service/ocsp.h
@@ -37,8 +37,7 @@ public:
     // OK, UNKNOWN, REVOKED, NO_NETWORK, TIMEOUT
     int verify(const CertificateImplVector &certificateChain);
 private:
-    int ocsp_verify(X509 *cert, X509 *issuer, STACK_OF(X509) *systemCerts, const std::string &url);
-    STACK_OF(X509) *systemCerts;
+    int ocsp_verify(X509 *cert, X509 *issuer, STACK_OF(X509) *trustedCerts, const std::string &url);
 };
 
 } // namespace CKM