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

Unexpectedly added version property to extraIdentity #842

Closed
8R0WNI3 opened this issue Jul 12, 2024 · 5 comments · Fixed by #844 or #1026
Closed

Unexpectedly added version property to extraIdentity #842

8R0WNI3 opened this issue Jul 12, 2024 · 5 comments · Fixed by #844 or #1026
Labels
component/ocm-cli OCM Command Line Interface component/ocm-core Open Component Model Core aka. go API kind/bugfix Bug
Milestone

Comments

@8R0WNI3
Copy link
Member

8R0WNI3 commented Jul 12, 2024

What happened:
When executing ocm sign componentversions on a component descriptor which contains multiple resources which share the same name and have extraIdentity explicitly set to {}, all resources have the version property added to the extraIdentity field except the last resource of that name (which is reasonable since the last resource is unique without extraIdentity being set as soon as all other resources have their version property added to it).

What you expected to happen:
Don't silently add properties to the extraIdentity field during ocm sign command (and certainly not inconsistently). Instead, rather fail verification.

How to reproduce it (as minimally and precisely as possible):
Example component descriptor which has to be signed using ocm sign componentversions command:

component:
  componentReferences: []
  name: example.org/my-component
  provider: ACME Inc.
  repositoryContexts: []
  resources:
  - access:
      imageReference: hello-world
      type: ociArtifact
    extraIdentity: {} # must not be omitted to reproduce behaviour
    name: my-resource # must be part of the component multiple times
    relation: external
    type: ociImage
    version: 0.1.1 # must be different for the resources sharing the same name
  - access:
      imageReference: hello-world
      type: ociArtifact
    extraIdentity: {} # must not be omitted to reproduce behaviour
    name: my-resource # must be part of the component multiple times
    relation: external
    type: ociImage
    version: 0.1.2 # must be different for the resources sharing the same name
  sources: []
  version: 1.0.0
meta:
  schemaVersion: v2
signatures: []

Anything else we need to know:

Environment:

@ccwienk
Copy link
Contributor

ccwienk commented Jul 12, 2024

Instead, rather fail verification

generally +1. However, I would personally prefer the sign command to at most emit a warning (which may be escalated to an error) upon validation error. At least, please offer a flag to ignore validation errors (motivation being identical to #755 (there should always be an override option for emergencies)

@morri-son morri-son added this to the 2024-Q3 milestone Jul 15, 2024
@morri-son morri-son added component/ocm-cli OCM Command Line Interface component/ocm-core Open Component Model Core aka. go API labels Jul 15, 2024
@github-project-automation github-project-automation bot moved this from 🆕 ToDo to 🍺 Done in OCM Backlog Board Jul 30, 2024
@ocmbot ocmbot bot moved this from 🍺 Done to 🔒Closed in OCM Backlog Board Aug 9, 2024
@8R0WNI3
Copy link
Member Author

8R0WNI3 commented Oct 29, 2024

The problem can still be reproduced in case the extraIdentity property of the to-be-signed component descriptor's resources is explicitly set to an empty object {} (in contrast to omitting the property or setting it to null).

@8R0WNI3 8R0WNI3 reopened this Oct 29, 2024
@github-project-automation github-project-automation bot moved this from 🔒Closed to 📋 Next-UP in OCM Backlog Board Oct 29, 2024
@morri-son morri-son modified the milestones: 2024-Q3, 2024-Q4 Oct 29, 2024
@mandelsoft
Copy link
Contributor

mandelsoft commented Oct 29, 2024

Some months ago, there was a decision to get rid of this stangfe version defaulting for the extra identity.

So all new coding should create appropriate extraIdentity version fields if required to make the identity unique.

Unfortunately, there is a bug in the OCM library, which did this only if there is at least a non-nil extraIdentity map.

I'll fix this, and introduce this defaulting only in scenarios, where a component version is created/modified.
In any case, it MUST be avoided to do such a defaulting for existing component versions, especially if they are already signed,
because the extra identity field are part of the signature. We have missed to define the normalized CD format to always include
a complete logical extraIdentity. This cannot be changed to not invalidate existing signatures.

@mandelsoft
Copy link
Contributor

@8R0WNI3 , could you please check #1026

@morri-son morri-son moved this from 📋 Next-UP to 🔍 Review in OCM Backlog Board Nov 14, 2024
@morri-son
Copy link
Contributor

fixed with #1026

@hilmarf hilmarf linked a pull request Nov 14, 2024 that will close this issue
@jakobmoellerdev jakobmoellerdev moved this from 🔍 Review to 🍺 Done in OCM Backlog Board Nov 28, 2024
@ocmbot ocmbot bot moved this from 🍺 Done to 🔒Closed in OCM Backlog Board Dec 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/ocm-cli OCM Command Line Interface component/ocm-core Open Component Model Core aka. go API kind/bugfix Bug
Projects
Status: 🔒Closed
5 participants