repository-service-tuf/repository-service-tuf-api

Task: add a new `PATCH /api/v1/metadata/sign` endpoint

MVrachev opened this issue · 7 comments

MVrachev commented

What is the task about?

Add a new PATCH /api/v1/metadata/sign endpoint.
This endpoint will be used to override an existing Distributed Asynchronous Process (DAS) no matter if it's for bootstrap or metadata update.
The payload should include a new root (or possibly other) metadata that is not yet fully signed.
For example:

"metadata": {
    "root": {
      "signatures": [
        {
          "keyid": "1cebe343e35f0213f6136758e6c3a8f8e1f9eeb7e47a07d5cb336462ed31dcb7",
          "sig": "c471d9376651ed68bf5b0ce61bd21ba0bef7c38f293f8a4cfb3d56934e1c2f9106d58a9d57fafdd559d1e66e2581c21755b1ef5a6cdef59c40fcdc7bb158c80b"
        },
        {
          "keyid": "7feb22ed6a1139913ebdbfac85513890814e661d94084212972ba0c0a5c8ce27",
          "sig": "5558cdf391cc1feaeacf2a25e26b2042e50d286296c2a0c4bc3421788e8e11465b6956c3dc0e7ada29672e3740fa003b518e9d9c2a8411b5b279be94a40f750f"
        }
      ],
      "signed": {
        "_type": "root",
        "consistent_snapshot": true,
        "expires": "2024-04-17T08:51:40Z",
        "keys": {
          "1cebe343e35f0213f6136758e6c3a8f8e1f9eeb7e47a07d5cb336462ed31dcb7": {
            "keytype": "ed25519",
            "keyval": {
              "public": "ad1709b3cb419b99c5cd7427d6411522e5a93aec6767453e91af921a73d22a3c"
            },
            "scheme": "ed25519"
          },
          "7feb22ed6a1139913ebdbfac85513890814e661d94084212972ba0c0a5c8ce27": {
            "keytype": "ed25519",
            "keyval": {
              "public": "664d366b6838648405e1e18e9ef5b4e326713b0c0338935e2f715b9f02a05313"
            },
            "scheme": "ed25519"
          },
          "800dfb5a1982b82b7893e58035e19f414f553fc08cbb1130cfbae302a7b7fee5": {
            "keytype": "ed25519",
            "keyval": {
              "public": "7098f769f6ab8502b50f3b58686b8a042d5d3bb75d8b3a48a2fcbc15a0223501"
            },
            "scheme": "ed25519"
          },
          "f7a6872f297634219a80141caa2ec9ae8802098b07b67963272603e36cc19fd8": {
            "keytype": "ed25519",
            "keyval": {
              "public": "9fe7ddccb75b977a041424a1fdc142e01be4abab918dc4c611fbfe4a3360a9a8"
            },
            "scheme": "ed25519"
          }
        },
        "roles": {
          "root": {
            "keyids": [
              "1cebe343e35f0213f6136758e6c3a8f8e1f9eeb7e47a07d5cb336462ed31dcb7",
              "800dfb5a1982b82b7893e58035e19f414f553fc08cbb1130cfbae302a7b7fee5",
              "7feb22ed6a1139913ebdbfac85513890814e661d94084212972ba0c0a5c8ce27"
            ],
            "threshold": 1
          },
          "snapshot": {
            "keyids": [
              "f7a6872f297634219a80141caa2ec9ae8802098b07b67963272603e36cc19fd8"
            ],
            "threshold": 1
          },
          "targets": {
            "keyids": [
              "f7a6872f297634219a80141caa2ec9ae8802098b07b67963272603e36cc19fd8"
            ],
            "threshold": 1
          },
          "timestamp": {
            "keyids": [
              "f7a6872f297634219a80141caa2ec9ae8802098b07b67963272603e36cc19fd8"
            ],
            "threshold": 1
          }
        },
        "spec_version": "1.0.30",
        "version": 2
      }
    }
  }

I imagine this endpoint will return failure when:

  • there is no SIGNING_ROLE config variable set for the given metadata role

Also, I imagine that if a user calls POST /api/v1/bootstrap when there is an existing DAS process for bootstrap (BOOTSTRAP has a value of signing-<task id>) then the POST /api/v1/bootstrap endpoint will return failure.
The user must be allowed to override the existing DAS process only through this new PATCH /api/v1/metadata/sign endpoint.

Finally, I imagine that there will be a new endpoint defined called update_metadata.

References

repository-service-tuf/repository-service-tuf#327 (comment)

Code of Conduct

  • I agree to follow this project's Code of Conduct
kairoaraujo commented

Also, I imagine that if a user calls POST /api/v1/bootstrap when there is an existing DAS process for bootstrap (BOOTSTRAP has a value of signing-<task id>) then the POST /api/v1/bootstrap endpoint will return failure. The user must be allowed to override the existing DAS process only through this new PATCH /api/v1/metadata/sign endpoint.

I noticed that it was missing this protection, so I filed another issue for it and updated the umbrella

Finally, I imagine that there will be a new endpoint defined called update_metadata.

I didn't get the functionality of this. To update what?
If there is so, why an endpoint update_metadata if we have the /api/v1/metadata and we can have the PUT method to /api/v1/metadata`?

The POST /api/v1/metadata intends to submit a new version of the metadata resource. What would be the PUT /api/v1/metadata?

kairoaraujo commented

Yesterday, there was a discussion about the PATCH /api/v1/metadata and PATCH /api/v1/bootstrap, which I don't agree and I leave here my views (tl;dr) why. I'm in favor of doing a PATCH /api/v1/metadata/sign

Using PATCH /api/v1/metadata/sign you cover if it is a pending bootstrap and a metadata update.

From my experience, using the PATCH /api/v1/metadata and PATCH /api/v1/bootstrap we will be using methods that are specific for existing metadata and bootstrap resources. If in the future there is a use case for those methods will bring us to create parameters in the request or create a new version in the endpoint.

Another thing is about, how I cancel/delete a sign. The PATCH solves when I want to redo it. But what if I want to stop it? I think we will still need a DELETE /api/v1/metadata/sign. We could use PATCH /api/v1/metadata/sign with empty metadata but doesn't sound ideal IMO.

lukpueh commented

we will still need a DELETE /api/v1/metadata/sign.

I suggest we start with DELETE before we implement PATCH. PATCH would then only be a shortcut for
DELETE .../SIGN + POST .../[BOOTSTRAP | METADATA], which is nice to have but not immediately necessary.

IIRC, the feature request started with DELETE and we only came up with PATCH, because we wanted to authenticate
the request with a metadata signature, to not only rely on the API auth token.

The ideas we had were all not that useful:

  • send along a signature over the pending metadata (existing signatures can easily be replayed)
  • require the signature from a key that has not yet signed (unnecessarily limits the keys)

With PATCH we assumed that the new metadata will have changed and the included signature(s) could not be replayed. Something, we'd actually need to check. Should we fail if the new metadata has not changed?

IMO this all sounds hacky. If we want to authenticate the request using tuf keys, and can't reliably use signatures over metadata, we should maybe use some standard public key authentication method (e.g. rfc4252).

I still think the additional authentication is not necessary. As we said, at worst an attacker, who has compromised the API token, can DOS the signing event, which should be easily and quickly detectable. The important thing is that they cannot issue new trusted metadata. Which they can't without additionally compromising a quorum of tuf keys.

TLDR: I suggest to only add DELETE /api/v1/metadata/sign for now, and to do so without additional authentication besides API token.

MVrachev commented

With PATCH we assumed that the new metadata will have changed and the included signature(s) could not be replayed. Something, we'd actually need to check. Should we fail if the new metadata has not changed?

I think that's reasonable and can easily be checked. We added __eq__ method to all python-tuf API classes.
I find it logical to fail with a message that Metadata cannot be patched - new metadata same as the old one or something like that.

IMO this all sounds hacky. If we want to authenticate the request using tuf keys, and can't reliably use signatures over metadata, we should maybe use some standard public key authentication method (e.g. rfc4252).

As I understand from rfc4252 the idea is that we have a public word/bytes that should be signed and we can verify that the signature is coming from the private key we expect?
If that's the case this seems reasonable to me.

TLDR: I suggest to only add DELETE /api/v1/metadata/sign for now, and to do so without additional authentication besides API token.

I agree it sounds more important to have a delete option before having a PATCH one as you want to have a way to stop a DAS process at all even if you are not sure what will be the new metadata.
Do you think we can try the above suggestion and implement an authentication mechanism by requiring signing with a private key of a public word and then verifying this signature?

  • 👍1
lukpueh commented

Do you think we can try the above suggestion and implement an authentication mechanism by requiring signing with a private key of a public word and then verifying this signature?

I think it adds unnecessary complexity. The attacker can't really get anything out of DOSing the signing event. It will be immediately detected by the other signers, which can then revoke the API token.

MVrachev commented

Okay, will implement first the DELETE /api/v1/metadata/sign endpoint, and then this one can be implemented as well.

The initial version of the DELETE /api/v1/metadata/sign endpoint won't include the discussed checks for signatures and private keys, instead, it will initially relly on the API token.
If there is a good incentive we can add the additional checks discussed above later.

MVrachev commented

DELETE /api/v1/metadata/sign endpoint is implemented and together with the community, we agreed that this feature is not needed for now.