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.

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

Jump to bottom

[1.0] Missing ICAC at wrapper init can cause unexpected NOC chain generation on Android #22455

Closed
tcarmelveilleux opened this issue Sep 7, 2022 · 0 comments · Fixed by #22458
Closed

[1.0] Missing ICAC at wrapper init can cause unexpected NOC chain generation on Android #22455

tcarmelveilleux opened this issue Sep 7, 2022 · 0 comments · Fixed by #22458
Assignees
@tcarmelveilleux

Comments

@tcarmelveilleux
Copy link
Contributor

Reproduction steps

1. Call `AndroidDeviceControllerWrapper::AllocateNew` will null byte array for existing ICAC
2. Observe that wrapper silently ignores the RCAC and NOC provided and uses opcreds issuer to generate an ephemeral keypair instead.

Bug prevalence

100%

GitHub hash of the SDK that was being used

383c416

Platform

android

Platform Version(s)

No response

Type

Platform Issue

Anything else?

This bug was quite hard to find and caused some users of Android APIs that did not use ICACs in their infrastructure to be unable to initialize the SDK without workaround that are non-obvious at first, such as passing an empty byte array when null is a valid java equivalent in most instances.
@tcarmelveilleux tcarmelveilleux self-assigned this Sep 7, 2022
tcarmelveilleux added a commit to tcarmelveilleux/connectedhomeip that referenced this issue Sep 7, 2022
@tcarmelveilleux
Allow null ICAC in AndroidDeviceControllerWrapper::AllocateNew
614d6c8 
- Passing null for existing ICAC, when a NOC and RCAC were provided and desired to
  be used casued confusing behavior of NOC/RCAC being ignored and a completely
  different cert chain being generated.

Fixes project-chip#22455

Changes done:
- Add code to just use empty internal ICAC when null is provided in Java
- Add log to tell users when an ephemeral NOC chain is generated.

Testing done:
- CI still passes
- Tested in unit/integration with our internal infrastructure that uses this API
  both with/without ICAC
@tcarmelveilleux tcarmelveilleux mentioned this issue Sep 7, 2022
Merged
woody-apple pushed a commit to tcarmelveilleux/connectedhomeip that referenced this issue Sep 8, 2022
@tcarmelveilleux @woody-apple
Allow null ICAC in AndroidDeviceControllerWrapper::AllocateNew
80669af 
- Passing null for existing ICAC, when a NOC and RCAC were provided and desired to
  be used casued confusing behavior of NOC/RCAC being ignored and a completely
  different cert chain being generated.

Fixes project-chip#22455

Changes done:
- Add code to just use empty internal ICAC when null is provided in Java
- Add log to tell users when an ephemeral NOC chain is generated.

Testing done:
- CI still passes
- Tested in unit/integration with our internal infrastructure that uses this API
  both with/without ICAC
woody-apple pushed a commit that referenced this issue Sep 8, 2022
@tcarmelveilleux
Allow null ICAC in AndroidDeviceControllerWrapper::AllocateNew (#22458
ac30d99 
)

* Allow null ICAC in `AndroidDeviceControllerWrapper::AllocateNew`

- Passing null for existing ICAC, when a NOC and RCAC were provided and desired to
  be used casued confusing behavior of NOC/RCAC being ignored and a completely
  different cert chain being generated.

Fixes #22455

Changes done:
- Add code to just use empty internal ICAC when null is provided in Java
- Add log to tell users when an ephemeral NOC chain is generated.

Testing done:
- CI still passes
- Tested in unit/integration with our internal infrastructure that uses this API
  both with/without ICAC

* Restyled
isiu-apple pushed a commit to isiu-apple/connectedhomeip that referenced this issue Sep 16, 2022
@tcarmelveilleux @isiu-apple
Allow null ICAC in AndroidDeviceControllerWrapper::AllocateNew (pro…
d789d9b 
…ject-chip#22458)

* Allow null ICAC in `AndroidDeviceControllerWrapper::AllocateNew`

- Passing null for existing ICAC, when a NOC and RCAC were provided and desired to
  be used casued confusing behavior of NOC/RCAC being ignored and a completely
  different cert chain being generated.

Fixes project-chip#22455

Changes done:
- Add code to just use empty internal ICAC when null is provided in Java
- Add log to tell users when an ephemeral NOC chain is generated.

Testing done:
- CI still passes
- Tested in unit/integration with our internal infrastructure that uses this API
  both with/without ICAC

* Restyled
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tcarmelveilleux tcarmelveilleux

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Allow null ICAC in AndroidDeviceControllerWrapper::AllocateNew tcarmelveilleux/connectedhomeip
1 participant
@tcarmelveilleux