Skip to content

Conversation

@jcabrero
Copy link
Member

@jcabrero jcabrero commented Apr 11, 2025

I know this PR touches way too many files, but it wasn’t possible to break it into smaller PRs while keeping everything functional.

This PR introduces support for attestation verification using the official nvtrust library instead of the default Azure library.

To avoid issues caused by the outdated dependencies of nv-trust-attestation-sdk, we have modularized the attestation logic into a separate, standalone package that runs inside a Docker container. nilai-api now interacts with this container via an internal API, ensuring a cleaner and more maintainable integration.

To make it easier to review, here’s what you need to know:
1 .We have removed packages/verifier. This was a copy of the source from the Azure Confidential GPU library. Since the source was removed, that’s why there are so many lines of code deleted.
2. There is a new directory, nilai-attestation, which includes all the code required for attestation. It is a FastAPI server with a single endpoint at /attestation/report. It is intentionally kept completely separate from the rest of the nilai code to avoid any dependency conflicts. It is meant to run in the attestation: container in the Docker Compose setup and has its own attestation.Dockerfile.
3. Now nilai-api simply forwards attestations to the attestation container. It no longer needs to handle any verification itself, so it no longer has any GPU-related dependencies. Thus all the changes to nilai_api/state.py (which was in charge of the attestations before), and now there is nilai_api/attestation/__init__.py which forwards a GET request.
4. There are other QoL improvements, such as removing the two port bindings from the nilai-api gunicorn.conf.

@jcabrero jcabrero force-pushed the feat/improved_attestation_verification branch from 42512e3 to 1742e2e Compare April 11, 2025 15:21
@jcabrero jcabrero requested a review from lumasepa April 21, 2025 08:32
@jcabrero jcabrero changed the title Feat/improved attestation verification feat: Improved attestation verification Apr 21, 2025
Comment on lines +12 to +15
@lru_cache(maxsize=1)
def load_sev_library() -> bool:
"""Load the SEV library"""
return sev.init()
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems a bit hacky. I understand you are trying to load it only once. Have you tried to initialize it globally on import? It should be called only once. And you are not checking if the init failed swallowing the error and delaying it to the execution of the attestation.

Suggested change
@lru_cache(maxsize=1)
def load_sev_library() -> bool:
"""Load the SEV library"""
return sev.init()
if not sev.init():
raise Exception("Failed to load SEV")

Copy link
Member Author

Choose a reason for hiding this comment

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

At the moment, I aim at allowing nilAI to run on SEV and non-SEV hosts. If /dev/sev-guest is not available, the idea is that it continues to work, though with fake attestations. That's why we need to make sure an exception is not raised here.

@jcabrero jcabrero force-pushed the feat/improved_attestation_verification branch from a7d4d0c to c5c0c6a Compare May 9, 2025 13:44
@jcabrero jcabrero force-pushed the feat/improved_attestation_verification branch from c5c0c6a to f5fe0a6 Compare May 9, 2025 14:01
@jcabrero jcabrero self-assigned this May 9, 2025
@jcabrero jcabrero merged commit fa4e166 into main May 9, 2025
4 checks passed
@jcabrero jcabrero deleted the feat/improved_attestation_verification branch May 21, 2025 07:55
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.

3 participants