open-telemetry / opamp-spec

OpAMP Specification
Apache License 2.0
102 stars 33 forks source link

Need clarification on how OpAMP sends sensitive credential data to the client #80

Closed dsvanlani closed 2 years ago

dsvanlani commented 2 years ago

This was first discussed in the workgroup on 5/3/2022.

Issue

How do we handle sending sensitive credentials to the client? Consider this use case.

A client uses an exporter that needs an API Key to send data. It does not want to store the key in its configuration directly but rather in a file located on disk. OpAMP should establish a way to send this sensitive data so that a client can recognize it, store it, and not report it back in its EffectiveConfig status message. Additionally, OpAMP should have the capability of sending a message that would update these credentials.

It's not clear in the spec how these credentials should be sent.

Discussed solutions

  1. Send credentials in AgentRemoteConfig message.

In this case the client would need to know to redact certain fields of its EffectiveConfig status message.

  1. Send credentials in ConnectionSettingsOffers Message.

Similar to above but the client wouldn't be required to send its EffectiveConfig after this message. The message might need to be expanded to be able to send arbitrary key-value pairs, leaving it up to the client to know what to do with them.

tigrannajaryan commented 2 years ago

We discussed the possibility to add arbitrary key/values to ConnectionSettings:

message ConnectionSettings {
   map<string,string> other_settings = 7;
}

A few things that we need to clarify with this approach:

tigrannajaryan commented 2 years ago

I looked at my initial prototype implementation where I actually use the OpAMP connection settings and own metrics connection settings and the code doesn't benefit in any way from the fact that ConnectionSettings is a shared message.

I am leaning towards having dedicated messages for each connection type, e.g.

message OpAMPConnectionSettings {
    string destination_endpoint = 1;
    Headers headers = 2;
    TLSCertificate certificate = 3;
    // in the future maybe we can add a field to specify the transport type for OpAMP (i.e. HTTP or WebSocket)
    ...
}

message TelemetryConnectionSettings {
    string destination_endpoint = 1;
    Headers headers = 2;
    TLSCertificate certificate = 3;
    // in the future maybe we can add a field to specify the transport type for OTLP (i.e. HTTP or gRPC)
    // or even add support for non-OTLP destinations.
}

message OtherConnectionSettings {
    string destination_endpoint = 1;
    Headers headers = 2;
    TLSCertificate certificate = 3;
    map<string,string> other_settings = 4; // agent-specific settings
    ...
}

Splitting the messages allows them to carry only the fields that are applicable to the particular destination type and allows to evolve them separately in the future.

@andykellr @dsvanlani what do you think?

tigrannajaryan commented 2 years ago

@andykellr @pmm-sumo @dsvanlani here is a draft that shows what it looks like in Go: https://github.com/open-telemetry/opamp-go/pull/82

What do you think?

tigrannajaryan commented 2 years ago

@andykellr @dsvanlani now that we have the other_settings map is there anything else is needed for this issue?

andykellr commented 2 years ago

This should work well.