golemfactory / golem-certificate

GNU Lesser General Public License v3.0
0 stars 1 forks source link

Strict fields validation when creating/signing new document #28

Open pwalski opened 1 year ago

pwalski commented 1 year ago

What: Add additional validation of fields to not let in fields containing only a whitespace and URLs like "protocol://". Do not change how verification of existing certificate works (so it obeys schema and nothing more).

Why: 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"
  }
}

Originally posted by @pwalski in https://github.com/golemfactory/yagna/issues/2542#issuecomment-1591899854

evik42 commented 1 year ago

So how should it be decided which URLs contain a supported protocol and which does not? Because yagna will process protocol://example.com:80 as a valid protocol that means tcp://example.com:80.

UrlValidator::resolve_ips

            let protocol = match url.scheme() {
                "udp" => Protocol::Udp,
                _ => Protocol::Tcp,
            };

Yes filtering could be added, it could guide the user more. But if there is a node-descriptor, manifest or certificate that contains urls which refers to protocols that are not on the curated list, copying that into the editor will fail and the user would need to figure out what is wrong, why the editor does not allow to do things that are working fine in the system.

You could say that lets automatically convert to tcp:// but then it would need to figure out that yagna actually processes 5 protocols later (http, https, ws, wss, ftp) and would need to add the default port number if it is missing. So we would end up with reproducing the same logic from yagna in the certificate manager, which (the certificate manager) can be used without ever using yagna. So I do not see why would need to connect yagna implementation to the certificate manager.