Skip to content

Change mbedtls_platform_context parameter to NULL #199

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 31, 2018
Merged

Change mbedtls_platform_context parameter to NULL #199

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions authcrypt/authcrypt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,8 @@ const char Authcrypt::message[] = "Some things are better left unread";

const char Authcrypt::metadata[] = "eg sequence number, routing info";

Authcrypt::Authcrypt(mbedtls_platform_context* platform_ctx)
Authcrypt::Authcrypt()
{
// The platform context can be used by cryptographic calls which require it.
// Please refer to https://github.com/ARMmbed/mbedtls/issues/1200 for more information.
_platform_ctx = platform_ctx;
memset(ciphertext, 0, sizeof(ciphertext));
memset(decrypted, 0, sizeof(decrypted));

Expand Down
7 changes: 1 addition & 6 deletions authcrypt/authcrypt.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class Authcrypt
/**
* Construct an Authcrypt instance
*/
Authcrypt(mbedtls_platform_context* platform_ctx);
Authcrypt();

/**
* Free any allocated resources
Expand Down Expand Up @@ -104,11 +104,6 @@ class Authcrypt
* The block cipher configuration
*/
mbedtls_cipher_context_t cipher;

/**
* The platform context
*/
mbedtls_platform_context* _platform_ctx;
};

#endif /* _AUTHCRYPT_H_ */
7 changes: 3 additions & 4 deletions authcrypt/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,14 @@
#include "mbedtls/platform.h"

int main() {
mbedtls_platform_context platform_ctx;
int exit_code = MBEDTLS_EXIT_FAILURE;

if((exit_code = mbedtls_platform_setup(&platform_ctx)) != 0) {
if((exit_code = mbedtls_platform_setup(NULL)) != 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is assigning an error code from mbedtls_platform_setup() to an exit_code used to exit main(). They are two different sets of values - and the error values don't map to valid exit codes. Initialise exit_code to MBEDTLS_EXIT_SUCCESS and remove these follow on assignments. They're not needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sbutcher-arm I disagree.
Although the exit code is not the return code of the functions, we use it for logging the error code. When we exit the application upon failure, the return code will always be MBEDTLS_EXIT_FAILURE

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I disagree. ;) I think it's reusing the variable for two different purposes with sets of values that don't necessarily mix. But it's a pre-existing problem, so I think I can ignore it for the moment, as the code works, just doesn't match my expectations.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I created #205 to track this

printf("Platform initialization failed with error %d\n", exit_code);
return MBEDTLS_EXIT_FAILURE;
}

Authcrypt *authcrypt = new Authcrypt(&platform_ctx);
Authcrypt *authcrypt = new Authcrypt();

if ((exit_code = authcrypt->run()) != 0) {
mbedtls_printf("Example failed with error %d\n", exit_code);
Expand All @@ -41,6 +40,6 @@ int main() {

delete authcrypt;

mbedtls_platform_teardown(&platform_ctx);
mbedtls_platform_teardown(NULL);
return exit_code;
}
7 changes: 2 additions & 5 deletions benchmark/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,6 @@ MBED_NOINLINE static int benchmark_sha512()
}
#endif /* MBEDTLS_SHA512_C */


#if defined(MBEDTLS_ARC4_C)
MBED_NOINLINE static int benchmark_arc4()
{
Expand Down Expand Up @@ -1316,13 +1315,12 @@ MBED_NOINLINE static int benchmark_ecdh_curve22519()

int main()
{
mbedtls_platform_context platform_ctx;
int exit_code = MBEDTLS_EXIT_SUCCESS;

memset(buf, 0xAA, sizeof(buf));
memset(tmp, 0xBB, sizeof(tmp));

if ((exit_code = mbedtls_platform_setup(&platform_ctx)) != 0) {
if ((exit_code = mbedtls_platform_setup(NULL)) != 0) {
mbedtls_printf("Platform initialization failed with error %d\r\n",
exit_code);
return MBEDTLS_EXIT_FAILURE;
Expand Down Expand Up @@ -1482,7 +1480,6 @@ int main()

mbedtls_printf("DONE\n");

mbedtls_platform_teardown(&platform_ctx);

mbedtls_platform_teardown(NULL);
return exit_code;
}
14 changes: 4 additions & 10 deletions hashing/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,8 @@ static const char hello_str[] = "Hello, world!";
static const unsigned char *hello_buffer = (const unsigned char *) hello_str;
static const size_t hello_len = strlen(hello_str);

static int example(mbedtls_platform_context* ctx)
static int example()
{
// The call below is used to avoid the "unused parameter" warning.
// The context itself can be used by cryptographic calls which require it.
// Please refer to https://github.com/ARMmbed/mbedtls/issues/1200 for more information.
(void)ctx;

mbedtls_printf("\n\n");

/*
Expand Down Expand Up @@ -157,20 +152,19 @@ static int example(mbedtls_platform_context* ctx)
}

int main() {
mbedtls_platform_context platform_ctx;
int exit_code = MBEDTLS_EXIT_FAILURE;

if((exit_code = mbedtls_platform_setup(&platform_ctx)) != 0) {
if((exit_code = mbedtls_platform_setup(NULL)) != 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same review comment as before - remove the assignment toexit_code, and initialise it to MBEDTLS_EXIT_SUCCESS instead.

printf("Platform initialization failed with error %d\n", exit_code);
return MBEDTLS_EXIT_FAILURE;
}

exit_code = example(&platform_ctx);
exit_code = example();
if (exit_code != 0) {
mbedtls_printf("Example failed with error %d\n", exit_code);
exit_code = MBEDTLS_EXIT_FAILURE;
}

mbedtls_platform_teardown(&platform_ctx);
mbedtls_platform_teardown(NULL);
return exit_code;
}
9 changes: 2 additions & 7 deletions tls-client/HelloHttpsClient.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,16 +71,11 @@ const char *HelloHttpsClient::HTTP_OK_STR = "200 OK";

HelloHttpsClient::HelloHttpsClient(const char *in_server_name,
const char *in_server_addr,
const uint16_t in_server_port,
mbedtls_platform_context* in_platform_ctx) :
const uint16_t in_server_port) :
socket(),
server_name(in_server_name),
server_addr(in_server_addr),
server_port(in_server_port),
/* The platform context is passed just in case any crypto calls need it.
* Please refer to https://github.com/ARMmbed/mbedtls/issues/1200 for more
* information. */
platform_ctx(in_platform_ctx)
server_port(in_server_port)
{
mbedtls_entropy_init(&entropy);
mbedtls_ctr_drbg_init(&ctr_drbg);
Expand Down
6 changes: 1 addition & 5 deletions tls-client/HelloHttpsClient.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
#include "TCPSocket.h"

#include "mbedtls/config.h"
#include "mbedtls/platform.h"
#include "mbedtls/ssl.h"
#include "mbedtls/entropy.h"
#include "mbedtls/ctr_drbg.h"
Expand Down Expand Up @@ -64,8 +63,7 @@ class HelloHttpsClient
*/
HelloHttpsClient(const char *in_server_name,
const char *in_server_addr,
const uint16_t in_server_port,
mbedtls_platform_context* in_platform_ctx);
const uint16_t in_server_port);

/**
* Free any allocated resources
Expand Down Expand Up @@ -233,8 +231,6 @@ class HelloHttpsClient
* The TLS configuration in use
*/
mbedtls_ssl_config ssl_conf;

mbedtls_platform_context* platform_ctx;
};

#endif /* _HELLOHTTPSCLIENT_H_ */
10 changes: 4 additions & 6 deletions tls-client/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,9 @@ const int SERVER_PORT = 443;
*/
int main()
{
mbedtls_platform_context platform_ctx;
int exit_code = MBEDTLS_EXIT_FAILURE;

if((exit_code = mbedtls_platform_setup(&platform_ctx)) != 0) {
if((exit_code = mbedtls_platform_setup(NULL)) != 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as above - remove the assignment to exit_code.

printf("Platform initialization failed with error %d\r\n", exit_code);
return MBEDTLS_EXIT_FAILURE;
}
Expand All @@ -74,13 +73,12 @@ int main()
#endif /* MBEDTLS_MAJOR_VERSION */

/* Allocate a HTTPS client */
client = new (std::nothrow) HelloHttpsClient(SERVER_NAME, SERVER_ADDR, SERVER_PORT,
&platform_ctx);
client = new (std::nothrow) HelloHttpsClient(SERVER_NAME, SERVER_ADDR, SERVER_PORT);

if (client == NULL) {
mbedtls_printf("Failed to allocate HelloHttpsClient object\n"
"\nFAIL\n");
mbedtls_platform_teardown(&platform_ctx);
mbedtls_platform_teardown(NULL);
return exit_code;
}

Expand All @@ -94,6 +92,6 @@ int main()

delete client;

mbedtls_platform_teardown(&platform_ctx);
mbedtls_platform_teardown(NULL);
return exit_code;
}