Skip to content

Commit a4382ff

Browse files
Remove HTTP from supported protocols in Downloader (eclipse-leda#42)
* Remove HTTP from supported protocols in Downloader * Fix review findings #1 * Fixed generation of self-signed certificate * Fixed OCaaS check (disable scan for curl tests) * Fix workflow (update of ca-certificates.conf) * Restrict self-signed certificate for testing to localhost * Fix review findings #2 * Fix OCaaS issues * Create unavailable directory for self-signed testing certificate * Fix OCaaS issues * Interpret backslash in github workflow * Fix path to crt file in apache2 configuration * Pass certificates from host to arm64 guest and update ca-certificates internally * Fix OCaaS issues * Update ca-certificates in arm64 guest on run * Fix OCaaS issues
1 parent f13ee27 commit a4382ff

17 files changed

+170
-71
lines changed

.github/actions/build-native-binary/action.yaml

+9-5
Original file line numberDiff line numberDiff line change
@@ -102,11 +102,13 @@ runs:
102102

103103
- name: Generate SSL key and certificate
104104
run: |
105+
sudo mkdir -p /usr/share/ca-certificates/extra
105106
sudo openssl req -x509 -nodes -days 365 -newkey rsa:2048 \
106107
-keyout /etc/ssl/private/selfupdateagent.key \
107-
-out /etc/ssl/certs/selfupdateagent.crt \
108-
-config utest/sua-certificate.config
109-
sudo tee -a /etc/ssl/certs/ca-certificates.crt < /etc/ssl/certs/selfupdateagent.crt > /dev/null
108+
-out /usr/share/ca-certificates/extra/selfupdateagent.crt \
109+
-subj '/CN=localhost' -extensions EXT -config utest/sua-certificate.config
110+
echo -e "\nextra/selfupdateagent.crt" | sudo tee -a /etc/ca-certificates.conf > /dev/null
111+
sudo update-ca-certificates
110112
shell: bash
111113

112114
- name: Install and configure apache2
@@ -145,13 +147,15 @@ runs:
145147
dockerRunArgs: |
146148
--volume "${PWD}:/sua"
147149
--volume "/data/selfupdates:/data/selfupdates"
148-
--volume "/etc/ssl/certs:/etc/ssl/certs"
150+
--volume "/usr/share/ca-certificates/extra:/usr/share/ca-certificates/extra"
149151
--net=host
150152
shell: /bin/sh
151153
install: |
152154
apt-get -y update
153-
apt-get -y install mosquitto-clients
155+
apt-get -y install mosquitto-clients ca-certificates
154156
run: |
157+
echo "\nextra/selfupdateagent.crt" | tee -a /etc/ca-certificates.conf > /dev/null
158+
update-ca-certificates
155159
cd /sua/dist_arm64/utest
156160
LD_LIBRARY_PATH=../lib ./TestSelfUpdateAgent > ../../unit_tests_report_arm64.txt
157161

.github/workflows/main.yaml

+1-1
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ jobs:
8080
runs-on: ubuntu-latest
8181
strategy:
8282
matrix:
83-
arch: [ arm64, amd64 ]
83+
arch: [ amd64, arm64 ]
8484
steps:
8585
- uses: actions/checkout@v3
8686
with:

.ort.yml

+6
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
excludes:
22
paths:
3+
- pattern: "project.spdx.yml"
4+
reason: "OTHER"
5+
comment: "Configuration for Open Source Scan"
36
- pattern: "3rdparty/openssl/tlsfuzzer/**"
47
reason: "TEST_OF"
58
comment: "Test suite for SSL & TLS, not included in production release."
@@ -36,6 +39,9 @@ excludes:
3639
- pattern: "3rdparty/curl/docs/**"
3740
reason: "DOCUMENTATION_OF"
3841
comment: "Code examples, not included in production release."
42+
- pattern: "3rdparty/curl/tests/**"
43+
reason: "TEST_OF"
44+
comment: "curl tests, not included in production release."
3945
- pattern: "3rdparty/glib/gio/tests/**"
4046
reason: "TEST_OF"
4147
comment: "Tests of glib, not included in production release."

3rdparty/curl

Submodule curl updated 2767 files

src/Download/Downloader.cpp

+63-29
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424
#include <cstdlib>
2525
#include <cstring>
2626

27+
#include <tuple>
28+
2729
#include <dirent.h>
2830
#include <curl/curl.h>
2931
#include <sys/stat.h>
@@ -82,18 +84,49 @@ namespace {
8284
return written;
8385
}
8486

85-
sua::TechCode download(const std::string & caPath, const std::string & caFile, const char* url)
87+
class CurlGlobalGuard {
88+
public:
89+
CURLcode init() {
90+
return curl_global_init(CURL_GLOBAL_ALL);
91+
}
92+
93+
~CurlGlobalGuard() {
94+
curl_global_cleanup();
95+
}
96+
};
97+
98+
class CurlEasyHandleGuard {
99+
public:
100+
CURL * init() {
101+
_handle = curl_easy_init();
102+
return _handle;
103+
}
104+
105+
~CurlEasyHandleGuard() {
106+
if(_handle) {
107+
curl_easy_cleanup(_handle);
108+
}
109+
}
110+
111+
private:
112+
CURL * _handle = nullptr;
113+
};
114+
115+
sua::DownloadResult download(const std::string & caPath, const std::string & caFile, const char* url)
86116
{
87-
CURLcode gres = curl_global_init(CURL_GLOBAL_ALL);
88-
if(gres != 0) {
89-
sua::Logger::critical("curl_global_init failed with code = {}", gres);
90-
return sua::TechCode::DownloadFailed;
117+
CurlGlobalGuard global_guard;
118+
auto init_status = global_guard.init();
119+
if(init_status != 0) {
120+
sua::Logger::critical("curl_global_init failed with code = {}", init_status);
121+
return std::make_tuple(sua::TechCode::DownloadFailed, "libcurl init failed");
91122
}
92123

93-
CURL* easy_handle = curl_easy_init();
94-
if(!easy_handle) {
124+
CurlEasyHandleGuard easy_guard;
125+
auto h = easy_guard.init();
126+
127+
if(!h) {
95128
sua::Logger::critical("curl_easy_init failed");
96-
return sua::TechCode::DownloadFailed;
129+
return std::make_tuple(sua::TechCode::DownloadFailed, "libcurl init failed");
97130
}
98131

99132
DIR* dir = opendir(FILE_DIR);
@@ -103,49 +136,50 @@ namespace {
103136
const int dir_err = mkdir(FILE_DIR, S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH);
104137
if(dir_err) {
105138
sua::Logger::critical("Issue with creating a dir {}, error: {}", FILE_DIR, dir_err);
106-
return sua::TechCode::DownloadFailed;
139+
return std::make_tuple(sua::TechCode::DownloadFailed, "temporary file folder could not be created");
107140
}
108141
}
109142

110143
long response_code;
111144
FILE* fp = fopen(FILE_PATH, "wb");
112145
if(!fp) {
113146
sua::Logger::critical("Failed to open '{}' for writing", FILE_PATH);
114-
return sua::TechCode::DownloadFailed;
147+
return std::make_tuple(sua::TechCode::DownloadFailed, "temporary file could not be opened for writing");
115148
}
116149

117-
curl_easy_setopt(easy_handle, CURLOPT_URL, url);
118-
curl_easy_getinfo(easy_handle, CURLINFO_RESPONSE_CODE, &response_code);
150+
curl_easy_setopt(h, CURLOPT_URL, url);
151+
curl_easy_getinfo(h, CURLINFO_RESPONSE_CODE, &response_code);
119152
if(!caFile.empty()) {
120-
curl_easy_setopt(easy_handle, CURLOPT_CAINFO, caFile.c_str());
153+
curl_easy_setopt(h, CURLOPT_CAINFO, caFile.c_str());
121154
} else {
122-
curl_easy_setopt(easy_handle, CURLOPT_CAPATH, caPath.c_str());
155+
curl_easy_setopt(h, CURLOPT_CAPATH, caPath.c_str());
123156
}
124-
curl_easy_setopt(easy_handle, CURLOPT_SSL_VERIFYPEER, 1);
125-
curl_easy_setopt(easy_handle, CURLOPT_WRITEFUNCTION, write_data);
126-
curl_easy_setopt(easy_handle, CURLOPT_WRITEDATA, fp);
127-
curl_easy_setopt(easy_handle, CURLOPT_FOLLOWLOCATION, 1L);
128-
curl_easy_setopt(easy_handle, CURLOPT_PROGRESSDATA, &data);
129-
curl_easy_setopt(easy_handle, CURLOPT_PROGRESSFUNCTION, progress_callback);
130-
curl_easy_setopt(easy_handle, CURLOPT_NOPROGRESS, 0);
131-
CURLcode res = curl_easy_perform(easy_handle);
157+
curl_easy_setopt(h, CURLOPT_SSL_VERIFYPEER, 1L);
158+
curl_easy_setopt(h, CURLOPT_SSL_VERIFYHOST, 2L);
159+
curl_easy_setopt(h, CURLOPT_WRITEFUNCTION, write_data);
160+
curl_easy_setopt(h, CURLOPT_WRITEDATA, fp);
161+
curl_easy_setopt(h, CURLOPT_FOLLOWLOCATION, 1L);
162+
curl_easy_setopt(h, CURLOPT_PROGRESSDATA, &data);
163+
curl_easy_setopt(h, CURLOPT_PROGRESSFUNCTION, progress_callback);
164+
curl_easy_setopt(h, CURLOPT_NOPROGRESS, 0);
165+
curl_easy_setopt(h, CURLOPT_PROTOCOLS_STR, "https");
166+
CURLcode res = curl_easy_perform(h);
132167

133168
sua::Logger::debug("curl_easy_perform ended with code = '{}'", res);
134169

135170
long http_code = 0;
136-
curl_easy_getinfo(easy_handle, CURLINFO_RESPONSE_CODE, &http_code);
137-
curl_easy_cleanup(easy_handle);
138-
curl_global_cleanup();
171+
curl_easy_getinfo(h, CURLINFO_RESPONSE_CODE, &http_code);
139172
fclose(fp);
140173
progressNotificationLimiter = 0;
141174

142175
sua::Logger::debug("CURLINFO_RESPONSE_CODE = {}", http_code);
143176
if(http_code != 200) {
144-
sua::Logger::error(curl_easy_strerror(res));
145-
return sua::TechCode::DownloadFailed;
177+
auto e = curl_easy_strerror(res);
178+
sua::Logger::error(e);
179+
return std::make_tuple(sua::TechCode::DownloadFailed, e);
146180
}
147181

148-
return sua::TechCode::OK;
182+
return std::make_tuple(sua::TechCode::OK, "");
149183
}
150184

151185
} // namespace
@@ -162,7 +196,7 @@ namespace sua {
162196
strncpy(FILE_PATH, filepath.c_str(), FILENAME_MAX - 1);
163197
}
164198

165-
TechCode Downloader::start(const std::string & input)
199+
DownloadResult Downloader::start(const std::string & input)
166200
{
167201
return download(_context.caDirectory, _context.caFilepath, input.c_str());
168202
}

src/Download/Downloader.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ namespace sua {
2828

2929
static const std::string EVENT_DOWNLOADING;
3030

31-
TechCode start(const std::string & input) override;
31+
DownloadResult start(const std::string & input) override;
3232

3333
private:
3434
class Context & _context;

src/Download/IDownloader.h

+4-1
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,17 @@
2020
#include "TechCodes.h"
2121

2222
#include <string>
23+
#include <tuple>
2324

2425
namespace sua {
2526

27+
using DownloadResult = std::tuple<TechCode, std::string>;
28+
2629
class IDownloader {
2730
public:
2831
virtual ~IDownloader() = default;
2932

30-
virtual TechCode start(const std::string & input) = 0;
33+
virtual DownloadResult start(const std::string & input) = 0;
3134
};
3235

3336
} // namespace sua

src/FSM/FSM.cpp

+2
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ namespace sua {
4141

4242
std::string FSM::activeState() const
4343
{
44+
assert(_currentState != nullptr);
45+
4446
return _currentState->name();
4547
}
4648

src/FSM/States/Downloading.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ namespace sua {
5757
Logger::info("Downloading bundle: '{}'", ctx.desiredState.bundleDownloadUrl);
5858
const auto result = ctx.downloaderAgent->start(ctx.desiredState.bundleDownloadUrl);
5959

60-
if(result == TechCode::OK) {
60+
if(std::get<0>(result) == TechCode::OK) {
6161
Logger::info("Download progress: 100%");
6262
send(ctx, IMqttProcessor::TOPIC_FEEDBACK, MqttMessage::Downloaded);
6363

@@ -67,9 +67,9 @@ namespace sua {
6767
return FotaEvent::DownloadSucceeded;
6868
} else {
6969
Logger::error("Download failed.");
70-
send(ctx, IMqttProcessor::TOPIC_FEEDBACK, MqttMessage::DownloadFailed);
7170
ctx.desiredState.actionStatus = "DOWNLOAD_FAILURE";
72-
ctx.desiredState.actionMessage = "Download failed.";
71+
ctx.desiredState.actionMessage = "Download failed: " + std::get<1>(result);
72+
send(ctx, IMqttProcessor::TOPIC_FEEDBACK, MqttMessage::DownloadFailed);
7373
return FotaEvent::DownloadFailed;
7474
}
7575
} else {

src/Mqtt/MqttMessagingProtocolJSON.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ namespace sua {
179179
case MqttMessage::DownloadFailed:
180180
return writeFeedbackWithPayload(ctx.desiredState,
181181
"DOWNLOAD_FAILURE", "Download failed.",
182-
"DOWNLOAD_FAILURE", "Download failed.",
182+
ctx.desiredState.actionStatus, ctx.desiredState.actionMessage,
183183
message, ctx.desiredState.downloadProgressPercentage);
184184
case MqttMessage::VersionChecking:
185185
return writeFeedbackWithPayload(ctx.desiredState,

utest/CMakeLists.txt

+1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
add_executable(TestSelfUpdateAgent
22
TestDispatcher.cpp
3+
TestDownloader.cpp
34
TestFSM.cpp
45
TestLogger.cpp
56
TestMqttMessagingProtocolJSON.cpp

utest/MockDownloader.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323

2424
class MockDownloader : public sua::IDownloader {
2525
public:
26-
MOCK_METHOD(sua::TechCode, start, (const std::string & input), (override));
26+
MOCK_METHOD(sua::DownloadResult, start, (const std::string & input), (override));
2727
};
2828

2929
#endif

utest/TestDownloader.cpp

+35
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
#include "gtest/gtest.h"
2+
3+
#include "Context.h"
4+
#include "Download/Downloader.h"
5+
6+
#include <spdlog/fmt/fmt.h>
7+
8+
namespace {
9+
10+
class TestDownloaderP
11+
: public ::testing::TestWithParam<std::string>
12+
{ };
13+
14+
TEST_P(TestDownloaderP, downloadViaUnsupportedProtocolFails)
15+
{
16+
sua::Context ctx;
17+
sua::Downloader d(ctx);
18+
19+
auto url = fmt::format("{}://127.0.0.1/bundle", GetParam());
20+
auto res = d.start(url);
21+
22+
EXPECT_EQ(std::get<0>(res), sua::TechCode::DownloadFailed);
23+
}
24+
25+
INSTANTIATE_TEST_CASE_P(
26+
TestDownloaderViaUnsupportedProtocols,
27+
TestDownloaderP,
28+
::testing::Values(
29+
"ftp", "http", "sftp", "smb", "smbs", "ldap", "ldaps"
30+
)
31+
);
32+
33+
}
34+
35+

utest/TestMqttMessagingProtocolJSON.cpp

+4-1
Original file line numberDiff line numberDiff line change
@@ -596,6 +596,9 @@ namespace {
596596
{
597597
d.downloadProgressPercentage = 66;
598598

599+
ctx.desiredState.actionStatus = "DOWNLOAD_FAILURE";
600+
ctx.desiredState.actionMessage = "Download failed: test";
601+
599602
const std::string result = ProtocolJSON().createMessage(ctx, sua::MqttMessage::DownloadFailed);
600603

601604
// clang-format off
@@ -614,7 +617,7 @@ namespace {
614617
},
615618
"status": "DOWNLOAD_FAILURE",
616619
"progress": 66,
617-
"message": "Download failed."
620+
"message": "Download failed: test"
618621
}
619622
]
620623
}

0 commit comments

Comments
 (0)