guacsec / guac

GUAC aggregates software security metadata into a high fidelity graph database.
https://guac.sh
Apache License 2.0
1.25k stars 165 forks source link

How to ingest self signed dsse document into guac? (implementing a CA verifier) #304

Closed saipraveen88 closed 1 year ago

saipraveen88 commented 1 year ago

Use case: Ingest a self signed dsse envelope, which generates a knowledge graph of an artifact.

Today, looks like guac only ingests dsse documents signed by Sigstore. Any self signed dsse documents are failing to ingest at verify signature step, as guac currently supports "sigstore" as the only registered provider.

We here at Guidewire Software are currently self signing the in-toto statement while generating the signed dsse document for our CI build stages. Can you please suggest on how can we ingest a self signed dsse document into the guac?

lumjjb commented 1 year ago

Thanks for opening this up @saipraveen88 ! Yes, this is something that should be included. It looks like this warrants the creation of a new verifier and adding it to the list of verifiers to go through for validation. We'll use this issue to track this work, will rename to reflect task.

What is the format of inputs you are looking to use? Is your authority an x509 cert or GPG key? In running the ingestor would the addition of a public key be sufficient like guacone --CA=ca.pem ...?

Also - we have a design document which came from #217 that may contain a relevant section "Trust Policy" that may be of interest to you - and we'd like some feedback on that.

(side note upon revisiting that code i realized that we forgot to register the sigstore verifier which led to a failure, created #307).

lumjjb commented 1 year ago

Would it be helpful for your usage if in the meantime we could provide a quick interim --no-verify flag which will skip verification in general, so you can at least ingest the DSSEs and use them if you are only gathering from trusted sources?

saipraveen88 commented 1 year ago

Thanks, @lumjjb.

Thanks for opening this up @saipraveen88 ! Yes, this is something that should be included. It looks like this warrants the creation of a new verifier and adding it to the list of verifiers to go through for validation. We'll use this issue to track this work, will rename to reflect task.

What is the format of inputs you are looking to use? Is your authority an x509 cert or GPG key? In running the ingestor would the addition of a public key be sufficient like guacone --CA=ca.pem ...?

Also - we have a design document which came from #217 that may contain a relevant section "Trust Policy" that may be of interest to you - and we'd like some feedback on that.

(side note upon revisiting that code i realized that we forgot to register the sigstore verifier which led to a failure, created #307).

For now, we are interested in ingesting DSSE documents only. We are using RSA crypto keys for signing and generating DSSE envelopes. As you mentioned an option to provide the public key as an argument, which then can be used to return an identity could be more meaningful.

Would it be helpful for your usage if in the meantime we could provide a quick interim --no-verify flag which will skip verification in general, so you can at least ingest the DSSEs and use them if you are only gathering from trusted sources?

A --no-verify flag makes sense, but the only downside is that we would be able to generate the Identity node for the Attestation Node. please correct me if I am wrong.

pxp928 commented 1 year ago

@saipraveen88 We have a maintainer planning meeting today and I will bring this up as a priority to fix/add.

saipraveen88 commented 1 year ago

thanks @pxp928

pxp928 commented 1 year ago

@saipraveen88 PR https://github.com/guacsec/guac/pull/311 will be merged soon. Let me know if that solves your issue.

pxp928 commented 1 year ago

Once caveat currently is that the keyID needs to be the hash of the public key:

{
  "payload": "<Base64(SERIALIZED_BODY)>",
  "payloadType": "<PAYLOAD_TYPE>",
  "signatures": [{
    "keyid": "<KEYID>",
    "sig": "<Base64(SIGNATURE)>"
  }]
}

We are looking to see if that needs to be changed in sigstore or other upstream places.

pxp928 commented 1 year ago

Hey @saipraveen88, did this work for you? Let me know and we can close out this issue or if you need something else.

saipraveen88 commented 1 year ago

Hi @pxp928, sorry for the late response, we will try it out and let you know. thanks.

sasi-07 commented 1 year ago

Hey @pxp928, we tested out the change by successfully ingesting a self-signed document by passing valid --verifier-keyID & valid --verifier-keyPath as arguments to the guacone cli. However the ingestion was successful even when we passed a valid --verifier-keyID and invalid --verifier-keyPath.

pxp928 commented 1 year ago

Hello @sasi-07, thank you for the feedback. Could you provide more details on your second use case where it was a valid --verifier-keyID and invalid --verifier-keyPath? Meaning the keyID was the hash of the valid public key but the public key you passed in via --verifier-keyPath was a different key? With the valid keyID and KeyPath, did you see identity nodes appear in the graph DB?

sasi-07 commented 1 year ago

Right, we passed the hash of the valid public key as KeyID and path to a different public key in --verifier-keyPath.

pxp928 commented 1 year ago

Interesting. I will have to test that on my end. Will get back to you. Thanks you @sasi-07

sasi-07 commented 1 year ago

Also with valid keyID and path, the identity nodes were created.

pxp928 commented 1 year ago

@sasi-07 what about the invalid keypath? Identity nodes created?

sasi-07 commented 1 year ago

@pxp928 Yes even for invalid keypath identity nodes were created.

pxp928 commented 1 year ago

@sasi-07 found the issue. Now it will not create identity nodes for invalid keys.

saipraveen88 commented 1 year ago

thank you @pxp928

pxp928 commented 1 year ago

@saipraveen88 @sasi-07 good to close this issue?

pxp928 commented 1 year ago

Closing this issue. Please let us know if there are any more issues and we can open a new issue.