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

Add SBOM files for applicable components #212

Closed
wants to merge 1 commit into from

Conversation

mahavirj
Copy link
Member

Please see more details at:
https://github.com/espressif/esp-idf-sbom

Checklist

  • CI passing

Change description

Please describe your change here

@mahavirj
Copy link
Member Author

@fhrbata PTAL

@github-actions
Copy link

Unit Test Results

  11 files  ±0    11 suites  ±0   15m 7s ⏱️ -2s
  26 tests ±0    26 ✔️ ±0  0 💤 ±0  0 ±0 
243 runs  ±0  243 ✔️ ±0  0 💤 ±0  0 ±0 

Results for commit e17cff5. ± Comparison against base commit b383228.

@@ -1,4 +1,5 @@
version: "2.5.0"
# Note: Please keep this version consistent with sbom.yml file
version: "2.5.0~1"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi @mahavirj , I'm a little bit confused and probably missing something. Why are we changing the versions in idf_component.yml? For example here from 2.5.0 to 2.5.0~1. Also just a note, if the version is not found in sbom.yml it will be take from idf_component.yml if presented. The same goes for description. So maybe we can avoid the duplication. Now I'm starting to think that maybe we should ask @kumekay if it would be possible to add the sbom fields support into idf_component.yml so we do not have to keep the info on two places for the managed components. Just a thought. Thank you.

Copy link
Member Author

Choose a reason for hiding this comment

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

Since we are adding a new file to this component, we must bump up its version so that it gets pushed to the upstream registry.

Also just a note, if the version is not found in sbom.yml it will be take from idf_component.yml if presented.

Okay, in that case, I can remove version field from the sbom.yml file. Probably we can remove description field as well?

it would be possible to add the sbom fields support into idf_component.yml so we do not have to keep the info on two places for the managed components

That makes sense.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you for the explanation @mahavirj . I will sync with @kumekay to see what can be done about this.

Copy link
Member

Choose a reason for hiding this comment

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

It would also make the implementation of automatic version updates (#174) a lot easier if there are less files which have to be modified for each version bump.

For now, espressif/github-actions#40 has added support for updating the submodule itself and the version in idf_component.yml. Having to update also sbom-version and sbom-hash in .gitmodules, plus the new sbom.yml file, definitely makes the implementation more complex...

Copy link
Member

Choose a reason for hiding this comment

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

By the way, do I understand it right that after the SBOM information is moved into sbom.yml, we will revert the .gitmodules changes done in #199? I'm trying to understand whether I have to implement automatic updates to sbom-version and sbom-hash entries from update_submodule_versions action.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@mahavirj , @kumekay I quickly tried, with idf-component-manager 1.3.2, some local test managed component and I can confirm that the root variables are used and not reported. With e.g. 1.2.3 I get this

Invalid manifest format
Unknown keys: cpe, originator, supplier
SUGGESTION: This component may be using a newer version of the component manager.

I just wanted to point this out, because if we update idf_component.yml files, people with older manager will run into this. I'm not sure if this is(could be) a problem.

Copy link
Member Author

Choose a reason for hiding this comment

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

have a special sbom namespace

This could be an override switch as well. If the version is present under sbom namespace then pick that one, else use the component version field itself.

Copy link
Member

Choose a reason for hiding this comment

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

This could be an override switch as well. If the version is present under sbom namespace then pick that one, else use the component version field itself.

Don't we actually have two libraries, from SBOM point of view, here?

flowchart TD
    app["Application"]
    espressif/libpng["espressif/libpng\n1.6.39~2"]
    libpng["libpng\n1.6.39"]
    app -- depends --> espressif/libpng
    espressif/libpng -- depends --> libpng
Loading

And in some cases, there could be more than one library inside a component. For example, console component in IDF:

flowchart TD
    app["Application"]
    console["$IDF_PATH/console"]
    argtable3["argtable3\n3.2.2"]
    linenoise["linenoise\n1.0.1"]
    app -- depends --> console
    console -- depends --> argtable3
    console -- depends --> linenoise
Loading

Not saying we should handle such cases in this MR, just thought it's relevant to the discussion.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi @igrr , thank you for the graphs!

IIUC the problem we are trying to solve is that even though we have the sbom information for submodules in .gitmodules, it's of course not propagated through component manager. So if I do

idf.py add-dependency "espressif/libpng^1.6.39"

the libpng component added to my project has no info that the libpng subdir in managed_components/espressif__libpng/ is actually a submodule in the idf-extra-components repo with sbom info for it in .gitmodules.

I agree with your point above, that from the sbom POV the libpng component should depend on libpng library, but this relationship is currently not possible to express in the tool :(. It works like presented in your graph if we have submodule entry for the libpng library available in .gitmodules and .gitmodules available while the sbom is generated.

For the first libpng component example, I think we have to extend the sbom.yml to allow to express this relationship. Maybe something like this added in idf_component.yml

sbom:
  contains:
    - path: libpng
      name: libpng
      version: 1.6.39
      cpe: cpe:2.3:a:libpng:libpng:{}:*:*:*:*:*:*:*
      supplier: 'Organization: libpng'
      description: Portable Network Graphics support, official PNG reference library

This would actually replace the sbom data and component->submodule relationship from .gitmodules .

For the second console component example, I think we can use the same approach as for libpng. We would just add two libs: argtable3 and linenoise. Also IIUC argtable3 and linenoise are actually not submodules, so maybe we can added their own sbom.yml into their dirs.

components/console/
├── argtable3
│   └── sbom.yml
├── linenoise
│   └── sbom.yml
└── sbom.yml

Anyway both approaches require changes in the tool. I'm open and grateful for any ideas WRT this. Thank you very much!

Copy link
Member Author

Choose a reason for hiding this comment

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

@fhrbata Proposal sounds good to me. Please let me know once you have tool side changes in place, alternatively please feel free to take over this PR. Thanks.

@fhrbata
Copy link
Collaborator

fhrbata commented Sep 17, 2023

Hi @mahavirj ,

Here #239 is a PR, which adds the sbom information using referenced manifest files. It a draft, because we also need to change the checking in CI and probably remove the sbom information from .gitmodules. I opened MR for esp-idf-sbom, which should allow to validate(check the hashes) recorded in manifest files, not only in .gitmodules.

Thank you

@mahavirj
Copy link
Member Author

Closing in favor or #239

@mahavirj mahavirj closed this Sep 18, 2023
@mahavirj mahavirj deleted the feature/components_sbom_yml branch October 9, 2023 06:32
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.

4 participants