project-alvarium / alvarium-sdk-go Goto Github PK

Implement a new Annotator that will validate whether or not the incoming request over HTTP is secured via TLS.

For now, this will not cover pub/sub scenarios (such as MQTT over TLS) but I expect that may become a requirement in the future.

Handle Target URI derived component as it is not handled by the parser in handler/base.go.
As indicated in the proposal, the Signature-Input header's value can contain “@target-uri” so that the full target URI is added to the signature’s seed.

Now that we've provided the annotator to validate an HTTP Signature header, it is suggested we also provide a means through the SDK to assemble the Signature-Input and Signature header values.

An example of how these might be used can be found in the logic for the test TestHttpPkiAnnotator_Do. Referring to the following line

A list of keys are provided to specify which headers should be included as part of the Signature-Input header. These are passed along with some other information into a function that assembles the header values for inclusion with the HTTP request that is about to be sent.

This functionality should be located in the /pkg directory. I'm looking for suggestions from the team. Once complete, this new functionality should replace the inline functions in the above referenced test. That is, the test should be able to use the new logic instead of maintaining a duplicate copy.

Currently the TPM annotator is simply setting its IsSatisfied property to false. Extend this with a simple check to what appears to be the default path to a TPM 2.0 device `/dev/tpm0'.

If a file indicating a mounted device exists at that location, IsSatisfied should be true.

Two potential follow-ups:

• At some point, someone may have a non-standard path and wish to pass this in via config. However passing in a path via config means someone could just pass in any path, not necessarily that of the TPM
• I looked to see if there was some programmatic way to get this information apart from checking the existence of a path but they would require that either the executing application consuming the SDK to run as root or looking for magic string values in logs/files.

Google's go-tpm module provides functionality for interacting with TPMs both version 1.2 and 2.0. However I'm avoiding that dependency at this time until we have more sophisticated needs. I want to keep the SDK's transitive dependencies as low as possible and there are quite a few in that module.

inside the Validate function on the AnnotationType enum, pki-http is not being checked on. pki-http should be added to the checks.

It appears the checks performed in the TPM Annotator may, in fact, be reversed.

Review against the example code from OpenTPM mentioned in the comment, and also review in the context of the current tests.

The Validator() function for AnnotationType is missing "src" as a valid value and so an error is being thrown erroneously.

func (t AnnotationType) Validate() bool {
	if t == AnnotationPKI || t == AnnotationTPM {
		return true
	}
	return false
}

I've learned during operational testing that the C-bindings in the IOTA Streams provider do not provide any kind of locking to guard against concurrent writes. The underlying Streams implementation requires a sequential write to the stream otherwise messages can get lost and/or individual subscribers might get confused and stop processing messages.

It looks like the simple way to deal with this for now is to wrap the act of writing the message to the stream in a mutex.

The mutex will guarantee only one writer gains access to this piece of logic at a time. The Alvarium SDK is bootstrapped in a given application as a single instance. In cases where two writes are performed quickly or simultaneously between goroutines, we can observe the above irregular behavior.

Confirmed this with @DyrellC from IOTA on a call a couple weeks ago.

in factory.go, the switch case for MqttStream produces the same error mesage for MockStream. It is required to change it:

from

"invalid cast for MockStream"

to

"invalid cast for MqttStream"

Use case:

• Service A originates a piece of data and passes it to Service B via REST
• Service B is responsible for mutating the data in some way, but first it annotates the received data via Sdk.Transit()
• Service B then performs its filter/mutate action on the original data and so calls Sdk.Mutate()
• Because the annotators for the Sdk are passed in once, both of these methods create the same annotations. However the new data element mutated from the original is penalized due to the TLS annotator called by Sdk.Mutate()

Certain annotations may not be relevant in all cases. Data that originates within a service will have no need of a TLS annotator. Only Transit() should care about that.

There are probably ramifications for the overall SDK design here -- indicating that annotations should be configued by Sdk method rather than universally applicable. This is because more than one SDK method can be called by an application as part of a given execution path.

For immediate purposes, the recommendation is to ignore the TLS annotator when Sdk.Mutate() is called.

During the TSC call on 19-June-2023 it was decided that the legacy C Bindings used to formerly integrate with IOTA will be removed. This is primarily because they have aged significantly since they were originally contributed. Following from their lack of use due to age, they add a further burden during integration as a statically linked dependency which may not even be used once the targeted application is deployed. This adds developer overhead as well as increasing the size of the build artifact.

• Directions for linking/building against the C Bindings should be removed from the ReadMe
• The internal/iota directory should be removed
• Audit project for other references to iota such as in constants, unit tests or configuration structs.

I propose consideration of extending the SDK interface with a Publish method. The purpose of this method would be to provide extensibility for annotators that may need to attest to the state of data before it is sent over the wire. This could then be compared to the result of annotations captured during Transit by a receiving party/host. Publish could also be useful in cases where the downstream host receiving the data isn't running Alvarium-enabled applications. Let me illustrate with a couple use cases.

Both of these assume the application developer has a need to implement custom annotators for their application domain by implementing the Annotator interface

1.) Some piece of data -- for example, a customer -- must be scrubbed of all personally-identifying information (PII) before the record is sent downstream. The developer implements a domain model representing the customer state and passes that to a custom PiiAnnotator to verify that the data has been scrubbed. This new annotator must be developed by the application owner since it requires knowledge of the application state. Before publishing the anonymized record, the application can invoke the SDK's Publish method to attest to the fact that it scrubbed the data before transmit.

2.) If the application owner also owns the downstream endpoint, then the custom annotator could be utilized to validate the scrubbed state of the data upon receipt. If the application owner does NOT own the downstream endpoint, the Publish annotation could provide evidence of how the data was handled prior to export.

It seems odd that unmarshaling a config designated as a Mock would return an actual provider config. Is there a reason for this? I'm using a Mock in a unit test, and the unit test is comparing the result of unmarshaling to a control instance of a configuration. The test config is provisioned as a mock, as is the control instance. However the comparison is failing b/c when the test instance is unmarshaled, it has this IOTAConfig in it. This means I have to put a spurious IOTAConfig in my control instance, which I'd prefer not to do.

A mocked StreamProvider should just bury anything sent to it, so it doesn't really need a Config instance at all -- or at least just a shim.

Currently only a PublishWrapper is provided by the messages package. The idea behind this type is to allow an indication of the type for the Content property's value. But when an instance of this type itself is marshaled to JSON as a byte array, it makes type introspection impossible on the other end (subscribing). As such, Content can't be cast to a type whose kind is provided in MessageType.

Suggest implementing a SubscribeWrapper to be used by subscribers where Content is a byte array. Individual publishers should then be responsible for handling the incoming PublishWrapper in such a manner that Content is a byte array when consumed on the other end.

As of Go v1.21, the Go standard library has a new structured logging package called slog.

All Alvarium-related applications found in this Github org that are written in Go currently utilize our custom provider-logging package. At the time these apps were created, no standard logging capability existed in the Go standard library so we created our own provider.

In the interest of reducing external dependencies, especially where we can get what we want from the standard library, I recommend the project evaluate this new slog package and compare it with our current solution. It appears we'll be able to obtain a similar level of control w/r/t the format of the messages. Each message is formatted in JSON allowing marshaling/unmarshaling to a type instance and easy filtration, sorting, etc via the back-end.

This issue targets the development task for integrating slog and removing provider-logging leading to a comparative evaluation of the resulting log format and ease of use for the slog API

The type of that property should not be interface{}. It should be []byte to agree with the SubscribeWrapper. Making this changes will eliminate the need for provider-specific wrapper classes such as the mqttWrapper

The above wrapper class was created when a problem was noticed unwrapping messages via MQTT. Now that the behavior has also been observed (and solved via local testing) using IOTA, the root cause was more properly identified.

This proposal comes as a result of work being done to integrate the Alvarium Go SDK into EdgeX Foundry. The integration work is being attempted on the "Levski" release as the 3.0 release of EdgeX is not yet finalized. During the functional testing of the Virtual Device service, an issue was encountered whereby the configuration markup format for the service (TOML) does not handle elements of the Alvarium SDK properly. The specific issue occurs on the following line:

As you can see the Annotators property is an array of values coming from an enum AnnotationType. The members of this enum are strings, however the TOML parser is getting confused by the type metadata associated with the values because of the enum. There IS a way to fix this, however it would require changes in the go-mod-configuration module which is used by every service in the EdgeX platform. This would greatly increase the surface area of the integration and expand the amount of functional testing required. Further, it would bleed Device-SDK specific type mgmt into a module whose concerns should be agnostic.

In the upcoming 3.0 release of EdgeX Foundry, the project is changing how configuration is defined and managed. In particular, the new config file markup will be YAML. I have POC'ed a small harness application to target the above scenario and see if the YAML implementation handles the enum typing correctly, and it does!

Therefore I propose we extend the configuration support within the Alvarium SDK to YAML. I believe this will be as simple as adding yaml struct tags to the members of all configuration types. The consuming app will implement the relevant config mgmt for reading the file and unmarshaling according to their desired format.

The Annotator Factory switch case handler does not currently include an entry for tls annotator instantiation. It incorrectly throws an error when tls is passed to it.

Provide the initial code for the Go SDK

There should be an abstraction/interface behind which we can provide several different key handlers. The handler used should be driven by configuration and returned via a factory pattern.

For now, the factory will return an error as no concrete types are available yet. Implemented along with it will be unit tests to validate this behavior.

The implementation of the Mutate method on the SDK violates guidelines related to directionality of import references. The guideline is that in general, references from /pkg (contracts) to /internal should be avoided, while references in the other direction are fine. This helps to avoid the problem of circular references which can happen when references are taken on for convenience. It also preserves application layer integrity by encapsulating internal functionality away from contract functionality.

However, there is a case where this direction of reference is allowed -- when using a factory pattern in the contract package. The factory returns an internal type that conforms to a contract defined interface. You don't want the client using the SDK to reference the internal type directly, however it needs some kind of externally facing interface to work with the type.

As shown here

This method implementation violates the principle by calling the SourceAnnotator constructor directly. Instead, the existing factory should be extended for this use case. Refactor the associated test as well.

Since the Annotation.Satisfied property is a boolean its name should indicate the type. Do so by renaming the property Annotation.IsSatisfied

For context, see comment here:

Essentially, our current PKI annotator has some constraints in that incoming data must support unmarshaling via JSON to a type that has a Signature property. This eliminates our ability to annotate data that is not in JSON format or otherwise does not provide the necessary property. We need to leverage a means independent of the data payload for this capability and so I propose looking for some way to leverage the means of transport.

The following IETF draft proposes using HTTP headers to support signature validation.
https://httpwg.org/http-extensions/draft-ietf-httpbis-message-signatures.html

Assuming the relevant Signature and Signature-Input headers are present, in an HTTP context the Request can be passed into the annotator which would obtain the values and run the verification. Using headers would also possibly provide insight toward a relevant abstraction for a similar means of verification in pub-sub scenarios where message headers are available.