fix(OSQUERY-001): 21 review findings across 7 files

openframe/openframe_encryption_service.cpp

@@ -1,26 +1,44 @@
+/**
+ * Copyright (c) 2014-present, The osquery authors
+ *
+ * This source code is licensed as defined by the LICENSE file found in the
+ * root directory of this source tree.
+ *
+ * SPDX-License-Identifier: (Apache-2.0 OR GPL-2.0-only)
+ */
+
 #include "openframe_encryption_service.h"
 #include <openssl/bio.h>
 #include <openssl/buffer.h>
 #include <openssl/evp.h>
 #include <sstream>
 #include <iomanip>
 
+#include <osquery/logger/logger.h>
+
+namespace osquery {
+
 OpenframeEncryptionService::OpenframeEncryptionService(const std::string& secret)
     : secret_(secret) {
-    if (secret_.empty()) {
-        throw std::runtime_error("Secret cannot be empty");
-    }
 }
 
-std::string OpenframeEncryptionService::decrypt(const std::string& data) {
+Status OpenframeEncryptionService::decrypt(const std::string& data, std::string& result) {
     if (secret_.empty()) {
-        throw std::runtime_error("Encryption service not initialized with secret");
+        return Status::failure("Encryption service not initialized with secret");
+    }
+
+    if (secret_.size() != 32) {
+        return Status::failure("Secret must be exactly 32 bytes for AES-256-GCM");
     }
 
     // Decode base64 data
-    auto decoded = base64Decode(data);
+    std::vector<unsigned char> decoded;
+    auto status = base64Decode(data, decoded);
+    if (!status.ok()) {
+        return status;
+    }
     if (decoded.size() < IV_SIZE + TAG_SIZE) {
-        throw std::runtime_error("Invalid encrypted data size");
+        return Status::failure("Invalid encrypted data size");
     }
 
     // Extract IV (first 12 bytes) and tag (last 16 bytes)
@@ -31,21 +49,21 @@ std::string OpenframeEncryptionService::decrypt(const std::string& data) {
     // Create and initialize the context
     EVP_CIPHER_CTX* ctx = EVP_CIPHER_CTX_new();
     if (!ctx) {
-        handleOpenSSLError();
+        return handleOpenSSLError();
     }
 
     // Initialize the decryption operation
     if (1 != EVP_DecryptInit_ex(ctx, EVP_aes_256_gcm(), nullptr, 
                                reinterpret_cast<const unsigned char*>(secret_.c_str()), 
                                iv.data())) {
         EVP_CIPHER_CTX_free(ctx);
-        handleOpenSSLError();
+        return handleOpenSSLError();
     }
 
     // Set the tag
     if (1 != EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_SET_TAG, TAG_SIZE, tag.data())) {
         EVP_CIPHER_CTX_free(ctx);
-        handleOpenSSLError();
+        return handleOpenSSLError();
     }
 
     // Decrypt the ciphertext
@@ -54,24 +72,25 @@ std::string OpenframeEncryptionService::decrypt(const std::string& data) {
     if (1 != EVP_DecryptUpdate(ctx, plaintext.data(), &len, 
                               ciphertext.data(), ciphertext.size())) {
         EVP_CIPHER_CTX_free(ctx);
-        handleOpenSSLError();
+        return handleOpenSSLError();
     }
 
     // Finalize the decryption
     int finalLen = 0;
     if (1 != EVP_DecryptFinal_ex(ctx, plaintext.data() + len, &finalLen)) {
         EVP_CIPHER_CTX_free(ctx);
-        handleOpenSSLError();
+        return handleOpenSSLError();
     }
 
     // Clean up
     EVP_CIPHER_CTX_free(ctx);
 
     // Convert the decrypted data to string
-    return std::string(plaintext.begin(), plaintext.begin() + len + finalLen);
+    result = std::string(plaintext.begin(), plaintext.begin() + len + finalLen);
+    return Status::success();
 }
 
-std::vector<unsigned char> OpenframeEncryptionService::base64Decode(const std::string& encoded) {
+Status OpenframeEncryptionService::base64Decode(const std::string& encoded, std::vector<unsigned char>& result) {
     BIO* b64 = BIO_new(BIO_f_base64());
     BIO_set_flags(b64, BIO_FLAGS_BASE64_NO_NL);
 
@@ -84,20 +103,25 @@ std::vector<unsigned char> OpenframeEncryptionService::base64Decode(const std::s
     BIO_free_all(bmem);
 
     if (decodedLen < 0) {
-        throw std::runtime_error("Failed to decode base64 data");
+        return Status::failure("Failed to decode base64 data");
     }
 
     decoded.resize(decodedLen);
-    return decoded;
+    result = std::move(decoded);
+    return Status::success();
 }
 
-void OpenframeEncryptionService::handleOpenSSLError() {
+Status OpenframeEncryptionService::handleOpenSSLError() {
     std::stringstream ss;
     unsigned long err;
     while ((err = ERR_get_error()) != 0) {
         char err_buf[256];
         ERR_error_string_n(err, err_buf, sizeof(err_buf));
         ss << err_buf << "; ";
     }
-    throw std::runtime_error("OpenSSL error: " + ss.str());
-} 
+    std::string message = "OpenSSL error: " + ss.str();
+    LOG(ERROR) << message;
+    return Status::failure(message);
+}
+
+} // namespace osquery

openframe/openframe_encryption_service.h

@@ -1,3 +1,11 @@
+/**
+ * Copyright (c) 2014-present, The osquery authors
+ *
+ * This source code is licensed as defined by the LICENSE file found in the
+ * root directory of this source tree.
+ *
+ * SPDX-License-Identifier: (Apache-2.0 OR GPL-2.0-only)
+ */
 #pragma once
 
 #include <string>
@@ -8,6 +16,10 @@
 #include <openssl/err.h>
 #include <stdexcept>
 
+#include <osquery/utils/status/status.h>
+
+namespace osquery {
+
 class OpenframeEncryptionService {
 public:
     explicit OpenframeEncryptionService(const std::string& secret);
@@ -16,10 +28,10 @@ class OpenframeEncryptionService {
     /**
      * Decrypts data using AES-GCM
      * @param data Base64 encoded encrypted data
-     * @return Decrypted data as string
-     * @throws std::runtime_error if decryption fails
+     * @param out Decrypted data as string, populated on success
+     * @return Status::success() on success, Status::failure() with an error message on failure
      */
-    std::string decrypt(const std::string& data);
+    Status decrypt(const std::string& data, std::string& out);
 
     std::vector<unsigned char> base64Decode(const std::string& encoded);
 
@@ -31,4 +43,6 @@ class OpenframeEncryptionService {
     void handleOpenSSLError();
 
     std::string secret_;
-}; 
+};
+
+} // namespace osquery

openframe/openframe_token_extractor.cpp

@@ -1,23 +1,38 @@
+/**
+ * Copyright (c) 2014-present, The osquery authors
+ *
+ * This source code is licensed as defined by the LICENSE file found in the
+ * root directory of this source tree.
+ *
+ * SPDX-License-Identifier: (Apache-2.0 OR GPL-2.0-only)
+ */
+
 #include "openframe_token_extractor.h"
 #include <fstream>
-#include <stdexcept>
+
+#include <osquery/logger/logger.h>
+
+namespace osquery {
 
 OpenframeTokenExtractor::OpenframeTokenExtractor(std::shared_ptr<OpenframeEncryptionService> encryption_service,
                                                const std::string& token_file_path)
     : encryption_service_(encryption_service), token_file_path_(token_file_path) {
     if (!encryption_service_) {
+        LOG(ERROR) << "Encryption service cannot be null";
         throw std::runtime_error("Encryption service cannot be null");
     }
     if (token_file_path_.empty()) {
+        LOG(ERROR) << "Token file path cannot be empty";
         throw std::runtime_error("Token file path cannot be empty");
     }
 }
 
-std::string OpenframeTokenExtractor::extractToken() {
+Status OpenframeTokenExtractor::extractToken(std::string& token) {
     // Open the token file
     std::ifstream token_file(token_file_path_);
     if (!token_file.is_open()) {
-        throw std::runtime_error("Failed to open token file at: " + token_file_path_);
+        LOG(WARNING) << "Failed to open token file at: " << token_file_path_;
+        return Status::failure("Failed to open token file at: " + token_file_path_);
     }
 
     // Read the encrypted token
@@ -26,13 +41,18 @@ std::string OpenframeTokenExtractor::extractToken() {
     token_file.close();
 
     if (encrypted_token.empty()) {
-        throw std::runtime_error("Token file is empty");
+        LOG(WARNING) << "Token file is empty: " << token_file_path_;
+        return Status::failure("Token file is empty");
     }
 
     try {
         // Decrypt the token using the encryption service
-        return encryption_service_->decrypt(encrypted_token);
+        token = encryption_service_->decrypt(encrypted_token);
+        return Status::success();
     } catch (const std::exception& e) {
-        throw std::runtime_error("Failed to decrypt token: " + std::string(e.what()));
+        LOG(WARNING) << "Failed to decrypt token: " << e.what();
+        return Status::failure("Failed to decrypt token: " + std::string(e.what()));
     }
-} 
+}
+
+} // namespace osquery

openframe/openframe_token_extractor.h

@@ -1,9 +1,20 @@
+/**
+ * Copyright (c) 2014-present, The osquery authors
+ *
+ * This source code is licensed as defined by the LICENSE file found in the
+ * root directory of this source tree.
+ *
+ * SPDX-License-Identifier: (Apache-2.0 OR GPL-2.0-only)
+ */
+
 #pragma once
 
 #include <string>
 #include <memory>
 #include "openframe_encryption_service.h"
 
+namespace osquery {
+
 class OpenframeTokenExtractor {
 public:
     explicit OpenframeTokenExtractor(std::shared_ptr<OpenframeEncryptionService> encryption_service,
@@ -16,4 +27,6 @@ class OpenframeTokenExtractor {
 private:
     std::string token_file_path_;
     std::shared_ptr<OpenframeEncryptionService> encryption_service_;
-}; 
+};
+
+} // namespace osquery

openframe/openframe_token_refresher.cpp

@@ -1,3 +1,12 @@
+/**
+ *  Copyright (c) 2014-present, The osquery authors
+ *
+ *  This source code is licensed as defined by the LICENSE file found in the
+ *  root directory of this source tree.
+ *
+ *  SPDX-License-Identifier: (Apache-2.0 OR GPL-2.0-only)
+ */
+
 #include "openframe_token_refresher.h"
 #include "openframe_authorization_manager_provider.h"
 
@@ -6,7 +15,7 @@ namespace osquery {
 OpenframeTokenRefresher::OpenframeTokenRefresher(std::shared_ptr<OpenframeTokenExtractor> extractor)
     : running_(false), extractor_(extractor) {
     if (!extractor_) {
-        throw std::runtime_error("Token extractor cannot be null");
+        LOG(ERROR) << "Token extractor cannot be null; token refresher will be inoperative";
     }
 }
 
@@ -21,6 +30,11 @@ void OpenframeTokenRefresher::start() {
         return;
     }
 
+    if (!extractor_) {
+        LOG(ERROR) << "Cannot start token refresher: token extractor is null";
+        return;
+    }
+
     running_ = true;
     refresh_thread_ = std::thread([this]() {
         while (running_) {
@@ -68,4 +82,4 @@ void OpenframeTokenRefresher::process() {
     }
 }
 
-} // namespace osquery 
+} // namespace osquery 

openframe/openframe_token_refresher.h

@@ -1,3 +1,12 @@
+/**
+ *  Copyright (c) 2014-present, The osquery authors
+ *
+ *  This source code is licensed as defined by the LICENSE file found in the
+ *  root directory of this source tree.
+ *
+ *  SPDX-License-Identifier: (Apache-2.0 OR GPL-2.0-only)
+ */
+
 #pragma once
 
 #include <string>
@@ -30,4 +39,4 @@ class OpenframeTokenRefresher {
     std::shared_ptr<OpenframeTokenExtractor> extractor_;
 };
 
-} // namespace osquery 
+} // namespace osquery