Add GetDevicePublicKey

[aromaa]

Description

• Adds GetDevicePublicKey.
• Bump version.

Motivation and Context

• Allows developers to get the public key from the device certificate store.

How Has This Been Tested?

• Locally by calling CertificateManager.GetDevicePublicKey().

Screenshots

Types of changes

• Improvement (non-breaking change that improves a feature, code or algorithm)
• Bug fix (non-breaking change which fixes an issue with code or algorithm)
• New feature (non-breaking change which adds functionality to code)
• Breaking change (fix or feature that would cause existing functionality to change)
• Config and build (change in the configuration and build system, has no impact on code or features)
• Dependencies (update dependencies and changes associated, has no impact on code or features)
• Unit Tests (add new Unit Test(s) or improved existing one(s), has no impact on code or features)
• Documentation (changes or updates in the documentation, has no impact on code or features)

Checklist:

• My code follows the code style of this project (only if there are changes in source code).
• My changes require an update to the documentation (there are changes that require the docs website to be updated).
• I have updated the documentation accordingly (the changes require an update on the docs in this repo).
• I have read the CONTRIBUTING document.
• I have tested everything locally and all new and existing tests passed (only if there are changes in source code).
• I have added new tests to cover my changes.

Summary by CodeRabbit

• Chores
  • Bumped application version from 1.11 to 1.12

[josesimoes]

@aromaa I was picturing this living in in the X502Cert class.... 🤔
https://github.com/nanoframework/System.Net/blob/main/nanoFramework.System.Net/X509Certificates/X509Certificate2.cs

[aromaa]

I was picturing this living in in the X502Cert class

Isn't that a bit funny? The certificate itself shouldn't know about how/where to retrieve certificates. The CertificateManager is already dealing with the certificate store so it seems like the most obvious place?

[coderabbitai]

Walkthrough

The pull request involves a simple version update in the version.json file, incrementing the version number from 1.11 to 1.12. This change suggests a new release version of the project, with no modifications to the file's structure or other configurations.

Changes

File	Change Summary
version.json	Version number updated from "1.11" to "1.12"

---

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d91fcb6 and 7dbb315.

⛔ Files ignored due to path filters (2)

• nanoFramework.System.Net/Properties/AssemblyInfo.cs is excluded by none and included by none
• nanoFramework.System.Net/Security/CertificateManager.cs is excluded by none and included by none

📒 Files selected for processing (1)

• version.json (1 hunks)

✅ Files skipped from review due to trivial changes (1)

• version.json

⏰ Context from checks skipped due to timeout of 90000ms (1)

• GitHub Check: System.Net (Build_Library)

---

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[josesimoes]

I was looking at this again.
Understood your point about this new API belonging in the certificate manager.
Now about the name, there should be no doubts on what public key this is all about. Therefore, I suggest renaming the method to GetDeviceCertificatePublicKey().

[josesimoes]

Reviewing the native implementation that one is actually returning the device certificate from storage.
This is going against the policy that we've been following with making it difficult/hard to extract information from a nano device.
Being able to so easily extract a device certificate seems like going against that. And a security flaw, btw.
@Ellerbach can you please chime in?

I understand the convenience to retrieve properties from the device cert to automate interaction with Azure services, for example.
Why don't we list those and implement it in such a way that only those can leave the device through the API?

[aromaa]

Reviewing the native implementation that one is actually returning the device certificate from storage.

I'm not sure I follow, the native implementation extracts the device certificate yes, but it does indeed only extract the PUBLIC key. Is the implementation not sufficient in all use cases?

For my use case, I have indeed verified that ONLY the public key is returned. There is no private key exposed to the managed layer. Are there cases where you observed the private key leaking out?

[josesimoes]

For my use case, I have indeed verified that ONLY the public key is returned. There is no private key exposed to the managed layer. Are there cases where you observed the private key leaking out?

I haven't said that the private key is being leaked. Rather that the full device certificate is being exposed.
In the new ssl_get_public_key_raw_internal()
https://github.com/nanoframework/nf-interpreter/pull/2400/files#diff-fa4ff28619012bd2de17f1e5516d630a33b4a979d8076f55c6b9d6136a70e3ee
At line 78 the certificate is being copied and that is what's being returned to the managed code in this PR.

I understand that accessing a device certificate in the Azure IoT Hub context is not inherently a security flaw. Still, I'm concerned that exposing it maybe against good practices. That's why I'm suggesting exposing only the relevant bits.
Can you let us know what properties of the X509 cert are you actually using?

[aromaa]

I understand that accessing a device certificate in the Azure IoT Hub context is not inherently a security flaw.

I see, I would be interested to know in which conditions would exposing the public key be a security flaw.

Still, I'm concerned that exposing it maybe against good practices. That's why I'm suggesting exposing only the relevant bits.
Can you let us know what properties of the X509 cert are you actually using?

I have no issues with it if its actually a security flaw. Yes, I'm currently using only the subject part.

[Ellerbach]

I see, I would be interested to know in which conditions would exposing the public key be a security flaw.

Exposing the public key is not a security concern at all. The private key is!

Now, when the cert is uploaded in the device, the magic when using Azure IoT Hub or anything else that require the cert should just work out of the box (as far as I remember with my various tests). And I'd like to understand the scenario for having only the pubic key.

[Ellerbach]

@aromaa looking at this into more details, I agree with José, we should not expose it like this. Because, you'll get everything from the certificate and in that case, it's not just the public key.

If you need to play with certs (CA or device certs), you can use:

• Internal Storage, if you need to read/write certs and other elements, use it!
• Configuration that you can use and it's considered as static, so, if you upload certs here, it's not accessible by code once flashed.

In both cases, you can use nanoff (for the second case, it's under PR, so very very soon) to flash those data into the storage or the device configuration.

[aromaa]

looking at this into more details, I agree with José, we should not expose it like this. Because, you'll get everything from the certificate and in that case, it's not just the public key.

Could you please elaborate in which cases does that apply to? For my limited testing, I have not observed my private key to being leaked out.

[josesimoes]

@aromaa no one has ever ever mentioned that the private key was leaking or has been observed. So, let's put that aside, OK?
Could explain what do you want to extract from the public key (or device cert, btw) and do you use it?

(if you prefer, OK to discuss this on Discord as the conversation flow is more smooth)

[aromaa]

no one has ever ever mentioned that the private key was leaking or has been observed. So, let's put that aside, OK?

All I am asking is what are we trying to protect here. Nobody has elaborated any further. We both have mutual understanding that the private key should not leak out, that is a must. But what else is there that is a secret other than the private key then?