Testing: extended tests in the certificates helper tool

[kamirr]

Have another person (not author / reviewer) look at the code of the helper tool and try to find new ways to break it.

[pwalski]

1. Help needs comments:

golem-certificate-cli --help
Usage: golem-certificate-cli <COMMAND>

Commands:
  create-key-pair
  fingerprint
  self-sign-certificate
  sign
  verify
  ui
  help                   Print this message or the help of the given subcommand(s)

Options:
  -h, --help  Print help

1.1 What fingerprint do?

golem-certificate-cli fingerprint --help
Usage: golem-certificate-cli fingerprint <INPUT_FILE_PATH>

Arguments:
  <INPUT_FILE_PATH>

User will expect it calculates checksum of given file, but it expects something different:

golem-certificate-cli fingerprint README.md
Error: expected value at line 1 column 1

?!

[pwalski]

2. golem-certificate-cli self-sign-certificate golem.cert.not_expired.json golem.key overwrites file when golem.cert.not_expired.signed.json already exists. Also it is not obvious that golem.cert.not_expired.signed.json is an output.

[pwalski]

3. Some time ago I reported issue with ability to sign expired certificate, and use it issue another certificate
   It is possible to sign expired cert and use it as an issuer golem-certificate#23

[pwalski]

4. golem-certificate-cli ui node descriptor sign wizard expects me to select some existing file to overwrite.
   It does not allow me to type file name I want it to create.
   I realized I need to use Tab to jump to file editor field...

[pwalski]

5. golem-certificate-cli ui panics when it needs to scroll. Happens on tmux and on native terminal. Both on zsh and bash. Works only when window is high enough so scrolling is not necessary.

tmux.2023-06-14.21-20-35.mp4

[pwalski]

6. When creating new certificate I placed private key as a pub key and when I went into signing phase it did not let me to Sign (which is expected), but it did not let me to Cancel (which was not expected). It does not let me out from this view when its field values are invalid.

[pwalski]

7. Showing error message Missing signing key or certificate is just lazy. You know which one is missing. Also it is misleading. Value None does not indicate this field value is incorrect

.

[pwalski]

8. Nitpicking. In previous view I did NOT select sign certificate permission. This view allows me to select self-signed, and when I try to do so it complains that Certificate signing is not permitted. Maybe it should not let me select self-signed in such case?

[pwalski]

9. It is an issue with schema in GAP-25 but as a result helper allows to create certificate with fields containing only whitespace, url like protocol:// (which is validated as an URI) and expired validity period valid for 0 seconds.

{
  "$schema": "https://golem.network/schemas/v1/certificate.schema.json",
  "certificate": {
    "keyUsage": [
      "signCertificate"
    ],
    "permissions": {
      "outbound": {
        "urls": [
          "protocol://"
        ]
      }
    },
    "publicKey": {
      "algorithm": "EdDSA",
      "key": "ea88cfd72621cc9f1b5f9652e7aaa609f99e56e5a157f434fa2a931c3826bbd4",
      "parameters": {
        "scheme": "Ed25519"
      }
    },
    "subject": {
      "displayName": " ",
      "contact": {
        "email": " "
      }
    },
    "validityPeriod": {
      "notBefore": "2003-06-14T00:00:00Z",
      "notAfter": "2003-06-14T00:00:00Z"
    }
  },
  "signature": {
    "algorithm": {
      "hash": "sha512",
      "encryption": "EdDSA"
    },
    "value": "593ea9241c5a48815d424d7a1aa62a04cdb44dec0c16575fa7e2958973b158692936167520d2bd6192845cafe8b2c0936e08d3325dd94f306f9df4983cffe202",
    "signer": "self"
  }
}

Helper should at least trim values to not sign docs with 'empty' fields.

[pwalski]

EDIT
nvm

[evik42]

1.1 What fingerprint do?

golem-certificate-cli fingerprint --help
Usage: golem-certificate-cli fingerprint <INPUT_FILE_PATH>

Arguments:
  <INPUT_FILE_PATH>

User will expect it calculates checksum of given file, but it expects something different:

golem-certificate-cli fingerprint README.md
Error: expected value at line 1 column 1

?!

I generates the fingerprint of the signed part of a certificate or node-descriptor. Used it for testing, you can make it hidden as it has not much use for general users.

[evik42]

5. golem-certificate-cli ui panics when it needs to scroll. Happens on tmux and on native terminal. Both on zsh and bash. Works only when window is high enough so scrolling is not necessary.

tmux.2023-06-14.21-20-35.mp4

Yeah this happens in the certificate/node-descriptor editor because as of now those cannot scroll and the cursor is out of screen.

[evik42]

9. It is an issue with schema in GAP-25 but as a result helper allows to create certificate with fields containing only whitespace, url like protocol:// (which is validated as an URI) and expired validity period valid for 0 seconds.

{
  "$schema": "https://golem.network/schemas/v1/certificate.schema.json",
  "certificate": {
    "keyUsage": [
      "signCertificate"
    ],
    "permissions": {
      "outbound": {
        "urls": [
          "protocol://"
        ]
      }
    },
    "publicKey": {
      "algorithm": "EdDSA",
      "key": "ea88cfd72621cc9f1b5f9652e7aaa609f99e56e5a157f434fa2a931c3826bbd4",
      "parameters": {
        "scheme": "Ed25519"
      }
    },
    "subject": {
      "displayName": " ",
      "contact": {
        "email": " "
      }
    },
    "validityPeriod": {
      "notBefore": "2003-06-14T00:00:00Z",
      "notAfter": "2003-06-14T00:00:00Z"
    }
  },
  "signature": {
    "algorithm": {
      "hash": "sha512",
      "encryption": "EdDSA"
    },
    "value": "593ea9241c5a48815d424d7a1aa62a04cdb44dec0c16575fa7e2958973b158692936167520d2bd6192845cafe8b2c0936e08d3325dd94f306f9df4983cffe202",
    "signer": "self"
  }
}

Helper should at least trim values to not sign docs with 'empty' fields.

Unfortunately JSON schema cannot define logical connections between properties and the format property is optionally supported. Even x509 spec does not specify that the notAfter timestamp would need to be after the notBefore timstamp.

So these rely on common sense of the user.

For protocol, this is exactly the case with manifests, anything that can be parsed into an URL is accepted, the GAP does not restrict the URL format either. When reading up the URLs the validator will ignore unknown protocols, or other malformed urls that it cannot resolve to IPs. (The examples in the GAP even contain path parts which is completely ignored by the IP filtering.)

[evik42]

Great findings! I think 5 and 6 are something that should be addressed sooner rather than later. The others are nice to haves.

I think these should go to a 'known bugs' list and not block merging.

[evik42]

6. When creating new certificate I placed private key as a pub key and when I went into signing phase it did not let me to Sign (which is expected), but it did not let me to Cancel (which was not expected). It does not let me out from this view when its field values are invalid.

I think Esc works.

[pwalski]

I think Esc works.

Yes, it does. Still I created an issue for this Cancel button problem golemfactory/golem-certificate#27

[pwalski]

Unfortunately JSON schema cannot define logical connections between properties and the format property is optionally supported. Even x509 spec does not specify that the notAfter timestamp would need to be after the notBefore timstamp.

Yes @evik42 , but there is nothing stopping us from trimming fields when generating/signing new certificate.
It will require some work, but I think we could have a different validation when we verify document and a different one when we generate/sign it.
When verifying document we should check it only against schema.
When signing/generating a new document we could e.g. trim all fields and then let it fail on schema verification because of empty fields. Alternatively we could have some additional validation before checking against schema?

For protocol, this is exactly the case with manifests, anything that can be parsed into an URL is accepted, the GAP does not restrict the URL format either. When reading up the URLs the validator will ignore unknown protocols, or other malformed urls that it cannot resolve to IPs. (The examples in the GAP even contain path parts which is completely ignored by the IP filtering.)

Same case. Maybe we should pass protocol:// when we verify document against schema, but maybe we should not allow user to add such URI when creating/signing new certificate in golem-certificate-cli ?