Skip to content

COCOS-153 - Add host-data option #163

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jul 8, 2024

Conversation

danko-miladinovic
Copy link
Contributor

@danko-miladinovic danko-miladinovic commented Jun 28, 2024

What type of PR is this?

This is a feature because it adds:

  • QEMU host-data option to the launching of CVMs.

This PR also renames the platform_info.json into backend_info.json.

What does this do?

This PR enables the Manager to launch CVMs with the host-data filed of the attestation report populated with the computation configuration of the Agent. This way, the Agent can check what is being sent to him by the Manager.
This PR also renames the platform_info.json into backend_info.json.

Which issue(s) does this PR fix/relate to?

Have you included tests for your changes?

The manual README.md has been changed to reflect this new feature.

Did you document any new/modified feature?

Documentation will be updated in a separate PR.

Notes

@danko-miladinovic danko-miladinovic marked this pull request as ready for review July 1, 2024 12:02
Comment on lines 112 to 117
jsonData, err := json.Marshal(ac)
if err != nil {
ms.publishEvent("vm-provision", c.Id, "failed", json.RawMessage{})
return "", errors.Wrap(ErrFailedToMarshalJSON, err)
}
computationHash := sha3.Sum256(jsonData)
Copy link
Contributor

Choose a reason for hiding this comment

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

move to a function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


type AttestationConfiguration struct {
SNPPolicy *check.Policy `json:"snp_policy,omitempty"`
RootOFTrust *check.RootOfTrust `json:"root_of_trust,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

RootOfTrust

Copy link
Contributor Author

@danko-miladinovic danko-miladinovic Jul 1, 2024

Choose a reason for hiding this comment

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

Done, my mistake. Thanks.

@@ -156,3 +170,12 @@ func (ms *managerService) publishEvent(event, cmpID, status string, details json
},
}
}

func getComputationHash(ac *agent.Computation) ([32]byte, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Rename to computationHash and use like ch := computationHash(ac). If there is no need to pass the pointer param here (i.e. we do not change the computation from this function), pass by value instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. The computation is passed by value now.

Comment on lines 65 to 86
measurement, err := base64.StdEncoding.DecodeString(args[0])
if err != nil {
log.Fatalf("Error could not decode base64: %v", err)
}

if len(measurement) != measurementLength {
log.Fatalf("Measurement must be 48 bytes in length")
}
attestationConfiguration := AttestationConfiguration{}

backendInfo, err := os.OpenFile(args[1], os.O_RDWR, filePermision)
if err != nil {
log.Fatalf("Error opening the backend information file: %v", err)
}
defer backendInfo.Close()

decoder := json.NewDecoder(backendInfo)
err = decoder.Decode(&attestationConfiguration)
if err != nil {
log.Fatalf("Error decoding the backend information file: %v", err)
}

attestationConfiguration.SNPPolicy.Measurement = measurement
if err = backendInfo.Truncate(0); err != nil {
log.Fatalf("Error could not truncate backend information JSON file: %v", err)
}

fileJson, err := json.MarshalIndent(attestationConfiguration, "", " ")
if err != nil {
log.Fatalf("Error marshaling the backend information JSON: %v", err)
}
if err = os.WriteFile(backendInfo.Name(), fileJson, filePermision); err != nil {
log.Fatalf("Error writing into backend information JSON file: %v", err)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to simplify with something like:

Suggested change
measurement, err := base64.StdEncoding.DecodeString(args[0])
if err != nil {
log.Fatalf("Error could not decode base64: %v", err)
}
if len(measurement) != measurementLength {
log.Fatalf("Measurement must be 48 bytes in length")
}
attestationConfiguration := AttestationConfiguration{}
backendInfo, err := os.OpenFile(args[1], os.O_RDWR, filePermision)
if err != nil {
log.Fatalf("Error opening the backend information file: %v", err)
}
defer backendInfo.Close()
decoder := json.NewDecoder(backendInfo)
err = decoder.Decode(&attestationConfiguration)
if err != nil {
log.Fatalf("Error decoding the backend information file: %v", err)
}
attestationConfiguration.SNPPolicy.Measurement = measurement
if err = backendInfo.Truncate(0); err != nil {
log.Fatalf("Error could not truncate backend information JSON file: %v", err)
}
fileJson, err := json.MarshalIndent(attestationConfiguration, "", " ")
if err != nil {
log.Fatalf("Error marshaling the backend information JSON: %v", err)
}
if err = os.WriteFile(backendInfo.Name(), fileJson, filePermision); err != nil {
log.Fatalf("Error writing into backend information JSON file: %v", err)
}
measurement, err := base64.StdEncoding.DecodeString(args[0])
if err != nil {
log.Fatalf("Error could not decode base64: %v", err)
}
if len(measurement) != measurementLength {
log.Fatalf("Measurement must be 48 bytes in length")
}
ac := AttestationConfiguration{}
bi, err := os.ReadFile(args[1])
if err != nil {
log.Fatalf("Failed to read the config file: %v", err)
}
if err := json.Unmarshal(bi, &ac); err != nil {
log.Fatalf("Failed to unmarshal JSON data: %v", err)
}
ac.SNPPolicy.Measurement = measurement
bi, err = json.MarshalIndent(ac, "", "")
if err != nil {
log.Fatalf("Failed to marshal JSON configuration: %v", err)
}
if err := os.WriteFile(args[1], bi, filePermission); err != nil {
log.Fatalf("Failed to write config to the file: %v", err)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

)

const (
filePermision = 0o755
Copy link
Contributor

Choose a reason for hiding this comment

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

Fix typo permission. Also, consider completely removing it and using predefined modes from the fs like fs.<Whatever_mode_we_need_here>.

}
}

func (cli *CLI) NewAddMeasurementCmd() *cobra.Command {
Copy link
Contributor

Choose a reason for hiding this comment

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

There are a lot of repetitions, please extract to an unexported function.

Copy link
Contributor Author

@danko-miladinovic danko-miladinovic Jul 5, 2024

Choose a reason for hiding this comment

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

Changed. I have written a single function that alters the backend info file (adds the measurement and host data) and takes the fieldType argument so that the function knows if it should change the measurement field or host data field.

)

const (
// 0o744 file permission gives RWX permission to the user and only the R permission to others
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// 0o744 file permission gives RWX permission to the user and only the R permission to others
// 0o744 file permission gives RWX permission to the user and only the R permission to others.

case measurementField:
ac.SNPPolicy.Measurement = data
case hostDataField:
ac.SNPPolicy.HostData = data
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a default case that returns an error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@dborovcanin dborovcanin merged commit 006897a into ultravioletrs:main Jul 8, 2024
1 check passed
@danko-miladinovic danko-miladinovic deleted the host_data branch December 3, 2024 13:44
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.

Feature: Add host-data QEMU option to CVM boot up
3 participants