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

scripts: west_commands: ncs-provision locked key #19328

Merged
merged 1 commit into from
Dec 10, 2024

Conversation

michalek-no
Copy link
Contributor

@michalek-no michalek-no commented Dec 6, 2024

adds ability to upload key as locked.

ref.: NCSDK-30867

@michalek-no michalek-no requested a review from a team as a code owner December 6, 2024 15:09
@github-actions github-actions bot added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Dec 6, 2024
@NordicBuilder
Copy link
Contributor

NordicBuilder commented Dec 6, 2024

CI Information

To view the history of this post, clich the 'edited' button above
Build number: 3

Inputs:

Sources:

sdk-nrf: PR head: 7e5cbef86ad26baa2cdf0234a26cc1c5d3108974

more details

sdk-nrf:

PR head: 7e5cbef86ad26baa2cdf0234a26cc1c5d3108974
merge base: 81814fef7447690d16de8b884e164e9b9aea2f70
target head (main): 8965d4f9a81c5f326f108d177886d219caeabe2f
Diff

Github labels

Enabled Name Description
ci-disabled Disable the ci execution
ci-all-test Run all of ci, no test spec filtering will be done
ci-force-downstream Force execution of downstream even if twister fails
ci-run-twister Force run twister
ci-run-zephyr-twister Force run zephyr twister
List of changed files detected by CI (1)
scripts
│  ├── west_commands
│  │  │ ncs-provision.py

Outputs:

Toolchain

Version:
Build docker image:

Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped; ⚠️ Quarantine

  • ◻️ Toolchain
  • ◻️ Build twister
  • ◻️ Integration tests
Disabled integration tests
    • desktop52_verification
    • test_ble_nrf_config
    • test-fw-nrfconnect-apps
    • test-fw-nrfconnect-ble_mesh
    • test-fw-nrfconnect-ble_samples
    • test-fw-nrfconnect-boot
    • test-fw-nrfconnect-chip
    • test-fw-nrfconnect-fem
    • test-fw-nrfconnect-nfc
    • test-fw-nrfconnect-nrf-iot_libmodem-nrf
    • test-fw-nrfconnect-nrf-iot_lwm2m
    • test-fw-nrfconnect-nrf-iot_mosh
    • test-fw-nrfconnect-nrf-iot_nrf_provisioning
    • test-fw-nrfconnect-nrf-iot_positioning
    • test-fw-nrfconnect-nrf-iot_samples
    • test-fw-nrfconnect-nrf-iot_serial_lte_modem
    • test-fw-nrfconnect-nrf-iot_thingy91
    • test-fw-nrfconnect-nrf-iot_zephyr_lwm2m
    • test-fw-nrfconnect-nrf_crypto
    • test-fw-nrfconnect-ps
    • test-fw-nrfconnect-rpc
    • test-fw-nrfconnect-rs
    • test-fw-nrfconnect-tfm
    • test-fw-nrfconnect-thread
    • test-fw-nrfconnect-zigbee
    • test-low-level
    • test-sdk-audio
    • test-sdk-dfu
    • test-sdk-find-my
    • test-sdk-mcuboot
    • test-sdk-pmic-samples
    • test-sdk-sidewalk
    • test-sdk-wifi
    • test-secdom-samples-public

Note: This message is automatically posted and updated by the CI

@nvlsianpu nvlsianpu requested a review from MarekPieta December 6, 2024 15:19
@@ -34,6 +36,8 @@ def do_add_parser(self, parser_adder):
"-k", "--key", type=Path, action='append', dest="keys",
help="Input .pem file with ED25519 private key"
)
upload_parser.add_argument("-p", "--policy", type=str, help="Keys policy",
choices=["default", "lock"], default="default")
Copy link

Choose a reason for hiding this comment

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

It's not clear what default means, why don't you use choices=["revoked", "locked"] and

  "-r",
  args.policy.uppder(),
  "-v",

Copy link
Contributor Author

Choose a reason for hiding this comment

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

translation table on top

Copy link
Contributor

Choose a reason for hiding this comment

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

agree with @fundakol , don't need extra table, and why to change parameter names from nrfprovision tool?
you can use:

        upload_parser.add_argument("-p", "--policy", type=str.upper, help="Keys policy",
            choices=["REVOKED", "ROTATING", "LOCKED"], default="REVOKED")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Name 'revoked' suggests it's dead right away. It should be named differently earlier on but here we are.

@nvlsianpu nvlsianpu added backport v2.8-branch auto-create a PR with same commits to v2.8-branch backport v2.9-branch auto-create a PR with same commits to v2.9-branch bugfix Fixes a known bug labels Dec 6, 2024
@michalek-no michalek-no requested a review from fundakol December 9, 2024 07:47
adds ability to upload key as locked.

Signed-off-by: Mateusz Michalek <[email protected]>
@michalek-no
Copy link
Contributor Author

rebase

@nvlsianpu nvlsianpu added this to the 2.9.0 milestone Dec 10, 2024
@shanthanordic
Copy link

@nvlsianpu should this be backported to 2.8 branch as well ?

@rlubos rlubos removed the backport v2.8-branch auto-create a PR with same commits to v2.8-branch label Dec 10, 2024
@rlubos rlubos merged commit c382665 into nrfconnect:main Dec 10, 2024
17 checks passed
@nvlsianpu
Copy link
Contributor

@shanthanordic Yes it should - it's mitigation to known issue, backward compatible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport v2.9-branch auto-create a PR with same commits to v2.9-branch bugfix Fixes a known bug changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants