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.

Sign up for GitHub

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

Jump to bottom

Unary gRPC transport template class #4970

Merged
merged 3 commits into from
Apr 2, 2024
Merged

Unary gRPC transport template class #4970

merged 3 commits into from
Apr 2, 2024

Conversation

pmcgrath17
Copy link
Contributor

@pmcgrath17 pmcgrath17 commented Apr 1, 2024 •
edited
Loading

Implementing unary transport class template for future Oak clients that use the unary interface.

This class allows for clients to supply any gRPC service name, but assumes the service has an Invoke and GetEndorsedEvidence gRPC methods with the same gRPC method signature as in https://github.com/project-oak/oak/blob/main/proto/session/service_unary.proto

@pmcgrath17 pmcgrath17 requested a review from ipetr0v April 1, 2024 19:32
@pmcgrath17
Copy link
Contributor Author

@tiziano88 fyi

ipetr0v
ipetr0v approved these changes Apr 2, 2024
View reviewed changes
cc/transport/grpc_unary_transport.h
// stubby Oak service that use a gRPC interface consistent with
// oak/proto/session/service_unary.proto.
template <typename OakBackendStub>
class GrpcUnaryTransport : public ::oak::transport::TransportWrapper {
Copy link
Contributor

Choose a reason for hiding this comment

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

Will OakBackendStub be used for an internal channel implementation and not only gRPC ?

Then this class probably could just be called UnaryTransport (since it won't only be used for gRPC). And if it is only for gRPC, then discard this comment.

Copy link
Contributor

Choose a reason for hiding this comment

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

The corresponding streaming version could also updated to become a template and renamed to StreamingTransport (but probably in a separate PR)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So you are correct in that OakBackendStub could be used for the internal channel implementation as well, however calls to the server differ between the internal version and gRPC. If you wanted a single unary class, in the future maybe we could have an option in the constructor for clients to specify whether they want to use gRPC or a different form of communication? This then determines how requests to the server are called. Do you feel that is an idea worth pursuing?

cc/transport/util.h Outdated
absl::Status to_absl_status(const grpc::Status& grpc_status);

} // namespace oak::transport
#endif // CC_TRANSPORT_UTIL_H_
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: add new line at the end of the file

@pmcgrath17 pmcgrath17 enabled auto-merge (squash) April 2, 2024 15:59
@pmcgrath17 pmcgrath17 merged commit fa50670 into project-oak:main Apr 2, 2024
24 checks passed
@pmcgrath17 pmcgrath17 deleted the unary_grpc_transport branch April 2, 2024 17:43
copybara-service bot pushed a commit that referenced this pull request Apr 9, 2024
@jblebrun
Update ack branch to main so builds continue succeeding.
f24e62e 
Squashed commit of the following:

commit 1b24ec1
Author: Tiziano Santoro <[email protected]>
Date:   Thu Apr 4 00:50:26 2024 +0100

    Update nix deps (#4979)

    Among other things, this update xz to v. 5.4.6

commit f68df2b
Author: Tiziano Santoro <[email protected]>
Date:   Thu Apr 4 00:46:45 2024 +0100

    Align with internal linter (#4978)

    b/332740854

commit 8bdd773
Author: jblebrun <[email protected]>
Date:   Wed Apr 3 21:43:20 2024 +0000

    Update h2 to resolve vulnerability discovered by deny (#4977)

    https://rustsec.org/advisories/RUSTSEC-2024-0332

commit 5bc91be
Author: jul-sh <[email protected]>
Date:   Wed Apr 3 16:05:06 2024 -0400

    Directly issue kernel provenance for attestation measurements (#4976)

    * Directly issue kernel provenance attestation measurements

    Previously the provenance created by the SLSA builder was just for the bzImage. Not the artifact that would be measured in the attestation. With this PR the provenance subjects should include binaries measured in the attestation.

    Change-Id: I16e3234d0d65e3790319294c416c378cd7611681

    * fix typo

    Change-Id: I3d078256d085ef05171e5997743d7497fc530ad0

commit 2ae6255
Author: Andri Saar <[email protected]>
Date:   Tue Apr 2 20:55:51 2024 +0000

    Do a page state change operation before invoking `PVALIDATE`

commit 8452885
Author: conradgrobler <[email protected]>
Date:   Wed Apr 3 17:54:04 2024 +0100

    Ensure CPUID triggered the #VC exception (#4974)

    We want to make sure that the instruction pointer in a #VC exception really pointed to a CPUID instruction since it is the only #VC exception type we support.

commit 4ad534f
Author: thmsbinder <[email protected]>
Date:   Wed Apr 3 18:09:46 2024 +0200

    Add and verify endorsement field for text reference value (#4973)

    The kernel command line reference value now follows the pattern from other reference values: skip, TR endorsement, or direct verification. When using TR endorsements in conjunction with the kernel command line the regex feature needs to be enabled.

commit fa50670
Author: Patrick McGrath <[email protected]>
Date:   Tue Apr 2 10:43:22 2024 -0700

    Unary gRPC transport template class (#4970)

    Implement unary transport class template for future Oak clients that use the unary interface.

commit 65f6b46
Author: k-naliuka <[email protected]>
Date:   Fri Mar 29 00:33:37 2024 +0100

    Add go and java options to the TcbVersion proto (#4969)

commit cefb3c3
Author: Andri Saar <[email protected]>
Date:   Thu Mar 28 15:46:31 2024 +0000

    Collect, and print out, some `PVALIDATE` stats in stage0

commit 579e92c
Author: k-naliuka <[email protected]>
Date:   Wed Mar 27 20:49:53 2024 +0100

    Refactor text reference values matching  (#4965)

    Allow literal string comparison and  make regex optional

commit 121a6b0
Author: Ivan Petrov <[email protected]>
Date:   Wed Mar 27 19:13:14 2024 +0000

    Sign group keys as part of Key Provisioning (#4961)

    This PR adds the ability to sign group keys in the attestation evidence as part of Key Provisioning.

    Ref #4442

commit 2a57cd6
Author: jul-sh <[email protected]>
Date:   Wed Mar 27 12:10:56 2024 -0400

    Revert "Increase the size of the certificate in Stage0 DICE data (#4946)" (#4966)

    This reverts commit c869644, as it introduced a breaking change that broke imports.

commit 57a8f73
Author: Ivan Petrov <[email protected]>
Date:   Wed Mar 27 15:29:07 2024 +0000

    Add GroupEncryptionKeyHandle to C++ Containers SDK (#4964)

    Ref #4442

commit 863ee00
Author: k-naliuka <[email protected]>
Date:   Wed Mar 27 14:15:48 2024 +0100

    Include regex in Bazel oak_crates_index (#4960)

commit 83d881d
Author: Tiziano Santoro <[email protected]>
Date:   Wed Mar 27 09:53:32 2024 +0000

    Fix username and host when building kernel (#4963)

    b/330744888

Change-Id: Iac4a71c2d14238ccaca13c3997f47aa265a789ba
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ipetr0v ipetr0v ipetr0v approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@pmcgrath17 @ipetr0v