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

Update PASE state machine to match the latest specifications #9582

Merged
merged 4 commits into from
Sep 13, 2021

Conversation

pan-apple
Copy link
Contributor

Problem

PASE state machine code is out of date compared to latest specifications.

Change overview

Update the code to match the specifications.

  • Use TLV formatted messages
  • Use StatusReport to indicate errors and completion

Testing

TestPASESession unit test.
Tested PASE flow using all-cluster app and chip-tool commissioner.

* Use TLV formatted messages
* Use StatusReport to indicate errors and completion
@github-actions
Copy link

github-actions bot commented Sep 9, 2021

Size increase report for "gn_qpg-example-build" from fce8561

File Section File VM
chip-qpg6100-lighting-example.out .text 1324 1324
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-qpg6100-lighting-example.out and ./pull_artifact/chip-qpg6100-lighting-example.out:

sections,vmsize,filesize
.debug_info,0,17922
.debug_loc,0,3354
.debug_line,0,2375
.text,1324,1324
.debug_ranges,0,536
.debug_frame,0,176
.debug_abbrev,0,142
.symtab,0,48
.debug_aranges,0,32
.shstrtab,0,-1
.strtab,0,-183
.debug_str,0,-489
[Unmapped],0,-1324

Comparing ./master_artifact/chip-qpg6100-lighting-example.out.map and ./pull_artifact/chip-qpg6100-lighting-example.out.map:

BLOAT EXECUTION FAILED WITH CODE 1:
bloaty: unknown file type for file './pull_artifact/chip-qpg6100-lighting-example.out.map'


@github-actions
Copy link

Size increase report for "nrfconnect-example-build" from fce8561

File Section File VM
chip-lock.elf text 1284 1284
chip-lock.elf rodata 96 100
chip-lock.elf device_handles -4 -4
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-shell.elf and ./pull_artifact/chip-shell.elf:

sections,vmsize,filesize
.debug_info,0,1080
.debug_str,0,125
.debug_loc,0,3

Comparing ./master_artifact/chip-lock.elf and ./pull_artifact/chip-lock.elf:

sections,vmsize,filesize
.debug_info,0,17423
.debug_loc,0,3248
.debug_line,0,2395
text,1284,1284
.debug_ranges,0,440
.debug_frame,0,176
.debug_abbrev,0,142
rodata,100,96
.debug_aranges,0,32
.shstrtab,0,3
device_handles,-4,-4
.strtab,0,-183
.debug_str,0,-496


@github-actions
Copy link

Size increase report for "esp32-example-build" from fce8561

File Section File VM
chip-shell.elf .flash.text 48 48
chip-lock-app.elf .flash.text 1216 1216
chip-lock-app.elf .flash.rodata 32 32
chip-bridge-app.elf .flash.text 1184 1184
chip-bridge-app.elf .flash.rodata 32 32
chip-temperature-measurement-app.elf .flash.text 1124 1124
chip-temperature-measurement-app.elf .flash.rodata 32 32
chip-all-clusters-app.elf .flash.text 1032 1032
chip-all-clusters-app.elf .flash.rodata 48 48
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-persistent-storage.elf and ./pull_artifact/chip-persistent-storage.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-ipv6only-app.elf and ./pull_artifact/chip-ipv6only-app.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-shell.elf and ./pull_artifact/chip-shell.elf:

sections,vmsize,filesize
.debug_info,0,990
.debug_str,0,128
.flash.text,48,48
[Unmapped],0,-48
.debug_loc,0,-54

Comparing ./master_artifact/chip-lock-app.elf and ./pull_artifact/chip-lock-app.elf:

sections,vmsize,filesize
.debug_info,0,12521
.debug_line,0,5240
.debug_loc,0,2885
.flash.text,1216,1216
.debug_ranges,0,1096
.debug_frame,0,96
.debug_abbrev,0,38
.debug_aranges,0,32
.flash.rodata,32,32
.xt.lit._ZNK4chip4SpanIhE7SubSpanEjj,0,-8
.xt.prop._ZNK4chip4SpanIhE7SubSpanEjj,0,-12
.xt.prop._ZN4chip8Encoding22PacketBufferWriterBaseINS0_12LittleEndian12BufferWriterEEC2EONS_6System18PacketBufferHandleE,0,-36
.xt.prop._ZNK4chip8Encoding12BufferWriter3FitEv,0,-36
.symtab,0,-80
[ELF Section Headers],0,-160
.strtab,0,-183
.shstrtab,0,-409
.debug_str,0,-488
[Unmapped],0,-1248

Comparing ./master_artifact/chip-bridge-app.elf and ./pull_artifact/chip-bridge-app.elf:

sections,vmsize,filesize
.debug_info,0,17807
.debug_line,0,5288
.debug_loc,0,2915
[Unmapped],0,2880
.flash.text,1184,1184
.debug_ranges,0,1096
.debug_frame,0,96
.debug_abbrev,0,86
.debug_aranges,0,32
.flash.rodata,32,32
.xt.prop._ZN4chip8Encoding22PacketBufferWriterBaseINS0_12LittleEndian12BufferWriterEEC2EONS_6System18PacketBufferHandleE,0,-36
.xt.prop._ZNK4chip8Encoding12BufferWriter3FitEv,0,-36
.symtab,0,-80
[ELF Section Headers],0,-160
.strtab,0,-183
.shstrtab,0,-409
.debug_str,0,-492

Comparing ./master_artifact/chip-pigweed-app.elf and ./pull_artifact/chip-pigweed-app.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-temperature-measurement-app.elf and ./pull_artifact/chip-temperature-measurement-app.elf:

sections,vmsize,filesize
.debug_info,0,17493
.debug_line,0,5163
[Unmapped],0,2940
.debug_loc,0,2709
.flash.text,1124,1124
.debug_ranges,0,1096
.debug_frame,0,96
.debug_abbrev,0,86
.debug_aranges,0,32
.flash.rodata,32,32
.xt.prop._ZN4chip8Encoding22PacketBufferWriterBaseINS0_12LittleEndian12BufferWriterEEC2EONS_6System18PacketBufferHandleE,0,-36
.xt.prop._ZNK4chip8Encoding12BufferWriter3FitEv,0,-36
.symtab,0,-80
[ELF Section Headers],0,-160
.strtab,0,-183
.shstrtab,0,-409
.debug_str,0,-491

Comparing ./master_artifact/chip-all-clusters-app.elf and ./pull_artifact/chip-all-clusters-app.elf:

sections,vmsize,filesize
.debug_info,0,13814
.debug_line,0,3969
.debug_loc,0,2708
.flash.text,1032,1032
.debug_ranges,0,896
.debug_frame,0,188
.flash.rodata,48,48
.debug_abbrev,0,38
.debug_aranges,0,32
.riscv.attributes,0,2
.shstrtab,0,-1
.symtab,0,-16
.strtab,0,-183
.debug_str,0,-491
[Unmapped],0,-1080


Copy link
Contributor

@tcarmelveilleux tcarmelveilleux left a comment

Choose a reason for hiding this comment

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

Approved conditional on the few comments marked with 👀 emoji being resolved

@todo
Copy link

todo bot commented Sep 10, 2021

- Update <Set/Get><Local/Peer>KeyId() functions to <Set/Get><Local/Peer>SessionId()

// TODO - Update <Set/Get><Local/Peer>KeyId() functions to <Set/Get><Local/Peer>SessionId()
SetPeerKeyId(initiatorSessionId);
SuccessOrExit(err = tlvReader.Next());
VerifyOrExit(TLV::TagNumFromTag(tlvReader.GetTag()) == ++decodeTagIdSeq, err = CHIP_ERROR_INVALID_TLV_TAG);
SuccessOrExit(err = tlvReader.Get(mPasscodeID));
SuccessOrExit(err = tlvReader.Next());
VerifyOrExit(TLV::TagNumFromTag(tlvReader.GetTag()) == ++decodeTagIdSeq, err = CHIP_ERROR_INVALID_TLV_TAG);
SuccessOrExit(err = tlvReader.Get(hasPBKDFParameters));


This comment was generated by todo based on a TODO comment in bea61e6 in #9582. cc @pan-apple.

@todo
Copy link

todo bot commented Sep 10, 2021

- Add a unit test that exercises mHavePBKDFParameters path

// TODO - Add a unit test that exercises mHavePBKDFParameters path
err = SetupSpake2p(mIterationCount, ByteSpan(mSalt, mSaltLength));
SuccessOrExit(err);
}
else
{
SuccessOrExit(err = tlvReader.Next());
SuccessOrExit(err = tlvReader.EnterContainer(containerType));
decodeTagIdSeq = 0;
SuccessOrExit(err = tlvReader.Next());


This comment was generated by todo based on a TODO comment in bea61e6 in #9582. cc @pan-apple.

@todo
Copy link

todo bot commented Sep 10, 2021

- Move SendStatusReport to a common part of the code.

// TODO - Move SendStatusReport to a common part of the code.
// This could be reused for all secure channel protocol state machinies.
GeneralStatusCode generalCode =
(protocolCode == kProtocolCodeSuccess) ? GeneralStatusCode::kSuccess : GeneralStatusCode::kFailure;
uint32_t protocolId = Protocols::SecureChannel::Id.ToFullyQualifiedSpecForm();
ChipLogDetail(SecureChannel, "Sending status report");
StatusReport statusReport(generalCode, protocolId, protocolCode);
Encoding::LittleEndian::PacketBufferWriter bbuf(System::PacketBufferHandle::New(statusReport.Size()));


This comment was generated by todo based on a TODO comment in bea61e6 in #9582. cc @pan-apple.

@todo
Copy link

todo bot commented Sep 10, 2021

- Move EstimateTLVStructOverhead to CHIPTLV header file

// TODO - Move EstimateTLVStructOverhead to CHIPTLV header file
constexpr size_t EstimateTLVStructOverhead(size_t dataLen, size_t nFields) { return dataLen + (sizeof(uint64_t) * nFields); }
void CloseExchange();
SessionEstablishmentDelegate * mDelegate = nullptr;
Protocols::SecureChannel::MsgType mNextExpectedMsg = Protocols::SecureChannel::MsgType::PASE_PakeError;
#ifdef ENABLE_HSM_SPAKE
Spake2pHSM_P256_SHA256_HKDF_HMAC mSpake2p;


This comment was generated by todo based on a TODO comment in bea61e6 in #9582. cc @pan-apple.

@todo
Copy link

todo bot commented Sep 10, 2021

- Update <Set/Get><Local/Peer>KeyId() functions to <Set/Get><Local/Peer>SessionId()

// TODO - Update <Set/Get><Local/Peer>KeyId() functions to <Set/Get><Local/Peer>SessionId()
SetPeerKeyId(initiatorSessionId);
SuccessOrExit(err = tlvReader.Next());
VerifyOrExit(TLV::TagNumFromTag(tlvReader.GetTag()) == ++decodeTagIdSeq, err = CHIP_ERROR_INVALID_TLV_TAG);
SuccessOrExit(err = tlvReader.Get(mPasscodeID));
SuccessOrExit(err = tlvReader.Next());
VerifyOrExit(TLV::TagNumFromTag(tlvReader.GetTag()) == ++decodeTagIdSeq, err = CHIP_ERROR_INVALID_TLV_TAG);
SuccessOrExit(err = tlvReader.Get(hasPBKDFParameters));


This comment was generated by todo based on a TODO comment in 27dd893 in #9582. cc @pan-apple.

@woody-apple
Copy link
Contributor

@andy31415 ?

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.

6 participants