Endorsement Store Interface Implementation by shefali-kamal · Pull Request #435 · veraison/services

shefali-kamal · 2026-08-04T12:49:31Z

This PR implements following:

contains the implementation of endorsement store plugin interface
converted coserv proxy plugins to endorsement store plugins
make changes in vts to use the endorsement store plugins
vts now uses a composite store made up of corimstore and store plugins as fallback

Address issue #431

Definition of protobuf messages used as arguments for endorsement store interface. Signed-off-by: Dhanus M Lal <Dhanus.MLal@fujitsu.com>

The store plugin interface definition and implementation of the RPC layer (for go-plugin client and server). Endorsement store interface is an aggregation of IEndorsementStoreReader and IEndorsementStoreWriter, for read and write operations on endorsement store respectively. Signed-off-by: Dhanus M Lal <Dhanus.MLal@fujitsu.com>

Register endorsementstore targets in the build system and docker deployment. Signed-off-by: Dhanus M Lal <Dhanus.MLal@fujitsu.com>

The coserv proxy plugins are now store plugins, with only the ExecuteCoservQuery method implemented. Note: builtin loader may not work. Note: the new stores (amd and nvidia) have not been tested yet. Signed-off-by: Dhanus M Lal <Dhanus.MLal@fujitsu.com>

Add corimstore based store plugin. The implementation converts existing store implementation into a plugin. Signed-off-by: Dhanus M Lal <Dhanus.MLal@fujitsu.com>

Update VTS to use the IEndorsementStore interface. Signed-off-by: Dhanus M Lal <Dhanus.MLal@fujitsu.com>

q -> %q Signed-off-by: Dhanus M Lal <Dhanus.MLal@fujitsu.com>

Signed-off-by: Dhanus M Lal <Dhanus.MLal@fujitsu.com>

setrofim

I don't think the notion of a "media type" makes sense in the context of the store backend. We don't really want to be selecting the backend based on it. I also don't like the somewhat arbitrary distinction between "primary" and "fallback" backends.

Suggestion: store backends are identified solely by name; for the sake of reusing existing plugin loader/manager the media type APIs implemented via a shim to to just return the name, and this is hidden as much as possible. Store configuration contains a list of backend names, which specifies which frontend plugins will actually be actively used by the store frontend. When servicing requests, the frontend tries the backends in the order specified until.

setrofim · 2026-08-05T09:18:32Z

+
+func NewStore() *DefaultStore {
+	logger := log.Named(PluginName)
+	logger.Debug("initializing default store")


This log line belongs inside DefaultStore.Init() below, not here.

setrofim · 2026-08-05T09:21:33Z

+
+func (s *DefaultStore) Init(params *plugin.Parameters) error {
+	if params == nil {
+		panic("parameters are required for corimstore")


Return an error rather than panic here.

setrofim · 2026-08-05T09:22:42Z

+
+	store, err := corimstore.Open(context.Background(), cfg.StoreConfig())
+	if err != nil {
+		panic(err)


Return the error rather than panic.

setrofim · 2026-08-05T09:22:47Z

+	// initialize it here.
+	if strings.Contains(cfg.DSN, ":memory:") {
+		if err := store.Init(); err != nil {
+			panic(err)


Return the error rather than panic.

setrofim · 2026-08-05T09:29:57Z

+	return SchemeName
+}
+
+func (s *DefaultStore) GetSupportedMediaTypes() map[string][]string {


I don't think the concept of a "supported media type" really makes sense for a store backend. I think we may want a different plugin interface for this.

Since store plugins can also serve CoSERV queries, the supported media types can be the CoSERV media types they support.

setrofim · 2026-08-05T09:35:30Z

+	return res, err
+}
+
+func (s *DefaultStore) ExecuteCoservQuery(mediaType, query string) (*coserv.Coserv, error) {


Why does this take a mediaType? It does not appear to be used, and also clashes with remaining store API that takea label instead. The media type should be resolved to a label by the time the request reaches the store.

setrofim · 2026-08-05T09:36:21Z

 }

-func (s CoservProxyHandler) GetEndorsements(tenantID string, query string) ([]byte, error) {
+func (s CoservProxyHandler) ExecuteCoservQuery(mediaType, query string) (*coserv.Coserv, error) {


Ditto here. the media type should be resolved by this point.

yogeshbdeshpande · 2026-08-07T08:25:02Z


-	if err := o.CoservProxyPluginManager.Close(); err != nil {
-		o.logger.Errorf("coserv plugin manager shutdown failed: %v", err)
+	// FIXME: close stores while closing store manager


Why do we need Line 174..?

Should not the Sore manager be responsible for closing the Stores!

That's correct. But with the current implementation of the store manager, it is not possible. This is because the pluggable interface lacks a Close method, which can be called at the end (similar to the Init method that gets called at the start). The StoreManager.Close method calls the plugin.Client.Kill method on the store plugin client, which terminates the plugin process, without closing the file descriptors and other resources that are in use by the plugin process. The note here was for myself and this should be fixed before the pull request can be merged.

yogeshbdeshpande · 2026-08-07T08:26:11Z


 	coservProxyDerived := c.assembleCoservMediaTypes(
-		c.CoservProxyPluginManager.GetRegisteredMediaTypes(),
+		c.StoreManager.GetRegisteredMediaTypes(),


Agree with Sergei, the Store should not have a Media Type.

The functionality must be changed!

yogeshbdeshpande

Have started reviewing it, will complete by the end of the day today!

thomas-fossati

Thanks for this!

I have inlined a few comments.

thomas-fossati · 2026-08-12T07:35:11Z

+		if err != nil {
+			o.logger.Warnw("could not find in store", "valID", valID, "error", err)


When joining the dots between L526-527 and L532-533, it looks like any store errors will be seen as ENOTFOUND from the pov of the caller. If that's the case, the caller (L446) will log and move on. So, imagine the case where the DB is down, valueTriples stays empty, getValueTriples returns ENOTFONUD, the caller logs a warning and proceeds with nil endorsements into AppraiseEvidence. I don't think it's what we want, right?

All errors other than ENOTFOUND from all the stores are logged as warning (/vts/trustedservices/store.go:122). The reason was to avoid the operation from failing some of the stores are down. But now I realized that this is a bad idea. Will update to return on unexpected store errors.

thomas-fossati · 2026-08-12T07:37:43Z

+	if err := CloseCorimStore(o.StoreManager); err != nil {
+		o.logger.Errorf("failed to close corim store: %v", err)
 	}

-	if err := o.Store.Close(); err != nil {
-		o.logger.Errorf("store closure failed: %v", err)
+	if err := o.StoreManager.Close(); err != nil {
+		o.logger.Errorf("store plugin manager shutdown failed: %v", err)


Potential ENOCOFFEE :-) Would this effectively result in a double close?

No, StoreManager.Close does not properly close the corim-store (see #435 (comment)). This is a hacky fix and should be fixed before merging the pr.

thomas-fossati · 2026-08-12T07:44:14Z

+	if err != nil {
+		o.logger.Infof("could not find coserv result in store")


What is the reasoning behind masking any error as ENOTFOUND/404?
From a ReST perspective, we want to be able to tell our clinets that there is something wrong with the service (5xx) rather than with their query (4xx).

The API layer seems to treats all errors from VTS as Internal Server Error.

coserv:

services/coserv/api/handler.go

Lines 225 to 231 in f997c52

// Forward query to VTS

res, err := o.EndorsementDistibutor.GetEndorsements(tenantID, coservQuery, mediaType)

if err != nil {

status := http.StatusBadRequest

reportProblem(c, status, err.Error())

return

}

verification:

services/verification/api/handler.go

Lines 414 to 430 in f997c52

// Forward the evidence to the verifier. We expect the verifier to be

// able to cope with bad evidence, so the error here should only be

// reported if something in the verifier or the connection goes wrong.

// Any problems with the evidence are expected to be reported via the

// attestation result.

attestationResult, err := o.Verifier.ProcessEvidence(tenantID, session.Nonce,

evidence, mediaType)

if err != nil {

o.logger.Error(err)

session.SetStatus(StatusFailed)

mustStoreSession(o.SessionManager, session, id, tenantID)

ReportProblem(c,

http.StatusInternalServerError,

"error encountered while processing evidence",

)

return

}

But more context can be added to the error. I will try to do that.

thomas-fossati · 2026-08-12T07:49:50Z

+
+func (s *DefaultStore) ExecuteCoservQuery(mediaType, query string) (*coserv.Coserv, error) {
+	s.logger.Infof("got coserv query: %v", query)
+	fallbackAuthority, err := comid.NewCryptoKeyTaggedBytes([]byte("dummyauth"))


curious: why we need to use this "dummyauth"?

This is a copy-paste of an older version of the code (before #420). I should update this with the latest changes. Thanks for pointing it out.

thomas-fossati · 2026-08-12T07:50:55Z

+option go_package = "github.com/veraison/services/proto";
+
+message GetEndorsementsArgs {
+  bytes environment = 1 [json_name = "environemnt-map"];


Suggested change

bytes environment = 1 [json_name = "environemnt-map"];

bytes environment = 1 [json_name = "environment-map"];

thomas-fossati · 2026-08-12T07:51:28Z

+	if subs["coserv"].IsSet("signer") {
+		coservContext, err = coserv.NewCoservContextFromViper(subs["coserv"])
+		if err != nil {
+			log.Fatal("CoSERV config initialization: %v", err)


Suggested change

log.Fatal("CoSERV config initialization: %v", err)

log.Fatalf("CoSERV config initialization: %v", err)

DhanusML added 9 commits July 31, 2026 12:09

feat(proto): protobuf definitions for store

313e82d

Definition of protobuf messages used as arguments for endorsement store interface. Signed-off-by: Dhanus M Lal <Dhanus.MLal@fujitsu.com>

build(endorsementstore): build and docker deploy

f8afe28

Register endorsementstore targets in the build system and docker deployment. Signed-off-by: Dhanus M Lal <Dhanus.MLal@fujitsu.com>

feat(endorsementstore): corimstore store plugin

4c5354e

Add corimstore based store plugin. The implementation converts existing store implementation into a plugin. Signed-off-by: Dhanus M Lal <Dhanus.MLal@fujitsu.com>

feat(vts): udpate vts to use endorsementstore

21418c3

Update VTS to use the IEndorsementStore interface. Signed-off-by: Dhanus M Lal <Dhanus.MLal@fujitsu.com>

fix(plugin): typo in logging

ec4f6db

q -> %q Signed-off-by: Dhanus M Lal <Dhanus.MLal@fujitsu.com>

style: fix formatting in scheme_rpc.go

05e95e8

Signed-off-by: Dhanus M Lal <Dhanus.MLal@fujitsu.com>

temp: ignore coverage for corimstore

3b90932

Signed-off-by: Dhanus M Lal <Dhanus.MLal@fujitsu.com>

setrofim requested changes Aug 5, 2026

View reviewed changes

yogeshbdeshpande reviewed Aug 7, 2026

View reviewed changes

THS-on requested a review from thomas-fossati August 11, 2026 15:37

thomas-fossati reviewed Aug 12, 2026

View reviewed changes

		if err != nil {
		o.logger.Warnw("could not find in store", "valID", valID, "error", err)

		if err != nil {
		o.logger.Infof("could not find coserv result in store")

	// Forward query to VTS
	res, err := o.EndorsementDistibutor.GetEndorsements(tenantID, coservQuery, mediaType)
	if err != nil {
	status := http.StatusBadRequest
	reportProblem(c, status, err.Error())
	return
	}

	// Forward the evidence to the verifier. We expect the verifier to be
	// able to cope with bad evidence, so the error here should only be
	// reported if something in the verifier or the connection goes wrong.
	// Any problems with the evidence are expected to be reported via the
	// attestation result.
	attestationResult, err := o.Verifier.ProcessEvidence(tenantID, session.Nonce,
	evidence, mediaType)
	if err != nil {
	o.logger.Error(err)
	session.SetStatus(StatusFailed)
	mustStoreSession(o.SessionManager, session, id, tenantID)
	ReportProblem(c,
	http.StatusInternalServerError,
	"error encountered while processing evidence",
	)
	return
	}

	bytes environment = 1 [json_name = "environemnt-map"];
	bytes environment = 1 [json_name = "environment-map"];

	log.Fatal("CoSERV config initialization: %v", err)
	log.Fatalf("CoSERV config initialization: %v", err)

Uh oh!

Conversation

shefali-kamal commented Aug 4, 2026

Uh oh!

setrofim left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yogeshbdeshpande left a comment

Choose a reason for hiding this comment

Uh oh!

thomas-fossati left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants