Skip to content
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

Requesting a nonexistent file generates a Length prefix missing error on Veracruz-Client #564

Open
gbryant-arm opened this issue Nov 11, 2022 · 3 comments
Labels
bug Something isn't working trusted-veracruz-runtime Something related to the trusted Veracruz runtime

Comments

@gbryant-arm
Copy link
Contributor

gbryant-arm commented Nov 11, 2022

Describe the bug
Requesting a nonexistent file generates a Length prefix missing error on Veracruz-Client.
This is because read_file_by_absolute_path() fails in read_file(), which ultimately results in the RM failing to send TLS data and sending an empty HTTP response.
@egrimley-arm 's current work on removing HTTP between VC-Client and RM might have an impact on that.

To Reproduce
Request a file that doesn't exist:

<client> --result /nonexisting_file=- --identity <...> --key <...>

Expected behaviour
The runtime manager should always prefix the messages it sends with the protocol buffer's length.

@gbryant-arm gbryant-arm added bug Something isn't working trusted-veracruz-runtime Something related to the trusted Veracruz runtime labels Nov 11, 2022
@egrimley-arm
Copy link
Contributor

I'm not sure that the person or program that invokes vc-client needs to know about the details of whether protocol buffers are prefixed with lengths. The main thing is that vc-client writes nothing to stdout and exits with a non-zero status code, right? It might be doing that correctly after #552 is merged, but we should check.

@gbryant-arm
Copy link
Contributor Author

I agree that the protocol buffer handling should be transparent to the invoker.
Just saying a FS-related error shouldn't propagate to the TLS layer.
Additionally, for security reasons, we should try to minimise the number of errors the RM can return.
Sure, let's wait for #552 to be merged 👍

@egrimley-arm
Copy link
Contributor

PR #584 might deal with this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working trusted-veracruz-runtime Something related to the trusted Veracruz runtime
Projects
None yet
Development

No branches or pull requests

2 participants