Skip to content

Conversation

stefanberger
Copy link
Contributor

@stefanberger stefanberger commented Sep 24, 2025

Summary

Replace sigstore_proto_buf with sigstore_models. In many cases the classes of sigstore_modelscan be called with unchanged parameters, but in some cases explicit base64 encoding needs to be done.

Checklist
  • All commits are signed-off, using DCO
  • All new code has docstrings and type annotations
  • All new code is covered by tests. Aim for at least 90% coverage. CI is configured to highlight lines not covered by tests.
  • Public facing changes are paired with documentation changes
  • Release note has been added to CHANGELOG.md if needed

@stefanberger stefanberger requested review from a team as code owners September 24, 2025 20:08
@stefanberger stefanberger marked this pull request as draft September 24, 2025 20:09
@stefanberger
Copy link
Contributor Author

stefanberger commented Sep 24, 2025

I running into all kinds of issue where I am not sure whether sigstore_models is wrong or whether we need to do something different:

(please excuse the debugging output.)

$ ./scripts/tests/testrunner

>>> Running test-sign-verify
Testing 'sign/verify key'
O1
O2
104
b"0f\x021\x00\xe9\xed\xd2\x94\xdd'cIt\xccq\xc2\xdb\xe1R\xc7\xa6\x018\x01\xfa\x08K\xab\x08S\xbb\xff\xf1\xaa\xf8g\x17\x0b\xe2\x0bts^\x0bJ\xa4\xe1Q\x9bU\xc0\xbe\x021\x00\xf1\xff\xfc\x08}\x8f&\xe5|(H\x9c\xf5\xe2\xd4>\x97\xceU\x05j\x85R\x81\xe6\x14\x85`\x9c\xd2\xf1\xef\rf\xe1\xa4\x1e)\x9b\x0c=\xab\x03\x84\x90\xd1\xa4\x82"
O3
O4
O4.1
O5
Signing succeeded
read1 {'mediaType': 'application/vnd.dev.sigstore.bundle.v0.3+json', 'verificationMaterial': {'publicKey': {'hint': 'e8450dec4eb99dae995da9af1bc2cc9f76ed669ee2e744f57abba763df3e3f8e'}, 'tlogEntries': []}, 'dsseEnvelope': {'payload': 'typehttps//intotoio/Statement/v1subjectnametmpikXEWEVn5gdigestsha256a153b84d71096e80bbccd687afeb34ebd8c2022029b6adb4a477121cbf390d57predicateTypehttps//modelsigning/signature/v10predicateresourcesnamesignme1digestbc9a717db6064a32cbe9eae6fc40651dd6d36ef1757183f590eff2b1765d7ebcalgorithmsha256namesignme2digestee6e00a2632ced18bf286a3f8e3a9c08f960ffb691b0efa8508bcf2741795530algorithmsha256serializationmethodfilesallowsymlinksfalsehashtypesha256ignorepathsgitignoregitattributesgitignoregithubmodelsig', 'payloadType': 'application/vnd.in-toto+json', 'signatures': [{'sig': '0f1cItqR8KSgtsJQU1HUjRc=', 'keyid': ''}]}} /tmp/tmp.ikXEWEVn5g/model.sig
Verification failed with error: 2 validation errors for Bundle
mediaType
  Field required [type=missing, input_value={}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.11/v/missing
verificationMaterial
  Field required [type=missing, input_value={}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.11/v/missing
Error: 'verify key' failed

I am not sure why it doesn't accept the mediaType even though it clearly is in the JSON object.

Sometimes the signing works (as above), at other times it fails like this:

$ ./scripts/tests/testrunner

>>> Running test-sign-verify
Testing 'sign/verify key'
O1
O2
102
b'0d\x0201\x90\xf7\xaf;\xa5\x85s\xa5\x93\x88q\xd1\x18\x8f<\xa1-.\xd3\nB\x91\xa8\xbb\x95Y\x91,K\x9d\x18\x8c\ni+\xfb\x08\xca\xb8@\xb0N\xc07\x08\xf0\x0b\x020R\x05\xc0\xbd.\xb9\xb89\x8e!%S\xc2-?,-\xba(\x0b\xae\x04\x98m\xdfv\xd0\xa8\xd0\x90\x14V\xce\xf5S\xbbW\xc30\x87\xd0z\x1e\x1e/6J '
Signing failed with error: 1 validation error for Signature
sig
  Value error, Incorrect padding [type=value_error, input_value=b'0d\x0201\x90\xf7\xaf;\x...30\x87\xd0z\x1e\x1e/6J ', input_type=bytes]
    For further information visit https://errors.pydantic.dev/2.11/v/value_error
Error: 'sign key' failed

It's the sig parameter to Signature(sig=sig,...) that is of type bytes, but then the padding is bad... eh? The sigstore_models code looks like it's supposed to convert the bytes to a b64 encoded string:

@woodruffw , maybe you have a hint?

@stefanberger
Copy link
Contributor Author

@woodruffw With this change to _core.py I can at least eliminate the 2nd problem above:

ProtoBytes = t.Annotated[
    bytes,
    #BeforeValidator(_validate_proto_bytes),
    PlainSerializer(_serialize_proto_bytes, return_type=bytes),
]

@stefanberger
Copy link
Contributor Author

I have the impression that BeforeValidator and PlainSerializer cannot both be in Annotated ...

@stefanberger stefanberger force-pushed the move_to_sigstore_models branch 3 times, most recently from 846aa28 to 2ed924e Compare September 24, 2025 23:10
@stefanberger
Copy link
Contributor Author

I got a lot farther now but had to disable verification tests with public keys because sigstore-models does seem to deal with plain public keys the same way as protobuf did.

@stefanberger stefanberger force-pushed the move_to_sigstore_models branch 2 times, most recently from 56cba6f to af16355 Compare September 25, 2025 01:02
@stefanberger stefanberger marked this pull request as ready for review September 25, 2025 01:05
@stefanberger stefanberger force-pushed the move_to_sigstore_models branch from af16355 to 9b60a55 Compare September 25, 2025 12:22
certificates=chain
)
),
tlog_entries=[],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@woodruffw Should this have to be here or is this a bug in the library?

@mihaimaruseac
Copy link
Collaborator

I got a lot farther now but had to disable verification tests with public keys because sigstore-models does seem to deal with plain public keys the same way as protobuf did.

Unfortunately this would break verifying models from the NVidia hub in that case, so we'd still need to patch to support that use case.

@stefanberger
Copy link
Contributor Author

I got a lot farther now but had to disable verification tests with public keys because sigstore-models does seem to deal with plain public keys the same way as protobuf did.

Unfortunately this would break verifying models from the NVidia hub in that case, so we'd still need to patch to support that use case.

We do have test cases verifying older signatures and they do pass now...
Also, do you have a link to a model on the Nvidia hub? I seem to not have looked at the right place to find something to git clone and verify.

@mihaimaruseac
Copy link
Collaborator

I got a lot farther now but had to disable verification tests with public keys because sigstore-models does seem to deal with plain public keys the same way as protobuf did.

Unfortunately this would break verifying models from the NVidia hub in that case, so we'd still need to patch to support that use case.

We do have test cases verifying older signatures and they do pass now... Also, do you have a link to a model on the Nvidia hub? I seem to not have looked at the right place to find something to git clone and verify.

In that case, let's merge this too and tentatively do a release and ask NVIDIA to also test.

One sample model: https://catalog.ngc.nvidia.com/orgs/nvidia/teams/tao/models/peoplenet?version=pruned_quantized_decrypted_v2.3.4

There are also instructions on https://developer.nvidia.com/blog/bringing-verifiable-trust-to-ai-models-model-signing-in-ngc/

In many cases the classes of sigstore_models can be called with unchanged
parameters. However, in some cases explicit base64 encoding needs to be
done.

Signed-off-by: Stefan Berger <[email protected]>
@stefanberger stefanberger force-pushed the move_to_sigstore_models branch from 9b60a55 to 9af5f42 Compare September 26, 2025 13:00
@stefanberger stefanberger marked this pull request as draft September 26, 2025 13:00
@stefanberger
Copy link
Contributor Author

stefanberger commented Sep 26, 2025

In that case, let's merge this too and tentatively do a release and ask NVIDIA to also test.

@mihaimaruseac I converted it to a draft for now, hoping to hear back from @woodruffw soon on the issues that I have seen with the library, such as the necessary additional parameter (tlog_entries) and the fact that it seems to want to do base64 encoding on its own but I had to do it explicitly ( I don't want this to work in sigstore-models at some point and we end up doing double-base64 encoding). The point is, we may need to possibly wait for sigstore-models>0.5.0 ...

@stefanberger
Copy link
Contributor Author

@mihaimaruseac
Copy link
Collaborator

Yeah, the signature is not displayed there, you download it with the CLI command

@stefanberger
Copy link
Contributor Author

Yeah, the signature is not displayed there, you download it with the CLI command

I must be doing something wrong. With the command ngc registry model download-version "nvidia/tao/peoplenet:pruned_quantized_decrypted_v2.3.4" copied to my clipboard from the (assumed) signed model at https://catalog.ngc.nvidia.com/orgs/nvidia/teams/tao/models/peoplenet/files?version=pruned_quantized_decrypted_v2.3.4 I get exactly the files that it shows on the webpage and none of them seems to be a signature:

$ ls -l
total 8612
-rw-------. 1 stefanb stefanb      16 Sep 29 08:16 labels.txt
-rw-------. 1 stefanb stefanb     207 Sep 29 08:16 nvinfer_config.txt
-rw-------. 1 stefanb stefanb 8788899 Sep 29 08:16 resnet34_peoplenet_int8.onnx
-rw-------. 1 stefanb stefanb   16678 Sep 29 08:16 resnet34_peoplenet_int8.txt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants