trustification / trustify

Apache License 2.0
8 stars 15 forks source link

GET /api/v1/advisory/urn:uuid:abcd/download does not work #394

Closed carlosthe19916 closed 4 weeks ago

carlosthe19916 commented 1 month ago

Response from the backend

{
    "error": "Unsupported hash algorithm",
    "message": "Unsupported algorithm urn:uuid"
}
ctron commented 1 month ago

Ok, I think that's fundamentally not possible that way.

The "key"/"id" in the store system is the digest (sha256). There's no alternative mapping. The idea was to have an opaque key, which internally is the sha256 digest for the filesystem, but could also be something different. But that was never documented or reflected by the argument names. Sorry for that.

So passing in the Id can never work. The only implementation can be, as it is now. Which is sha256, failing otherwise.

If we want to download by ID, we need to add a lookup first. Which would go through the database. I do believe that's the right approach anyway. Because for one, that ensures the endpoint actually returns an advisory (right now it could return even a cat video). And second, we might need to do some ACL checking in the future.

carlosthe19916 commented 4 weeks ago

This is the current response I get when I fetch a single Advisory

{
    "uuid": "urn:uuid:b29cd30f-ac8a-4770-b675-5be7db6610ca", // primary universal key of an advisory
    "identifier": "CVE-1999-0001",
    "hashes": [
        "sha256:4c8ca7269dca3a736f75aebf9d80a99fd24e2b17be0eaa7bc852e04631f88f73"
    ],
    "issuer": {
        "id": 2,
        "name": "CVE® (MITRE Corporation",
        "cpe_key": null,
        "website": null
    },
    "published": "2000-02-04T05:00:00Z",
    "modified": "2005-12-17T00:00:00Z",
    "title": null,
    "vulnerabilities": [
        {
            "identifier": "CVE-1999-0001",
            "published": "2000-02-04T05:00:00Z",
            "modified": "2005-12-17T00:00:00Z",
            "non_normative": {
                "non_normative": true,
                "identifier": "CVE-1999-0001",
                "released": "2000-02-04T05:00:00Z"
            },
            "severity": "none",
            "score": 0.0,
            "cvss3_scores": [],
            "assertions": {}
        }
    ]
}

Based on the long discussion we all had at https://github.com/trustification/trustify/issues/377 I thought the uuid field of an advisory was going to be the universal way of referring to an advisory so I was assuming I could use that one to download the advisory file itself with:

/api/v1/advisory/{key}/download

Where key is the uuid field of an advisory.

I admit I ignore the technical difficulties or constraints of this requirement, but I also believe we have to think on a solution that allow us to download the source file of an advisory based on its uuid field.

ctron commented 4 weeks ago

And that assumption makes sense! I think we still haven't completely figured out ids.

I am just trying to come up with a test, and uploading an advisory returns an "id" as well. But it is the ID from the document content (e.g. CVE-1234). Which you can't use at all as an id.

bobmcwhirter commented 4 weeks ago

Yah a lookup of the key to find internal UUID to fetch from storage makes sense to me. and would support multiple keys for same doc.