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 manufacturer information to SBOM metadata #582

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

kornefalk
Copy link

@kornefalk kornefalk commented Nov 20, 2024

Adding manufacturer information to the Maven Plugin. (see https://cyclonedx.org/docs/1.6/json/#metadata_manufacturer for model reference)

This is very useful for companies and organizations that can add information to the SBOM files about the source and how to contact the application vendor. This information is optional and one configuration example is included. If this information is not included, the plugin works as before without the manufacturer section.

When using a centralized parent POM file, all artefacts can create a manufacturer information. This kind of information is not often changed withing the company or organization.

Some checks of the manufacturer information is available, should maybe be moved to the core project.

@ppkarwasz
Copy link
Contributor

ppkarwasz commented Nov 20, 2024

I would rather reuse what is already there in the POM file, e.g.:

<organization>
  <name>The Apache Software Foundation</name>
  <url>https://apache.org</url>
</organization>
<developers>
  <developer>
    <name>Apache Logging Services Security Team</name>
    <email>[email protected]</email>
    <url>https://logging.apache.org/security.html</url>
    <roles>
      <role>security_contact</role>
    </roles>
    <properties>
      <postalAddress>
        The Apache Software Foundation
        1000 N West Street, Suite 1200
        Wilmington, DE 19801
        U.S.A.
      </postalAddress>
    </properties>
  </developer>
</developers>

The Developers section is there to provide a contact with the developers.

@kornefalk
Copy link
Author

The organization section is also relevant to the manufacturer information. All fields in the *developers` properties section is "This element is where any other properties about the person goes."

I can rewrite this PR with usage of the organization and developers, and it will not by default add fields that does not match one to one like postal address.

Comment on lines 39 to 56
OrganizationalEntity manufacturer = mojo.createManufacturer(null, null);
assertNotNull(manufacturer);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be null if there is no data?

Copy link
Author

Choose a reason for hiding this comment

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

Security check if you have a very minimal POM file without anything. Could be removed, since checks are in place before calling the tested function, but I didn't see any unit testing at all to the BaseCycloneDxMojo, only some testing with POM files running with the super classes. Writing unit test for execute() function will take lot of time.

                if (organization != null || (developers != null && !developers.isEmpty())) {
                    metadata.setManufacturer(createManufacturer(organization, developers));
                }

Comment on lines 125 to 130
public static void setParentParameter(Object cc, String fieldName, Object value)
throws NoSuchFieldException, IllegalAccessException {
Field field = cc.getClass().getSuperclass().getDeclaredField(fieldName);
field.setAccessible(true);
field.set(cc, value);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this used anywhere?

Copy link
Author

Choose a reason for hiding this comment

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

Sorry forgot to remove from the first PR, not used any more. It was used to set the private variable manufacturer before.

@kornefalk
Copy link
Author

With the given example given by @ppkarwasz , the output will be

{
 "metadata" : {
    "authors" : [
      {
        "name" : "Apache Logging Services Security Team",
        "email" : "[email protected]"
      }
    ],
    "manufacturer" : {
      "name" : "The Apache Software Foundation",
      "url" : [
        "https://apache.org",
        "https://logging.apache.org/security.html"
      ],
      "contact" : [
        {
          "name" : "Apache Logging Services Security Team",
          "email" : "[email protected]"
        }
      ]
    }
}

Parsing a property field with postal address into ZIP code, street address, city, country etc is very complicated to make it work with over 100 countries who has different ways of writing the postal information. The manufacturer section several fields witch is very hard to get it correct.

The section manufacturer.contact and authors in the metadata, has identical formats, which is little confusing.

@hboutemy hboutemy changed the title Add manufacturer information Add manufacturer information to SBOM metadata Nov 21, 2024
@hboutemy
Copy link
Contributor

hboutemy commented Nov 21, 2024

from the code update, I see now that we have both manufacturer https://cyclonedx.org/docs/1.6/json/#metadata_manufacturer
and authors https://cyclonedx.org/docs/1.6/json/#metadata_authors
with data extracted from POM organization https://maven.apache.org/ref/3.9.9/maven-model/maven.html#class_organization and developers https://maven.apache.org/ref/3.9.9/maven-model/maven.html#class_developer

but no documentation any more: I'd like to have a page on this, like https://cyclonedx.github.io/cyclonedx-maven-plugin/external-references.html
this will help users getting suprising values and discovering what they have in their effective POM (with everything they inherited, eventually not really being aware of it: I've seen that a lot with this plugin, where people complain about the plugin instead of trying to fix their data)

@hboutemy
Copy link
Contributor

reading CycloneDX doc for manufacturer and authors

The organization that created the BOM.
Manufacturer is common in BOMs created through automated processes. BOMs created through manual means may have @.authors instead.

The person(s) who created the BOM.
Authors are common in BOMs created through manual processes. BOMs created through automated means may have @.manufacturer instead.

and thinking at real world content for developers in POMs, where there is a description of many individuals, and not always accurate

perhaps we should avoid the authors part: I fear this will just add noise in real world

@ppkarwasz
Copy link
Contributor

this will help users getting suprising values and discovering what they have in their effective POM (with everything they inherited, eventually not really being aware of it: I've seen that a lot with this plugin, where people complain about the plugin instead of trying to fix their data)

This is how we discovered that all the effective <url> metadata in our POMs was bogus, because of inheritance.

and thinking at real world content for developers in POMs, where there is a description of many individuals, and not always accurate

perhaps we should avoid the authors part: I fear this will just add noise in real world

Maybe this will give developers an opportunity to clean up their POMs. For projects that are not maintained by a single person, it doesn't make sense to add anything else than a link to the maintainer team in <developers> (and it doesn't make sense to use @author Javadoc tags).

Projects with hundreds of contributors and dozens of past maintainers should probably add a single long term contact information to the <developers> section. However, to limit the impact of this PR and prevent users from sending security reports to public mailing lists and similar, I would include in $.authors only the developers with a specific <role>, e.g. "Security Team".

@hboutemy
Copy link
Contributor

hboutemy commented Nov 21, 2024

I see you are well opinionated on how to fill <developers> in parent POM: this plugin is becoming a linter and best-practices enforcer for parent POMs...

I like your proposal on <role>, but this is going even more in depth: this will at least require proper documentation in this plugin

another key finding:
like me, you are thinking by default at use case where the SBOM is generated by the project, with the plugin configured in pom.xml and bound to normal build lifecycle
I see more and more people (perhaps @kornefalk ) who are more in a case where the SBOM is done by someone else later, and the plugin purely invoked from CLI without anything in pom.xml but everything as -Dxxx=yyy: in this case, getting data from effective POM is misleading

@kornefalk
Copy link
Author

  • Move the manufacturer section from metadata to metadata/component/manufacturer due to deprecation
  • Modified the creation of manufacturer data to work with aggregation.
  • Re-fractured the code
  • Removed call to setAuthors since it is not needed

Example output:

{
  "metadata" : {
    "component" : {
      "type" : "library",
...
      "manufacturer" : {
        "name" : "The Apache Software Foundation",
        "url" : [
          "https://apache.org",
          "https://logging.apache.org/security.html"
        ],
        "contact" : [
          {
            "name" : "Apache Logging Services Security Team",
            "email" : "[email protected]"
          }
        ]
      },
...
    }
   }
}

@kornefalk
Copy link
Author

One option with the <role>, could be to have one configuration parameter, that determines which developer should be includes. Like:

   <configuration>
      <includeRoles>Security Team</includeRoles>
   <configuration>

And implement it as a List<String>, so you can specify one or more roles to include. It could be defaulted to Security Team.
If so, this new configuration parameter needs to be documented. What do you say @hboutemy ?

@hboutemy
Copy link
Contributor

hboutemy commented Nov 28, 2024

given https://cyclonedx.org/docs/1.6/json/#metadata_manufacturer says "The organization that created the BOM.", the more I think about it, the more I feel extracting data from pom.xml is going the wrong route in terms of semantics

pom.xml is the project that maintains the code: they may not be the people running the plugin to generated the BOM = what BOM manufacturer is about

not extracting data from pom.xml by default is safer: people invoking through CLI can configure with -D if they wish (or even rework data after the initial generation)

but all the data extraction from pom.xml and customization of role is complexity targetted to the wrong people

@ppkarwasz @kornefalk WDYT?
(sorry, one thing I learned over time with this plugin is that what is hard is not code but agreeing on semantics first)

@ppkarwasz
Copy link
Contributor

Yes, it seems that $.metadata.manufacturer is not the right element to fill with data from the POM file. What about $.components[*].manufacturer (https://cyclonedx.org/docs/1.6/json/#components_items_manufacturer)?

@kornefalk
Copy link
Author

The major problem for me is an upcoming new law within the European Union (EU) . The proposal is that all software must be shipped together with a SBOM file.

One of the requirement of SBOM file is:

the name, registered trade name or registered trademark of the manufacturer, and the
postal address, the email address or other digital contact as well as, where
available, the website at which the manufacturer can be contacted

European Parliament legislative resolution of 12 March 2024 on the proposal for a
regulation of the European Parliament and of the Council on horizontal cybersecurity
requirements for products with digital elements and amending Regulation (EU)
2019/1020 (COM(2022)0454 – C9-0308/2022 – 2022/0272(COD))

So therefore I need to be able to fill in the metadata -> component -> manufacturer. The components section is for used libraries, not for the actual delivered library or application.

So my first pull request that @ppkarwasz didn't like, was to be able to configure the <manufature> as a section. in the plugin configuration. And doing manual post-adding of that data taken much manual resources, which is not acceptable.

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.

3 participants