Skip to content

Commit 4c4afff

Browse files
authored
Auth segfault fix (#1046)
* Auth segfault fix * Added unit tests
1 parent 1d01bbd commit 4c4afff

File tree

8 files changed

+143
-36
lines changed

8 files changed

+143
-36
lines changed

.github/workflows/ci.yml

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ on:
1111

1212
jobs:
1313
mac-os-build-clang:
14-
runs-on: macos-13
14+
runs-on: macos-11
1515
env:
1616
AWS_KVS_LOG_LEVEL: 2
1717
permissions:
@@ -23,6 +23,7 @@ jobs:
2323
- name: Install dependencies
2424
run: |
2525
brew install pkg-config openssl cmake gstreamer log4cplus
26+
brew unlink openssl
2627
- name: Build repository
2728
run: |
2829
mkdir build && cd build
@@ -42,7 +43,7 @@ jobs:
4243
./tst/producerTest
4344
4445
mac-os-build-gcc:
45-
runs-on: macos-13
46+
runs-on: macos-12
4647
permissions:
4748
id-token: write
4849
contents: read
@@ -56,6 +57,7 @@ jobs:
5657
- name: Install dependencies
5758
run: |
5859
brew install pkg-config openssl cmake gstreamer log4cplus
60+
brew unlink openssl
5961
- name: Build repository
6062
run: |
6163
mkdir build && cd build

CMake/Dependencies/libkvscproducer-CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ include(ExternalProject)
77
# clone repo only
88
ExternalProject_Add(libkvscproducer-download
99
GIT_REPOSITORY https://github.com/awslabs/amazon-kinesis-video-streams-producer-c.git
10-
GIT_TAG a3dd1d08600854e249dab5760752451e5daca9e4
10+
GIT_TAG release-1.5.0
1111
SOURCE_DIR "${CMAKE_CURRENT_BINARY_DIR}/kvscproducer-src"
1212
BINARY_DIR "${CMAKE_CURRENT_BINARY_DIR}/kvscproducer-build"
1313
CONFIGURE_COMMAND ""

CMakeLists.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/CMake;${CMAKE_MODULE_PATH}")
33
include(Utilities)
44
project(KinesisVideoProducerCpp)
55

6-
project(KinesisVideoProducerCpp VERSION 3.3.1)
6+
project(KinesisVideoProducerCpp VERSION 3.4.0)
77

88
set(CMAKE_CXX_STANDARD 11)
99
include(GNUInstallDirs)
@@ -27,7 +27,7 @@ option(MEMORY_SANITIZER "Build with MemorySanitizer" OFF)
2727
option(THREAD_SANITIZER "Build with ThreadSanitizer" OFF)
2828
option(UNDEFINED_BEHAVIOR_SANITIZER "Build with UndefinedBehaviorSanitizer" OFF)
2929

30-
add_definitions(-DVERSION_STRING=\"${PROJECT_VERSION}\")
30+
add_definitions(-DCPP_VERSION_STRING=\"${PROJECT_VERSION}\")
3131

3232
set(CMAKE_MACOSX_RPATH TRUE)
3333
get_filename_component(ROOT "${CMAKE_CURRENT_SOURCE_DIR}" ABSOLUTE)

src/Auth.cpp

Lines changed: 39 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ DeviceCertToTokenFunc CredentialProvider::deviceCertToTokenCallback() {
9090

9191
STATUS CredentialProvider::getStreamingTokenHandler(UINT64 custom_data, PCHAR stream_name, STREAM_ACCESS_MODE access_mode, PServiceCallContext p_service_call_context) {
9292
LOG_DEBUG("getStreamingTokenHandler invoked");
93+
STATUS status = STATUS_SUCCESS;
9394
UNUSED_PARAM(stream_name);
9495
UNUSED_PARAM(access_mode);
9596

@@ -111,14 +112,25 @@ STATUS CredentialProvider::getStreamingTokenHandler(UINT64 custom_data, PCHAR st
111112
freeAwsCredentials(&this_obj->security_token_);
112113

113114
// Store the buffer so we can release it at the end
114-
createAwsCredentials((PCHAR) access_key.c_str(), access_key_len, (PCHAR) secret_key.c_str(), secret_key_len,
115-
(PCHAR) session_token.c_str(), session_token_len, expiration, &this_obj->security_token_);
115+
if(IS_EMPTY_STRING(session_token.c_str())) {
116+
status = createAwsCredentials((PCHAR) access_key.c_str(), access_key_len, (PCHAR) secret_key.c_str(), secret_key_len,
117+
nullptr, 0, expiration, &this_obj->security_token_);
116118

117-
STATUS status = getStreamingTokenResultEvent(
118-
p_service_call_context->customData, SERVICE_CALL_RESULT_OK,
119-
reinterpret_cast<PBYTE>(this_obj->security_token_),
120-
this_obj->security_token_->size,
121-
expiration);
119+
} else {
120+
status = createAwsCredentials((PCHAR) access_key.c_str(), access_key_len, (PCHAR) secret_key.c_str(), secret_key_len,
121+
(PCHAR) session_token.c_str(), session_token_len, expiration, &this_obj->security_token_);
122+
123+
}
124+
125+
if(STATUS_SUCCEEDED(status)) {
126+
status = getStreamingTokenResultEvent(
127+
p_service_call_context->customData, SERVICE_CALL_RESULT_OK,
128+
reinterpret_cast<PBYTE>(this_obj->security_token_),
129+
this_obj->security_token_->size,
130+
expiration);
131+
} else {
132+
LOG_ERROR("getStreamingTokenHandler failed with code " << std::hex << status);
133+
}
122134

123135
return status;
124136
}
@@ -127,10 +139,11 @@ STATUS CredentialProvider::getSecurityTokenHandler(UINT64 custom_data, PBYTE* pp
127139
LOG_DEBUG("getSecurityTokenHandler invoked");
128140

129141
auto this_obj = reinterpret_cast<CredentialProvider*>(custom_data);
130-
142+
STATUS status = STATUS_SUCCESS;
131143
Credentials credentials;
132144
this_obj->getCredentials(credentials);
133145

146+
134147
auto access_key = credentials.getAccessKey();
135148
auto access_key_len = access_key.length();
136149
auto secret_key = credentials.getSecretKey();
@@ -145,13 +158,26 @@ STATUS CredentialProvider::getSecurityTokenHandler(UINT64 custom_data, PBYTE* pp
145158
*p_expiration = credentials.getExpiration().count() * HUNDREDS_OF_NANOS_IN_A_SECOND;
146159

147160
// Store the buffer so we can release it at the end
148-
createAwsCredentials((PCHAR) access_key.c_str(), access_key_len, (PCHAR) secret_key.c_str(), secret_key_len,
149-
(PCHAR) session_token.c_str(), session_token_len, *p_expiration, &this_obj->security_token_);
161+
if(IS_EMPTY_STRING(session_token.c_str())) {
162+
// Store the buffer so we can release it at the end
163+
status = createAwsCredentials((PCHAR) access_key.c_str(), access_key_len, (PCHAR) secret_key.c_str(), secret_key_len,
164+
nullptr, 0, *p_expiration, &this_obj->security_token_);
150165

151-
*pp_token = (PBYTE) (this_obj->security_token_);
152-
*p_size = this_obj->security_token_->size;
166+
} else {
167+
// Store the buffer so we can release it at the end
168+
status = createAwsCredentials((PCHAR) access_key.c_str(), access_key_len, (PCHAR) secret_key.c_str(), secret_key_len,
169+
(PCHAR) session_token.c_str(), session_token_len, *p_expiration, &this_obj->security_token_);
153170

154-
return STATUS_SUCCESS;
171+
}
172+
173+
if(STATUS_SUCCEEDED(status)) {
174+
*pp_token = (PBYTE) (this_obj->security_token_);
175+
*p_size = this_obj->security_token_->size;
176+
} else {
177+
LOG_ERROR("getSecurityTokenHandler failed with code " << std::hex << status);
178+
}
179+
180+
return status;
155181
}
156182

157183
} // namespace video

src/gstreamer/gstkvssink.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,8 @@ G_BEGIN_DECLS
5757
(G_TYPE_CHECK_CLASS_TYPE((klass),GST_TYPE_KVS_SINK))
5858
#define GST_KVS_SINK_CAST(obj) ((GstKvsSink *)obj)
5959

60-
#ifdef VERSION_STRING
61-
#define KVSSINK_USER_AGENT_POSTFIX_VERSION VERSION_STRING
60+
#ifdef CPP_VERSION_STRING
61+
#define KVSSINK_USER_AGENT_POSTFIX_VERSION CPP_VERSION_STRING
6262
#else
6363
#define KVSSINK_USER_AGENT_POSTFIX_VERSION "UNKNOWN"
6464
#endif

tst/ProducerApiTest.cpp

Lines changed: 52 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -140,10 +140,45 @@ PVOID ProducerTestBase::basicProducerRoutine(KinesisVideoStream* kinesis_video_s
140140
return NULL;
141141
}
142142

143+
TEST_F(ProducerApiTest, invalid_credentials)
144+
{
145+
std::unique_ptr<Credentials> credentials;
146+
std::unique_ptr<CredentialProvider> credential_provider;
147+
credentials.reset(new Credentials("",
148+
"SecretKey",
149+
"SessionToken",
150+
std::chrono::seconds(TEST_STREAMING_TOKEN_DURATION_IN_SECONDS)));
151+
152+
credential_provider.reset(new TestCredentialProvider(*credentials.get(), TEST_STREAMING_TOKEN_DURATION_IN_SECONDS));
153+
EXPECT_THROW(CreateProducer(std::move(credential_provider)), std::runtime_error);
154+
155+
credentials.reset(new Credentials("AccessKey",
156+
"",
157+
"SessionToken",
158+
std::chrono::seconds(TEST_STREAMING_TOKEN_DURATION_IN_SECONDS)));
159+
credential_provider.reset(new TestCredentialProvider(*credentials.get(), TEST_STREAMING_TOKEN_DURATION_IN_SECONDS));
160+
EXPECT_THROW(CreateProducer(std::move(credential_provider)), std::runtime_error);
161+
162+
credentials.reset(new Credentials("",
163+
"",
164+
"SessionToken",
165+
std::chrono::seconds(TEST_STREAMING_TOKEN_DURATION_IN_SECONDS)));
166+
credential_provider.reset(new TestCredentialProvider(*credentials.get(), TEST_STREAMING_TOKEN_DURATION_IN_SECONDS));
167+
EXPECT_THROW(CreateProducer(std::move(credential_provider)), std::runtime_error);
168+
169+
credentials.reset(new Credentials("AccessKey",
170+
"SecretKey",
171+
"",
172+
std::chrono::seconds(TEST_STREAMING_TOKEN_DURATION_IN_SECONDS)));
173+
credential_provider.reset(new TestCredentialProvider(*credentials.get(), TEST_STREAMING_TOKEN_DURATION_IN_SECONDS));
174+
CreateProducer(std::move(credential_provider)); // expect no exception thrown since empty session token is allowed
175+
}
176+
143177
TEST_F(ProducerApiTest, create_free_stream)
144178
{
145179
// Check if it's run with the env vars set if not bail out
146180
if (!access_key_set_) {
181+
LOG_WARN("Creds not set");
147182
return;
148183
}
149184

@@ -181,6 +216,7 @@ TEST_F(ProducerApiTest, DISABLED_create_produce_offline_stream)
181216
{
182217
// Check if it's run with the env vars set if not bail out
183218
if (!access_key_set_) {
219+
LOG_WARN("Creds not set");
184220
return;
185221
}
186222

@@ -224,6 +260,7 @@ TEST_F(ProducerApiTest, create_produce_start_stop_stream)
224260
{
225261
// Check if it's run with the env vars set if not bail out
226262
if (!access_key_set_) {
263+
LOG_WARN("Creds not set");
227264
return;
228265
}
229266

@@ -280,7 +317,7 @@ TEST_F(ProducerApiTest, create_produce_start_stop_stream)
280317
EXPECT_TRUE(kinesis_video_stream->stopSync()) << "Timed out awaiting for the stream stop notification";
281318
EXPECT_TRUE(gProducerApiTest->stop_called_) << "Status of stopped state " << gProducerApiTest->stop_called_;
282319

283-
kinesis_video_producer_->freeStream(move(streams_[0]));
320+
kinesis_video_producer_->freeStream(std::move(streams_[0]));
284321
streams_[0] = nullptr;
285322
}
286323
}
@@ -289,6 +326,7 @@ TEST_F(ProducerApiTest, create_produce_start_stop_stream_endpoint_cached)
289326
{
290327
// Check if it's run with the env vars set if not bail out
291328
if (!access_key_set_) {
329+
LOG_WARN("Creds not set");
292330
return;
293331
}
294332

@@ -345,7 +383,7 @@ TEST_F(ProducerApiTest, create_produce_start_stop_stream_endpoint_cached)
345383
EXPECT_TRUE(kinesis_video_stream->stopSync()) << "Timed out awaiting for the stream stop notification";
346384
EXPECT_TRUE(gProducerApiTest->stop_called_) << "Status of stopped state " << gProducerApiTest->stop_called_;
347385

348-
kinesis_video_producer_->freeStream(move(streams_[0]));
386+
kinesis_video_producer_->freeStream(std::move(streams_[0]));
349387
streams_[0] = nullptr;
350388
}
351389
}
@@ -354,6 +392,7 @@ TEST_F(ProducerApiTest, create_produce_start_stop_stream_all_cached)
354392
{
355393
// Check if it's run with the env vars set if not bail out
356394
if (!access_key_set_) {
395+
LOG_WARN("Creds not set");
357396
return;
358397
}
359398

@@ -410,7 +449,7 @@ TEST_F(ProducerApiTest, create_produce_start_stop_stream_all_cached)
410449
EXPECT_TRUE(kinesis_video_stream->stopSync()) << "Timed out awaiting for the stream stop notification";
411450
EXPECT_TRUE(gProducerApiTest->stop_called_) << "Status of stopped state " << gProducerApiTest->stop_called_;
412451

413-
kinesis_video_producer_->freeStream(move(streams_[0]));
452+
kinesis_video_producer_->freeStream(std::move(streams_[0]));
414453
streams_[0] = nullptr;
415454
}
416455
}
@@ -419,6 +458,7 @@ TEST_F(ProducerApiTest, create_produce_start_stop_reset_stream_endpoint_cached)
419458
{
420459
// Check if it's run with the env vars set if not bail out
421460
if (!access_key_set_) {
461+
LOG_WARN("Creds not set");
422462
return;
423463
}
424464

@@ -478,14 +518,15 @@ TEST_F(ProducerApiTest, create_produce_start_stop_reset_stream_endpoint_cached)
478518
kinesis_video_stream->resetStream();
479519
}
480520

481-
kinesis_video_producer_->freeStream(move(streams_[0]));
521+
kinesis_video_producer_->freeStream(std::move(streams_[0]));
482522
streams_[0] = nullptr;
483523
}
484524

485525
TEST_F(ProducerApiTest, create_produce_start_stop_reset_stream_all_cached)
486526
{
487527
// Check if it's run with the env vars set if not bail out
488528
if (!access_key_set_) {
529+
LOG_WARN("Creds not set");
489530
return;
490531
}
491532

@@ -545,14 +586,15 @@ TEST_F(ProducerApiTest, create_produce_start_stop_reset_stream_all_cached)
545586
kinesis_video_stream->resetStream();
546587
}
547588

548-
kinesis_video_producer_->freeStream(move(streams_[0]));
589+
kinesis_video_producer_->freeStream(std::move(streams_[0]));
549590
streams_[0] = nullptr;
550591
}
551592

552593
TEST_F(ProducerApiTest, create_produce_stream)
553594
{
554595
// Check if it's run with the env vars set if not bail out
555596
if (!access_key_set_) {
597+
LOG_WARN("Creds not set");
556598
return;
557599
}
558600

@@ -631,6 +673,7 @@ TEST_F(ProducerApiTest, create_caching_endpoing_produce_stream)
631673
{
632674
// Check if it's run with the env vars set if not bail out
633675
if (!access_key_set_) {
676+
LOG_WARN("Creds not set");
634677
return;
635678
}
636679

@@ -692,7 +735,7 @@ TEST_F(ProducerApiTest, exceed_max_track_count)
692735
stream_definition->addTrack(1, testTrackName, testCodecId, MKV_TRACK_INFO_TYPE_VIDEO);
693736
stream_definition->addTrack(2, testTrackName, testCodecId, MKV_TRACK_INFO_TYPE_AUDIO);
694737
stream_definition->addTrack(3, testTrackName, testCodecId, MKV_TRACK_INFO_TYPE_VIDEO);
695-
EXPECT_ANY_THROW(kinesis_video_producer_->createStream(move(stream_definition)));
738+
EXPECT_ANY_THROW(kinesis_video_producer_->createStream(std::move(stream_definition)));
696739
}
697740

698741
TEST_F(ProducerApiTest, segment_uuid_variations)
@@ -732,7 +775,7 @@ TEST_F(ProducerApiTest, segment_uuid_variations)
732775
vector<uint8_t>(),
733776
DEFAULT_TRACK_ID));
734777

735-
EXPECT_NE(nullptr, kinesis_video_producer_->createStreamSync(move(stream_definition)));
778+
EXPECT_NE(nullptr, kinesis_video_producer_->createStreamSync(std::move(stream_definition)));
736779
kinesis_video_producer_->freeStreams();
737780

738781
// Valid
@@ -768,7 +811,7 @@ TEST_F(ProducerApiTest, segment_uuid_variations)
768811
vector<uint8_t>(),
769812
DEFAULT_TRACK_ID));
770813

771-
EXPECT_NE(nullptr, kinesis_video_producer_->createStreamSync(move(stream_definition)));
814+
EXPECT_NE(nullptr, kinesis_video_producer_->createStreamSync(std::move(stream_definition)));
772815
kinesis_video_producer_->freeStreams();
773816

774817
// invalid - larger
@@ -804,7 +847,7 @@ TEST_F(ProducerApiTest, segment_uuid_variations)
804847
vector<uint8_t>(),
805848
DEFAULT_TRACK_ID));
806849

807-
EXPECT_NE(nullptr, kinesis_video_producer_->createStreamSync(move(stream_definition)));
850+
EXPECT_NE(nullptr, kinesis_video_producer_->createStreamSync(std::move(stream_definition)));
808851
kinesis_video_producer_->freeStreams();
809852

810853
// shorter length

0 commit comments

Comments
 (0)