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 openSSL based AES 256 CCM #326

Closed
wants to merge 10 commits into from
Closed

Implement openSSL based AES 256 CCM #326

wants to merge 10 commits into from

Conversation

bhaskar-apple
Copy link
Contributor

@bhaskar-apple bhaskar-apple commented Apr 9, 2020

Problem

Implement openSSL based AES-256 CCM

Summary of Changes

  1. Implement the encrypt and decrypt functions in src/crypto/CHIPCryptoPALOpenSSL.cpp. Note I changed the existing C stub implementation file to be C++ as some of the pulled in headers are C++ e.g. CHIP_ERROR
  2. I wasn't sure what the best thing to do here was with regards to openSSL and so made a new openSSL only library libChipCryptoOpenSSL in src/crypto/Makefile.am that implements these common crypto primitives.
  3. Add unit tests based on some sample test vectors in src/crypto/tests/CHIPCryptoPALOpenSSLTest.cpp.
  4. Added a new test target TestOpenSSLCrypto for the tests.

fixes : #256

@bhaskar-apple bhaskar-apple changed the title Bs aescc mopen ssl Implement openSSL based AES 256 CCM Apr 9, 2020
@bhaskar-apple
Copy link
Contributor Author

@balducci-apple - please take a look.

@gerickson
Copy link
Contributor

@roydsouza, @emargolis, @dougsteed

@bhaskar-apple
Copy link
Contributor Author

bhaskar-apple commented Apr 10, 2020

Figure out why github build is failing

Copy link
Contributor

@woody-apple woody-apple left a comment

Choose a reason for hiding this comment

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

Build is failing:
https://github.com/bhaskar-apple/connectedhomeip/runs/575597212

Making all in crypto
4637
  CC       libChipCrypto_a-CHIPOpenSSL.o
4638
../../../../src/crypto/CHIPOpenSSL.c:7:10: fatal error: CHIPCrypto.h: No such file or directory
4639
 #include "CHIPCrypto.h"
4640
          ^~~~~~~~~~~~~~
4641

"name": "CHIP openSSL Tests",
"type": "cppdbg",
"request": "launch",
"program": "${workspaceFolder}/build/x86_64-unknown-linux-gnu/src/crypto/tests/TestOpenSSLCrypto",
Copy link
Contributor

@rwalker-apple rwalker-apple Apr 10, 2020

Choose a reason for hiding this comment

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

is build/x86_64-unknown-linux-gnu working for you?

reason I ask is that tasks.json has been changed to build into build/default


#define kKeyLengthInBits 256

bool _isValidTagLength(size_t tag_length)
Copy link
Contributor

Choose a reason for hiding this comment

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

can this be static (i.e. not exported?)


static void TestAES_CCM_256EncryptTestVectors(nlTestSuite * inSuite, void * inContext)
{
int numOfTestVectors = sizeof(ccm_test_vectors) / sizeof(ccm_test_vectors[0]);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think one of our nl friends has ArraySize()

* Test Suite. It lists all the test functions.
*/

// clang-format off
Copy link
Contributor

Choose a reason for hiding this comment

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

why?

@bhaskar-apple
Copy link
Contributor Author

Closing this as the commit history on this one is too messed up and I am having a hard time reconciling it.
Opening a new one.

@bhaskar-apple bhaskar-apple deleted the BS-AESCCMopenSSL branch April 13, 2020 22:21
mlepage-google added a commit to mlepage-google/connectedhomeip that referenced this pull request Nov 24, 2021
Field identifiers were not being used. Instead, the order of field
identifiers was used, with ordinal (0 to N-1) indexes.

This is likely true of other code generated elements, but this PR only
fixes it for struct items.

This PR requires PR project-chip#326 in the third_party/zap/repo to function,
as that PR actually includes the field identifier in the query
results.

Tested by running Matter cert tests, and also by inspecting the
generated code for correctness.
andy31415 added a commit that referenced this pull request Nov 25, 2021

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
* Use field identifiers when code gen struct items

Field identifiers were not being used. Instead, the order of field
identifiers was used, with ordinal (0 to N-1) indexes.

This is likely true of other code generated elements, but this PR only
fixes it for struct items.

This PR requires PR #326 in the third_party/zap/repo to function,
as that PR actually includes the field identifier in the query
results.

Tested by running Matter cert tests, and also by inspecting the
generated code for correctness.

* Update zap submodule to latest

* Update a few generated zap files

Re-ran cert tests.

* Restyling

Why are we styling generated code?!

* Revert zap repo update

* Update zap to latest zap version

* Regen zap with the new zap version

Co-authored-by: Andrei Litvin <andy314@gmail.com>
rerasool pushed a commit to SiliconLabs/matter that referenced this pull request Dec 19, 2022
…eature/CCP_BLE_bringup to RC_1.0.2-1.0

Merge in WMN_TOOLS/matter from cherry_pick/tinycrypt_changes to RC_1.0.2-1.0

Squashed commit of the following:

commit 003a64f0f3d3e5f22db764fd9bb8293d5ad4a00d
Author: ShubhamNarayanrao Malasane <ShubhamNarayanrao.Malasane@silabs.com>
Date:   Wed Nov 30 14:54:14 2022 +0000

    Pull request project-chip#326: Feature/shubham tinycrypt

    Merge in WMN_TOOLS/matter from feature/shubham_tinycrypt to feature/CCP_BLE_bringup

    Squashed commit of the following:

    commit 40c432ef8ac2a7ff34940c42f969bc70c1af5068
    Author: ShubhamMalasane <shubhamnarayanrao.malasane@silabs.com>
    Date:   Fri Nov 25 13:11:22 2022 +0530

        modified gni file

    commit 9bb17ab4eeefa1d65929ddd81b111840c5d7bc17
    Author: ShubhamMalasane <shubhamnarayanrao.malasane@silabs.com>
    Date:   Thu Nov 24 14:20:07 2022 +0530

        remove changes from platform specific file

    commit 6faa303012a0211ef0805b08cfb253f09a6ff624
    Author: ShubhamMalasane <shubhamnarayanrao.malasane@silabs.com>
    Date:   Wed Nov 23 22:43:07 2022 +0530

        tinycrypt changes
jamesharrow pushed a commit that referenced this pull request Dec 4, 2023
DSRR-160-B-Implement_SDK_DEM_Delegate

Approved-by: James Harrow
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Need to implement openSSL based AES-CCM
5 participants