Skip to content

Comments

Configuration file spec#76

Merged
SteveLasker merged 10 commits intonotaryproject:mainfrom
SteveLasker:configuration
Sep 14, 2021
Merged

Configuration file spec#76
SteveLasker merged 10 commits intonotaryproject:mainfrom
SteveLasker:configuration

Conversation

@SteveLasker
Copy link
Contributor

Capturing the configuration file, at its current state

Steve Lasker and others added 6 commits June 19, 2020 17:39
Signed-off-by: Steve Lasker <stevelasker@hotmail.com>
Signed-off-by: Steve Lasker <stevelasker@hotmail.com>
Signed-off-by: Steve Lasker <stevelasker@hotmail.com>
Signed-off-by: Steve Lasker <stevelasker@hotmail.com>
Signed-off-by: Steve Lasker <stevenlasker@hotmail.com>
Signed-off-by: Steve Lasker <stevenlasker@hotmail.com>
@SteveLasker SteveLasker requested a review from a team August 27, 2021 22:19
Signed-off-by: Steve Lasker <stevenlasker@hotmail.com>
Signed-off-by: Steve Lasker <stevenlasker@hotmail.com>
Comment on lines +30 to +41
"verificationCerts": {
"certs": [
{
"name": "wabbit-networks.io",
"path": "~/./notary/keys/wabbit-networks.crt"
},
{
"name": "import.acme-rockets.io",
"path": "~/./notary/keys/import-acme-rockets.crt"
}
]
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need certs inside of verificationCerts? It is redundant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was trying to align with keys which has default nested, so it needed keys.
Suggestions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Leaving for this PR, and lets address how we want to handle verificationCerts and signingKeys
Do we want to pull default out to a root node?

Copy link
Contributor

Choose a reason for hiding this comment

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

@SteveLasker - Moving this into the notary location exposes the notation CLI to need ACLs and policise that are present with notaryV1. My recommendatiaon is to have this under .notation for now and open an issue to unify the config location with notaryv1 if that is the requirement.
/cc @gokarnm

Copy link
Contributor

Choose a reason for hiding this comment

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

  • How about naming it verification-identities and changing structure to something like this?
"verification-identities": [
    {
        "name": "wabbit-networks.io",
        "certificate-path": "~/./notary/keys/wabbit-networks.crt"
    },
    {
        "name": "import.acme-rockets.io",
        "certificate-path": "~/./notary/keys/import-acme-rockets.crt"
    }
]

If we are planning to support only referencing(using path) not inlining of certificate then we can rename certificate-path to certificate.

  • Also, are we thinking of supporting truststore and trustpolicy evaluation using this cli or its just that signature leads to a trusted certificate? If its former then we might not need verificationCerts instead we can refer trusstore+trustpolicy.

Copy link
Contributor

Choose a reason for hiding this comment

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

@SteveLasker - Moving this into the notary location exposes the notation CLI to need ACLs and policise that are present with notaryV1. My recommendatiaon is to have this under .notation for now and open an issue to unify the config location with notaryv1 if that is the requirement.
/cc @gokarnm

@sajayantony @SteveLasker would like to understand better if there is a requirement, and the motivation for unifying config with notary V1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

is a requirement, and the motivation for unifying config with notary V1.

It was more about reducing folder sprawl. I'm trying to avoid end users having to "feel the pain" of multiple folders being created as a result of using v1 and v2.
We don't have much usage of notary v1, so it's a minimal issue.
It's not the most pressing issue, so you could also argue, if most users don't have notary v1, is it really folder sprawl as they'll only have a single folder, "./notation".
I'll update to use notation and not be something to take more time for us to think about

Copy link
Contributor Author

Choose a reason for hiding this comment

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

are we thinking of supporting truststore and trustpolicy evaluation

Yes, we should incorporate truststore policy. However, suggesting we add that as we iterate the details of the trust store policy. This will document the current state, so we can release a notation cli that we can iterate with.
As we complete the truststore policy design and implementation, we can update this config spec.

- `enabled` - bool, which acts as an on/off switch for default notation behavior. --enabled false would disable all automatic validations
- `verificationCerts` - collection of name/value pairs for a collection of public certs that are used for verification. These may be replaced with a future policy configuration.
- `name` - a named reference to the certificate
- `path` - a location by which the certificate can be found by the notation cli or notation libraries
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to define whether it is an absolute path or it can be a relative path or both.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question. What do you think from a security perspective?

Copy link
Contributor

Choose a reason for hiding this comment

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

It is better if it is an absolute path.

- `path` - a location by which the certificate can be found by the notation cli or notation libraries
- `signing-keys` - a collection of name/value pairs of signing keys.
- `name` - a named reference to the key
- `path` - a location by which the key can be found by the notation cli or notation libraries
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here for absolute or / and relative path.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question. What do you think from a security perspective?

Signed-off-by: Steve Lasker <stevenlasker@hotmail.com>
@SteveLasker SteveLasker added the cli Issue or PR released to Notation CLI label Aug 30, 2021

Property | Type | Value
------ | ------ | ---
`enabled`|_bool_|on/off switch for default notation behavior. --enabled false would disable all automatic validations
Copy link
Contributor

Choose a reason for hiding this comment

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

enabled is used for docker-notation. This field is not meaningful for the notation CLI.

Copy link
Contributor

Choose a reason for hiding this comment

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

Whats the purpose of enabled field? When will customer actually use it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@shizhMSFT, this is in the current config file. Are we having separate configs for docker-notation and notation?

Comment on lines +46 to +58
"signingKeys": {
"default": "wabbit-networks.io",
"keys": [
{
"name": "wabbit-networks.io",
"path": "~/./notary/keys/wabbit-networks.key"
},
{
"name": "import.acme-rockets.io",
"path": "~/./notary/keys/import-acme-rockets.key"
}
]
},
Copy link
Contributor

Choose a reason for hiding this comment

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

  • Along with signingKey(private key) we will need signing certificate and certificate chain as signing certificate and certchain will be embedded in signature envelope.
  • Also, how about using default name for default identity? Although this works better with INI format.
"signing-identites": [
  {
      "name": "default",
      "signing-certificate": "~/./notary/keys/wabbit-networks.crt",
      "signing-certificate-chain": ""~/./notary/keys/wabbit-networks-chain.crt",
      "private-key": "~/./notary/keys/wabbit-networks.key"
  },
  {
      "name": "import-acme-rockets",
      "signing-certificate": "~/./notary/keys/import-acme-rockets.crt",
      "signing-certificate-chain": ""~/./notary/keys/import-acme-rockets-chain.crt",
      "private-key": "~/./notary/keys/import-acme-rockets.key"
  }
]

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 on requiring signing certificate in addition to private key.

Copy link
Contributor

Choose a reason for hiding this comment

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

opened issue: #89

Comment on lines +30 to +41
"verificationCerts": {
"certs": [
{
"name": "wabbit-networks.io",
"path": "~/./notary/keys/wabbit-networks.crt"
},
{
"name": "import.acme-rockets.io",
"path": "~/./notary/keys/import-acme-rockets.crt"
}
]
},
Copy link
Contributor

Choose a reason for hiding this comment

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

  • How about naming it verification-identities and changing structure to something like this?
"verification-identities": [
    {
        "name": "wabbit-networks.io",
        "certificate-path": "~/./notary/keys/wabbit-networks.crt"
    },
    {
        "name": "import.acme-rockets.io",
        "certificate-path": "~/./notary/keys/import-acme-rockets.crt"
    }
]

If we are planning to support only referencing(using path) not inlining of certificate then we can rename certificate-path to certificate.

  • Also, are we thinking of supporting truststore and trustpolicy evaluation using this cli or its just that signature leads to a trusted certificate? If its former then we might not need verificationCerts instead we can refer trusstore+trustpolicy.


Property | Type | Value
------ | ------ | ---
`enabled`|_bool_|on/off switch for default notation behavior. --enabled false would disable all automatic validations
Copy link
Contributor

Choose a reason for hiding this comment

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

Whats the purpose of enabled field? When will customer actually use it?

`key.name`|_string_|a named reference to the key
`key.path`|_string_|a location by which the key can be found by the notation cli or notation libraries
`signing-keys.default`|_string_|the signing key to be used when `notation sign` is called without `--name`
`insecureRegistries`|_array_|a list of registries that may be used without https
Copy link
Contributor

Choose a reason for hiding this comment

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

How is insecureRegistries related to notary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

notation pushes and pulls content to a registry using the oras-go libraries. insecureRegistires is a list of registries enables the flow, as an exception to the normal HTTPS required flag.
It's not just for local instances of distribution. Many IoT scenarios run non https scenarios.

Signed-off-by: Steve Lasker <stevenlasker@hotmail.com>
@SteveLasker
Copy link
Contributor Author

This reflects the current implementation of #83

Suggestion to merge this and #83, and open Issues and PRs for the proposed name changes.
This provides a baseline to test and iterate with issues on functioning bits from main.

Copy link
Contributor

@NiazFK NiazFK left a comment

Choose a reason for hiding this comment

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

LGTM with #89

@SteveLasker SteveLasker merged commit af8b2b3 into notaryproject:main Sep 14, 2021
@iamsamirzon iamsamirzon added cli Issue or PR released to Notation CLI and removed cli Issue or PR released to Notation CLI labels Sep 22, 2021
7h3-3mp7y-m4n pushed a commit to 7h3-3mp7y-m4n/notation that referenced this pull request Mar 29, 2025
* Configuration file spec
* move under the specs folder
* Incorporate feedback on table layout and naming
Signed-off-by: Steve Lasker <stevelasker@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cli Issue or PR released to Notation CLI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants