Some API improvements and a bug fix (#18)

* Clang Tidy fixes

* Fix reading of .crt files with GnuTLS

* Hide d_crypto pointer from public API

* Read public key type also from .pem files

---------

Co-authored-by: cesaaargm <cesare.martinez@proton.me>

src/core/libs/CMakeLists.txt

@@ -93,7 +93,6 @@ target_link_libraries(core_libs
         core_libs_supl
         core_system_parameters
         pvt_libs
-        algorithms_libs
     PRIVATE
         algorithms_libs
         Boost::serialization

src/core/libs/osnma_msg_receiver.cc

@@ -22,9 +22,9 @@
 #include "Galileo_OSNMA.h"
 #include "gnss_crypto.h"
 #include "gnss_satellite.h"
-#include "osnma_dsm_reader.h"  // for OSNMA_DSM_Reader
+#include "gnss_sdr_make_unique.h"   // for std::make_unique in C++11
-#include "osnma_helper.h"
+#include "osnma_dsm_reader.h"       // for OSNMA_DSM_Reader
-#include "osnma_nav_data_manager.h"
+#include "osnma_helper.h"           // for Osnma_Helper
 #include <gnuradio/io_signature.h>  // for gr::io_signature::make
 #include <algorithm>
 #include <cmath>
@@ -237,6 +237,12 @@ void osnma_msg_receiver::msg_handler_osnma(const pmt::pmt_t& msg)
 }
 
 
+void osnma_msg_receiver::read_merkle_xml(const std::string& merklepath)
+{
+    d_crypto->read_merkle_xml(merklepath);
+}
+
+
 void osnma_msg_receiver::process_osnma_message(const std::shared_ptr<OSNMA_msg>& osnma_msg)
 {
     if (d_flag_alert_message && (d_public_key_verified || d_kroot_verified))

src/core/libs/osnma_msg_receiver.h

@@ -23,21 +23,20 @@
 #define FRIEND_TEST(test_case_name, test_name) \
     friend class test_case_name##_##test_name##_Test
 
-#include "galileo_inav_message.h"  // for OSNMA_msg
+#include "galileo_inav_message.h"    // for OSNMA_msg
-#include "gnss_block_interface.h"  // for gnss_shared_ptr
+#include "gnss_block_interface.h"    // for gnss_shared_ptr
-#include "gnss_sdr_make_unique.h"  // for std::make:unique in C++11
+#include "osnma_data.h"              // for OSNMA_data structures
-#include "osnma_data.h"            // for OSNMA_data structures
+#include "osnma_nav_data_manager.h"  // for OSNMA_nav_data_Manager
-#include "osnma_nav_data_manager.h"
+#include <gnuradio/block.h>          // for gr::block
-#include <gnuradio/block.h>  // for gr::block
+#include <pmt/pmt.h>                 // for pmt::pmt_t
-#include <pmt/pmt.h>         // for pmt::pmt_t
+#include <array>                     // for std::array
-#include <array>             // for std::array
+#include <cstdint>                   // for uint8_t
-#include <cstdint>           // for uint8_t
+#include <ctime>                     // for std::time_t
-#include <ctime>             // for std::time_t
+#include <map>                       // for std::map, std::multimap
-#include <map>               // for std::map, std::multimap
+#include <memory>                    // for std::shared_ptr
-#include <memory>            // for std::shared_ptr
+#include <string>                    // for std::string
-#include <string>            // for std::string
+#include <utility>                   // for std::pair
-#include <utility>           // for std::pair
+#include <vector>                    // for std::vector
-#include <vector>            // for std::vector
 
 /** \addtogroup Core
  * \{ */
@@ -62,9 +61,10 @@ osnma_msg_receiver_sptr osnma_msg_receiver_make(const std::string& pemFilePath,
 class osnma_msg_receiver : public gr::block
 {
 public:
-    ~osnma_msg_receiver() = default;                //!< Default destructor
+    ~osnma_msg_receiver() = default;                      //!< Default destructor
-    std::unique_ptr<Gnss_Crypto> d_crypto;          // access to cryptographic functions
+    void msg_handler_osnma(const pmt::pmt_t& msg);        //!< For testing purposes
-    void msg_handler_osnma(const pmt::pmt_t& msg);  // GnssCrypto and the message handler are needed by public method within TestVectors fixture
+    void read_merkle_xml(const std::string& merklepath);  //!< Public for testing purposes
+
 private:
     friend osnma_msg_receiver_sptr osnma_msg_receiver_make(const std::string& pemFilePath, const std::string& merkleFilePath, bool strict_mode);
     osnma_msg_receiver(const std::string& crtFilePath, const std::string& merkleFilePath, bool strict_mode);
@@ -112,6 +112,7 @@ private:
     std::array<uint16_t, 16> d_number_of_blocks{};
     std::array<uint8_t, 60> d_mack_message{};  // C: 480 b
 
+    std::unique_ptr<Gnss_Crypto> d_crypto;                       // class for cryptographic functions
     std::unique_ptr<OSNMA_DSM_Reader> d_dsm_reader;              // osnma parameters parser
     std::unique_ptr<Osnma_Helper> d_helper;                      // helper class with auxiliary functions
     std::unique_ptr<OSNMA_nav_data_Manager> d_nav_data_manager;  // refactor for holding and processing navigation data

src/core/system_parameters/gnss_crypto.cc

@@ -20,6 +20,7 @@
 #include "Galileo_OSNMA.h"
 #include <pugixml.hpp>
 #include <cstddef>
+#include <cstring>
 #include <fstream>
 #include <iomanip>
 #include <iostream>
@@ -912,11 +913,14 @@ void Gnss_Crypto::set_public_key(const std::vector<uint8_t>& publicKey)
     point = EC_POINT_new(group);
     if (!point)
         {
+            EC_GROUP_free(group);
             return;
         }
 
     if (!EC_POINT_oct2point(group, point, publicKey.data(), publicKey.size(), nullptr))
         {
+            EC_GROUP_free(group);
+            EC_POINT_free(point);
             return;
         }
 
@@ -930,11 +934,16 @@ void Gnss_Crypto::set_public_key(const std::vector<uint8_t>& publicKey)
         }
     if (!ec_key)
         {
+            EC_GROUP_free(group);
+            EC_POINT_free(point);
             return;
         }
 
     if (!EC_KEY_set_public_key(ec_key, point))
         {
+            EC_KEY_free(ec_key);
+            EC_POINT_free(point);
+            EC_GROUP_free(group);
             return;
         }
     if (!pubkey_copy(ec_key, &d_PublicKey))
@@ -1066,6 +1075,7 @@ void Gnss_Crypto::readPublicKeyFromPEM(const std::string& pemFilePath)
         {
             return;
         }
+    d_PublicKeyType = "Unknown";
     std::string pemContent((std::istreambuf_iterator<char>(pemFile)), std::istreambuf_iterator<char>());
 #if USE_GNUTLS_FALLBACK
     // Import the PEM data
@@ -1085,6 +1095,52 @@ void Gnss_Crypto::readPublicKeyFromPEM(const std::string& pemFilePath)
             return;
         }
 
+    // store the key type - needed for the Kroot in case no DSM-PKR available
+    gnutls_pk_algorithm_t pk_algorithm;
+    unsigned int bits;
+
+    ret = gnutls_pubkey_get_pk_algorithm(pubkey, &bits);
+    if (ret < 0)
+        {
+            LOG(WARNING) << "GnuTLS: Failed to get public key algorithm from .pem file: " << gnutls_strerror(ret);
+            gnutls_pubkey_deinit(pubkey);
+            return;
+        }
+
+    pk_algorithm = static_cast<gnutls_pk_algorithm_t>(ret);
+
+    if (pk_algorithm == GNUTLS_PK_ECDSA)
+        {
+            gnutls_ecc_curve_t curve;
+            ret = gnutls_pubkey_export_ecc_raw(pubkey, &curve, nullptr, nullptr);
+            if (ret < 0)
+                {
+                    LOG(WARNING) << "GnuTLS: Failed to get EC curve from .pem file: " << gnutls_strerror(ret);
+                    gnutls_pubkey_deinit(pubkey);
+                    return;
+                }
+
+            if (curve == GNUTLS_ECC_CURVE_SECP256R1)
+                {
+                    d_PublicKeyType = "ECDSA P-256";
+                }
+            else if (curve == GNUTLS_ECC_CURVE_SECP521R1)
+                {
+                    d_PublicKeyType = "ECDSA P-521";
+                }
+            else
+                {
+                    LOG(WARNING) << "GnuTLS: Trying to read unknown EC curve from .pem file";
+                    gnutls_pubkey_deinit(pubkey);
+                    return;
+                }
+        }
+    else
+        {
+            LOG(WARNING) << "GnuTLS: Trying to read unknown key type from .pem file";
+            gnutls_pubkey_deinit(pubkey);
+            return;
+        }
     pubkey_copy(pubkey, &d_PublicKey);
     gnutls_pubkey_deinit(pubkey);
 #else  // OpenSSL
@@ -1096,9 +1152,77 @@ void Gnss_Crypto::readPublicKeyFromPEM(const std::string& pemFilePath)
             return;
         }
 #if USE_OPENSSL_3
-    d_PublicKey = PEM_read_bio_PUBKEY(bio, nullptr, nullptr, nullptr);
+    EVP_PKEY* pubkey = nullptr;
+    pubkey = PEM_read_bio_PUBKEY(bio, nullptr, nullptr, nullptr);
+
+    // store the key type - needed for the Kroot in case no DSM-PKR available
+    // Get the key type
+    int key_type = EVP_PKEY_base_id(pubkey);
+    if (key_type == EVP_PKEY_EC)
+        {
+            // It's an EC key, now we need to determine the curve
+            char curve_name[256];
+            size_t curve_name_len = sizeof(curve_name);
+
+            if (EVP_PKEY_get_utf8_string_param(pubkey, OSSL_PKEY_PARAM_GROUP_NAME, curve_name, curve_name_len, &curve_name_len) == 1)
+                {
+                    if (strcmp(curve_name, "prime256v1") == 0 || strcmp(curve_name, "P-256") == 0)
+                        {
+                            d_PublicKeyType = "ECDSA P-256";
+                        }
+                    else if (strcmp(curve_name, "secp521r1") == 0 || strcmp(curve_name, "P-521") == 0)
+                        {
+                            d_PublicKeyType = "ECDSA P-521";
+                        }
+                    else
+                        {
+                            LOG(WARNING) << "OpenSSL: Trying to read an unknown EC curve from .pem file";
+                            BIO_free(bio);
+                            EVP_PKEY_free(pubkey);
+                            return;
+                        }
+                }
+            else
+                {
+                    LOG(WARNING) << "OpenSSL: Trying to read an unknown EC curve from .pem file";
+                    BIO_free(bio);
+                    EVP_PKEY_free(pubkey);
+                    return;
+                }
+        }
+    else
+        {
+            LOG(WARNING) << "OpenSSL: Trying to read an unknown key type from .pem file";
+            BIO_free(bio);
+            EVP_PKEY_free(pubkey);
+            return;
+        }
+    pubkey_copy(pubkey, &d_PublicKey);
+    EVP_PKEY_free(pubkey);
 #else  // OpenSSL 1.x
-    d_PublicKey = PEM_read_bio_EC_PUBKEY(bio, nullptr, nullptr, nullptr);
+    EC_KEY* pubkey = nullptr;
+    pubkey = PEM_read_bio_EC_PUBKEY(bio, nullptr, nullptr, nullptr);
+    if (!pubkey)
+        {
+            LOG(WARNING) << "OpenSSL: Failed to extract the public key from .pem file";
+            BIO_free(bio);
+            return;
+        }
+    const EC_GROUP* group = EC_KEY_get0_group(pubkey);
+    int nid = EC_GROUP_get_curve_name(group);
+    const char* curve_name = OBJ_nid2sn(nid);
+    const std::string curve_str(curve_name);
+    if (curve_str == "prime256v1")
+        {
+            d_PublicKeyType = "ECDSA P-256";
+        }
+    else if (curve_str == "secp521r1")
+        {
+            d_PublicKeyType = "ECDSA P-521";
+        }
+
+    pubkey_copy(pubkey, &d_PublicKey);
+    EC_KEY_free(pubkey);
 #endif
     BIO_free(bio);
     if (d_PublicKey == nullptr)
@@ -1172,20 +1296,8 @@ bool Gnss_Crypto::readPublicKeyFromCRT(const std::string& crtFilePath)
 
     if (pk_algorithm == GNUTLS_PK_ECDSA)
         {
-            gnutls_datum_t params;
-            ret = gnutls_pubkey_export_ecc_raw(pubkey, nullptr, &params, nullptr);
-            if (ret < 0)
-                {
-                    LOG(WARNING) << "GnuTLS: Failed to export EC parameters: " << gnutls_strerror(ret);
-                    gnutls_pubkey_deinit(pubkey);
-                    gnutls_x509_crt_deinit(cert);
-                    return false;
-                }
-
             gnutls_ecc_curve_t curve;
-            ret = gnutls_ecc_curve_get_id(reinterpret_cast<const char*>(params.data));
+            ret = gnutls_pubkey_export_ecc_raw(pubkey, &curve, nullptr, nullptr);
-            gnutls_free(params.data);
-
             if (ret < 0)
                 {
                     LOG(WARNING) << "GnuTLS: Failed to get EC curve: " << gnutls_strerror(ret);
@@ -1194,8 +1306,6 @@ bool Gnss_Crypto::readPublicKeyFromCRT(const std::string& crtFilePath)
                     return false;
                 }
 
-            curve = static_cast<gnutls_ecc_curve_t>(ret);
-
             if (curve == GNUTLS_ECC_CURVE_SECP256R1)
                 {
                     d_PublicKeyType = "ECDSA P-256";

src/tests/unit-tests/signal-processing-blocks/osnma/gnss_crypto_test.cc

@@ -70,6 +70,7 @@ TEST(GnssCryptoTest, VerifyPublicKeyStorage)
     auto d_crypto2 = std::make_unique<Gnss_Crypto>(f1, "");
     bool result2 = d_crypto2->store_public_key(f2);
     ASSERT_TRUE(result2);
+    ASSERT_TRUE(d_crypto2->get_public_key_type() == "ECDSA P-256");
 
     std::ifstream t(f1);
     std::string content_file((std::istreambuf_iterator<char>(t)), std::istreambuf_iterator<char>());

src/tests/unit-tests/signal-processing-blocks/osnma/osnma_test_vectors.cc

@@ -243,7 +243,7 @@ bool OsnmaTestVectors::feedOsnmaWithTestVectors(osnma_msg_receiver_sptr osnma_ob
 
             if (test_step == 1 && d_flag_NPK == true ){
                     // step 2: this simulates the osnma connecting to the GSC server and downloading the Merkle tree of the next public key
-                    osnma_object->d_crypto->read_merkle_xml(
+                    osnma_object->read_merkle_xml(
                         std::string(BASE_OSNMA_TEST_VECTORS) + "cryptographic_material/Merkle_tree_2/MerkleTree/OSNMA_MerkleTree_20231007081500_PKID_8.xml");
                 }