Skip to content
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.

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

Implement TLSSocket #8311

Merged
merged 2 commits into from
Oct 17, 2018
Merged

Implement TLSSocket #8311

merged 2 commits into from
Oct 17, 2018

Conversation

SeppoTakalo
Copy link
Contributor

@SeppoTakalo SeppoTakalo commented Oct 3, 2018

Description

Provide TLSSocket implementation. This implementation allows TLSSocket
to wrap around any existing socket. Currently only TLS supported. DTLS
not yet implemented.

Reviewed design document also provided with the implementation.

Documentation submitted to Mbed OS handbook.
ARMmbed/mbed-os-5-docs#759

NOTE: Design review has already been passed earlier. This PR is not requesting API review or design suggestions anymore.

Pull request type

[ ] Fix
[ ] Refactor
[ ] Target update
[X] Functionality change
[ ] Breaking change

@SeppoTakalo
Copy link
Contributor Author

@kjbracey-arm Please review.

@SeppoTakalo
Copy link
Contributor Author

I'll fix the Doxygen documentation blocks.

@SeppoTakalo
Copy link
Contributor Author

@0xc0170 Please test.

@0xc0170 0xc0170 requested review from a team and AnotherButler and removed request for a team October 4, 2018 15:34
@0xc0170
Copy link
Contributor

0xc0170 commented Oct 4, 2018

@AnotherButler Please review the design document

@SeppoTakalo Any security review needs to be done (tls team or similar) or all fine here?

@SeppoTakalo
Copy link
Contributor Author

@0xc0170 We have done the internal reviews already with TLS team and architecture group. See internal Jira ticket ONME-3788 for a record.

Thanks.

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 5, 2018

/morph build

Copy link
Contributor

@0xc0170 0xc0170 left a comment

Choose a reason for hiding this comment

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

There are 3 TODO left, shall they be captured somewhere?

@mbed-ci
Copy link

mbed-ci commented Oct 5, 2018

Build : FAILURE

Build number : 3245
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/8311/

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 5, 2018

Failures look related to the changeset

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 5, 2018

/morph build

@mbed-ci
Copy link

mbed-ci commented Oct 5, 2018

Build : FAILURE

Build number : 3249
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/8311/

@SeppoTakalo
Copy link
Contributor Author

Can this be retested now?

The test result is showing previous test just, just a minutes before rebase and fixing the Arm compiler issue.

Provide TLSSocket implementation. This implementation allows TLSSocket
to wrap around any existing socket. Currently only TLS supported. DTLS
not yet implemented.

Design document also provided with the implementation.

Documentation submitted to Mbed OS handbook.
@cmonr
Copy link
Contributor

cmonr commented Oct 15, 2018

/morph build

@mbed-ci
Copy link

mbed-ci commented Oct 15, 2018

Build : SUCCESS

Build number : 3357
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/8311/

Triggering tests

/morph test
/morph export-build
/morph mbed2-build

@mbed-ci
Copy link

mbed-ci commented Oct 15, 2018

@mbed-ci
Copy link

mbed-ci commented Oct 15, 2018

@cmonr
Copy link
Contributor

cmonr commented Oct 16, 2018

IAR network license issue.

/morph export-build
/morph mbed2-build

@mbed-ci
Copy link

mbed-ci commented Oct 16, 2018

@cmonr cmonr removed the needs: work label Oct 16, 2018
@cmonr
Copy link
Contributor

cmonr commented Oct 16, 2018

@ARMmbed/mbed-os-test When y'all have a moment, unsure what's going on with the cloud-client-test

@cmonr
Copy link
Contributor

cmonr commented Oct 16, 2018

Error located:

[Build K66F ARM] "/tmp/Mx9GAo", line 120 (column 3): Warning: L6312W: Empty Execution region description for region RW_IRAM1
[Build K66F ARM] Error: L6218E: Undefined symbol mbedtls_strerror (referred from BUILD/K66F/ARM/mbed-os/features/netsocket/TLSSocketWrapper.o).
[Build K66F ARM] Finished: 0 information, 1 warning and 1 error messages.
[Build K66F ARM] [ERROR] "/tmp/Mx9GAo", line 120 (column 3): Warning: L6312W: Empty Execution region description for region RW_IRAM1
[Build K66F ARM] Error: L6218E: Undefined symbol mbedtls_strerror (referred from BUILD/K66F/ARM/mbed-os/features/netsocket/TLSSocketWrapper.o).
[Build K66F ARM] Finished: 0 information, 1 warning and 1 error messages.
[Build K66F ARM]
[Build K66F ARM] [mbed] ERROR: "/usr/bin/python" returned error.
[Build K66F ARM] Code: 1
[Build K66F ARM] Path: "/builds/ws/mbed-os-ci_cloud-client-test/mbed-cloud-client-example"
[Build K66F ARM] Command: "/usr/bin/python -u /builds/ws/mbed-os-ci_cloud-client-test/mbed-cloud-client-example/mbed-os/tools/make.py -t ARM -m K66F --source . --build ./BUILD/K66F/ARM"
[Build K66F ARM] Tip: You could retry the last command with "-v" flag for verbose output
[Build K66F ARM] ---
[Build K66F ARM] [mbed] Auto-installing missing Python modules...

@ARMmbed/mbed-os-test The build step should have probably failed.

If MBEDTLS_ERROR_C is not defined, that strerror function does not
exist, or is just dummy definition if MBEDTLS_ERROR_STRERROR_DUMMY is
defined.
@SeppoTakalo
Copy link
Contributor Author

Cloud client build now fixed.

@cmonr
Copy link
Contributor

cmonr commented Oct 17, 2018

/morph build

@mbed-ci
Copy link

mbed-ci commented Oct 17, 2018

Build : SUCCESS

Build number : 3382
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/8311/

Triggering tests

/morph test
/morph export-build
/morph mbed2-build

@mbed-ci
Copy link

mbed-ci commented Oct 17, 2018

@mbed-ci
Copy link

mbed-ci commented Oct 17, 2018

@cmonr cmonr merged commit fc41c3d into ARMmbed:master Oct 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants