From 8f6d5ce05a81e673d3e37e2a66393d5d3aeec44a Mon Sep 17 00:00:00 2001 From: Seth Terashima Date: Wed, 28 Feb 2024 16:07:50 -0800 Subject: [PATCH 1/5] Document protocol-level errors --- pkg/protocol/protobuf/universal_message.proto | 48 +++++++++---------- .../universalmessage/universal_message.pb.go | 48 +++++++++---------- pkg/protocol/protocol.md | 4 ++ 3 files changed, 52 insertions(+), 48 deletions(-) diff --git a/pkg/protocol/protobuf/universal_message.proto b/pkg/protocol/protobuf/universal_message.proto index fd9c2e81..4b46739c 100644 --- a/pkg/protocol/protobuf/universal_message.proto +++ b/pkg/protocol/protobuf/universal_message.proto @@ -29,30 +29,30 @@ enum OperationStatus_E enum MessageFault_E { - MESSAGEFAULT_ERROR_NONE = 0; - MESSAGEFAULT_ERROR_BUSY = 1; - MESSAGEFAULT_ERROR_TIMEOUT = 2; - MESSAGEFAULT_ERROR_UNKNOWN_KEY_ID = 3; - MESSAGEFAULT_ERROR_INACTIVE_KEY = 4; - MESSAGEFAULT_ERROR_INVALID_SIGNATURE = 5; - MESSAGEFAULT_ERROR_INVALID_TOKEN_OR_COUNTER = 6; - MESSAGEFAULT_ERROR_INSUFFICIENT_PRIVILEGES = 7; - MESSAGEFAULT_ERROR_INVALID_DOMAINS = 8; - MESSAGEFAULT_ERROR_INVALID_COMMAND = 9; - MESSAGEFAULT_ERROR_DECODING = 10; - MESSAGEFAULT_ERROR_INTERNAL = 11; - MESSAGEFAULT_ERROR_WRONG_PERSONALIZATION = 12; - MESSAGEFAULT_ERROR_BAD_PARAMETER = 13; - MESSAGEFAULT_ERROR_KEYCHAIN_IS_FULL = 14; - MESSAGEFAULT_ERROR_INCORRECT_EPOCH = 15; - MESSAGEFAULT_ERROR_IV_INCORRECT_LENGTH = 16; - MESSAGEFAULT_ERROR_TIME_EXPIRED = 17; - MESSAGEFAULT_ERROR_NOT_PROVISIONED_WITH_IDENTITY = 18; - MESSAGEFAULT_ERROR_COULD_NOT_HASH_METADATA = 19; - MESSAGEFAULT_ERROR_TIME_TO_LIVE_TOO_LONG = 20; - MESSAGEFAULT_ERROR_REMOTE_ACCESS_DISABLED = 21; - MESSAGEFAULT_ERROR_REMOTE_SERVICE_ACCESS_DISABLED = 22; - MESSAGEFAULT_ERROR_COMMAND_REQUIRES_ACCOUNT_CREDENTIALS = 23; + MESSAGEFAULT_ERROR_NONE = 0; // Request succeeded. + MESSAGEFAULT_ERROR_BUSY = 1; // Required vehicle subsystem is busy. Try again. + MESSAGEFAULT_ERROR_TIMEOUT = 2; // Vehicle subsystem did not respond. Try again. + MESSAGEFAULT_ERROR_UNKNOWN_KEY_ID = 3; // Vehicle did not recognize the key used to authorize command. Make sure your key is paired with the vehicle. + MESSAGEFAULT_ERROR_INACTIVE_KEY = 4; // Key used to authorize command has been disabled. + MESSAGEFAULT_ERROR_INVALID_SIGNATURE = 5; // Command signature/MAC is incorrect. Use included session info to update session and try again. + MESSAGEFAULT_ERROR_INVALID_TOKEN_OR_COUNTER = 6; // Command anti-replay counter has been used before. Use included session info to update session and try again. + MESSAGEFAULT_ERROR_INSUFFICIENT_PRIVILEGES = 7; // User is not authorized to execute command. This can be because of the role or because of vehicle state. + MESSAGEFAULT_ERROR_INVALID_DOMAINS = 8; // Command was malformed or addressed to an unrecognized vehicle system. May indicate client error or older vehicle firmware. + MESSAGEFAULT_ERROR_INVALID_COMMAND = 9; // Unrecognized command. May indicate client error or unsupported vehicle firmware. + MESSAGEFAULT_ERROR_DECODING = 10; // Could not parse command. Indicates client error. + MESSAGEFAULT_ERROR_INTERNAL = 11; // Internal vehicle error. Try again. Most commonly encountered when the vehicle has not finished booting. + MESSAGEFAULT_ERROR_WRONG_PERSONALIZATION = 12; // Command sent to wrong VIN. + MESSAGEFAULT_ERROR_BAD_PARAMETER = 13; // Command was malformed or used a deprecated parameter. + MESSAGEFAULT_ERROR_KEYCHAIN_IS_FULL = 14; // Vehicle's keychain is full. You must delete a key before you can add another. + MESSAGEFAULT_ERROR_INCORRECT_EPOCH = 15; // Session ID mismatch. Use included session info to update session and try again. + MESSAGEFAULT_ERROR_IV_INCORRECT_LENGTH = 16; // Initialization Value length is incorrect (AES-GCM must use 12-byte IVs). Indicates a client programming error. + MESSAGEFAULT_ERROR_TIME_EXPIRED = 17; // Command expired. Use included session info to determine if clocks have desynchronized and try again. + MESSAGEFAULT_ERROR_NOT_PROVISIONED_WITH_IDENTITY = 18; // Vehicle has not been provisioned with a VIN and may require service. + MESSAGEFAULT_ERROR_COULD_NOT_HASH_METADATA = 19; // Internal vehicle error. + MESSAGEFAULT_ERROR_TIME_TO_LIVE_TOO_LONG = 20; // Vehicle rejected command because its expiration time was too far in the future. This is a security precaution. + MESSAGEFAULT_ERROR_REMOTE_ACCESS_DISABLED = 21; // The vehicle owner has disabled Mobile access. + MESSAGEFAULT_ERROR_REMOTE_SERVICE_ACCESS_DISABLED = 22; // The command was authorized with a Service key, but the vehicle has not been configured to permit remote service commands. + MESSAGEFAULT_ERROR_COMMAND_REQUIRES_ACCOUNT_CREDENTIALS = 23; // The command requires proof of Tesla account credentials but was not sent over a channel that provides this proof. Resend the command using Fleet API. } message MessageStatus diff --git a/pkg/protocol/protobuf/universalmessage/universal_message.pb.go b/pkg/protocol/protobuf/universalmessage/universal_message.pb.go index 051ae132..c303197c 100644 --- a/pkg/protocol/protobuf/universalmessage/universal_message.pb.go +++ b/pkg/protocol/protobuf/universalmessage/universal_message.pb.go @@ -127,30 +127,30 @@ func (OperationStatus_E) EnumDescriptor() ([]byte, []int) { type MessageFault_E int32 const ( - MessageFault_E_MESSAGEFAULT_ERROR_NONE MessageFault_E = 0 - MessageFault_E_MESSAGEFAULT_ERROR_BUSY MessageFault_E = 1 - MessageFault_E_MESSAGEFAULT_ERROR_TIMEOUT MessageFault_E = 2 - MessageFault_E_MESSAGEFAULT_ERROR_UNKNOWN_KEY_ID MessageFault_E = 3 - MessageFault_E_MESSAGEFAULT_ERROR_INACTIVE_KEY MessageFault_E = 4 - MessageFault_E_MESSAGEFAULT_ERROR_INVALID_SIGNATURE MessageFault_E = 5 - MessageFault_E_MESSAGEFAULT_ERROR_INVALID_TOKEN_OR_COUNTER MessageFault_E = 6 - MessageFault_E_MESSAGEFAULT_ERROR_INSUFFICIENT_PRIVILEGES MessageFault_E = 7 - MessageFault_E_MESSAGEFAULT_ERROR_INVALID_DOMAINS MessageFault_E = 8 - MessageFault_E_MESSAGEFAULT_ERROR_INVALID_COMMAND MessageFault_E = 9 - MessageFault_E_MESSAGEFAULT_ERROR_DECODING MessageFault_E = 10 - MessageFault_E_MESSAGEFAULT_ERROR_INTERNAL MessageFault_E = 11 - MessageFault_E_MESSAGEFAULT_ERROR_WRONG_PERSONALIZATION MessageFault_E = 12 - MessageFault_E_MESSAGEFAULT_ERROR_BAD_PARAMETER MessageFault_E = 13 - MessageFault_E_MESSAGEFAULT_ERROR_KEYCHAIN_IS_FULL MessageFault_E = 14 - MessageFault_E_MESSAGEFAULT_ERROR_INCORRECT_EPOCH MessageFault_E = 15 - MessageFault_E_MESSAGEFAULT_ERROR_IV_INCORRECT_LENGTH MessageFault_E = 16 - MessageFault_E_MESSAGEFAULT_ERROR_TIME_EXPIRED MessageFault_E = 17 - MessageFault_E_MESSAGEFAULT_ERROR_NOT_PROVISIONED_WITH_IDENTITY MessageFault_E = 18 - MessageFault_E_MESSAGEFAULT_ERROR_COULD_NOT_HASH_METADATA MessageFault_E = 19 - MessageFault_E_MESSAGEFAULT_ERROR_TIME_TO_LIVE_TOO_LONG MessageFault_E = 20 - MessageFault_E_MESSAGEFAULT_ERROR_REMOTE_ACCESS_DISABLED MessageFault_E = 21 - MessageFault_E_MESSAGEFAULT_ERROR_REMOTE_SERVICE_ACCESS_DISABLED MessageFault_E = 22 - MessageFault_E_MESSAGEFAULT_ERROR_COMMAND_REQUIRES_ACCOUNT_CREDENTIALS MessageFault_E = 23 + MessageFault_E_MESSAGEFAULT_ERROR_NONE MessageFault_E = 0 // Request succeeded. + MessageFault_E_MESSAGEFAULT_ERROR_BUSY MessageFault_E = 1 // Required vehicle subsystem is busy. Try again. + MessageFault_E_MESSAGEFAULT_ERROR_TIMEOUT MessageFault_E = 2 // Vehicle subsystem did not respond. Try again. + MessageFault_E_MESSAGEFAULT_ERROR_UNKNOWN_KEY_ID MessageFault_E = 3 // Vehicle did not recognize the key used to authorize command. Make sure your key is paired with the vehicle. + MessageFault_E_MESSAGEFAULT_ERROR_INACTIVE_KEY MessageFault_E = 4 // Key used to authorize command has been disabled. + MessageFault_E_MESSAGEFAULT_ERROR_INVALID_SIGNATURE MessageFault_E = 5 // Command signature/MAC is incorrect. Use included session info to update session and try again. + MessageFault_E_MESSAGEFAULT_ERROR_INVALID_TOKEN_OR_COUNTER MessageFault_E = 6 // Command anti-replay counter has been used before. Use included session info to update session and try again. + MessageFault_E_MESSAGEFAULT_ERROR_INSUFFICIENT_PRIVILEGES MessageFault_E = 7 // User is not authorized to execute command. This can be because of the role or because of vehicle state. + MessageFault_E_MESSAGEFAULT_ERROR_INVALID_DOMAINS MessageFault_E = 8 // Command was malformed or addressed to an unrecognized vehicle system. May indicate client error or older vehicle firmware. + MessageFault_E_MESSAGEFAULT_ERROR_INVALID_COMMAND MessageFault_E = 9 // Unrecognized command. May indicate client error or unsupported vehicle firmware. + MessageFault_E_MESSAGEFAULT_ERROR_DECODING MessageFault_E = 10 // Could not parse command. Indicates client error. + MessageFault_E_MESSAGEFAULT_ERROR_INTERNAL MessageFault_E = 11 // Internal vehicle error. Try again. Most commonly encountered when the vehicle has not finished booting. + MessageFault_E_MESSAGEFAULT_ERROR_WRONG_PERSONALIZATION MessageFault_E = 12 // Command sent to wrong VIN. + MessageFault_E_MESSAGEFAULT_ERROR_BAD_PARAMETER MessageFault_E = 13 // Command was malformed or used a deprecated parameter. + MessageFault_E_MESSAGEFAULT_ERROR_KEYCHAIN_IS_FULL MessageFault_E = 14 // Vehicle's keychain is full. You must delete a key before you can add another. + MessageFault_E_MESSAGEFAULT_ERROR_INCORRECT_EPOCH MessageFault_E = 15 // Session ID mismatch. Use included session info to update session and try again. + MessageFault_E_MESSAGEFAULT_ERROR_IV_INCORRECT_LENGTH MessageFault_E = 16 // Initialization Value length is incorrect (AES-GCM must use 12-byte IVs). Indicates a client programming error. + MessageFault_E_MESSAGEFAULT_ERROR_TIME_EXPIRED MessageFault_E = 17 // Command expired. Use included session info to determine if clocks have desynchronized and try again. + MessageFault_E_MESSAGEFAULT_ERROR_NOT_PROVISIONED_WITH_IDENTITY MessageFault_E = 18 // Vehicle has not been provisioned with a VIN and may require service. + MessageFault_E_MESSAGEFAULT_ERROR_COULD_NOT_HASH_METADATA MessageFault_E = 19 // Internal vehicle error. + MessageFault_E_MESSAGEFAULT_ERROR_TIME_TO_LIVE_TOO_LONG MessageFault_E = 20 // Vehicle rejected command because its expiration time was too far in the future. This is a security precaution. + MessageFault_E_MESSAGEFAULT_ERROR_REMOTE_ACCESS_DISABLED MessageFault_E = 21 // The vehicle owner has disabled Mobile access. + MessageFault_E_MESSAGEFAULT_ERROR_REMOTE_SERVICE_ACCESS_DISABLED MessageFault_E = 22 // The command was authorized with a Service key, but the vehicle has not been configured to permit remote service commands. + MessageFault_E_MESSAGEFAULT_ERROR_COMMAND_REQUIRES_ACCOUNT_CREDENTIALS MessageFault_E = 23 // The command requires proof of Tesla account credentials but was not sent over a channel that provides this proof. Resend the command using Fleet API. ) // Enum value maps for MessageFault_E. diff --git a/pkg/protocol/protocol.md b/pkg/protocol/protocol.md index b5b4ba5d..e9c8b3fd 100644 --- a/pkg/protocol/protocol.md +++ b/pkg/protocol/protocol.md @@ -715,6 +715,10 @@ Vehicle responses also RoutableMessages. If a protocol-layer error occurred, then the vehicle sets the `RoutableMessage.signedMessageStatus.signed_message_fault` field. +Error codes and their remediation are summarized in +[universal_message.proto](protobuf/universal_message.proto). +See comments in the `MessageFault_E` definition. + ### Infotainment application-layer responses If a reply comes from the Infotainment domain, the client should parse the From a0438f04c3a11883cfa6699eaeed100a66854084 Mon Sep 17 00:00:00 2001 From: Seth Terashima Date: Wed, 28 Feb 2024 17:01:12 -0800 Subject: [PATCH 2/5] Typo fix --- pkg/cli/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cli/config.go b/pkg/cli/config.go index 856677e1..c7596cc6 100644 --- a/pkg/cli/config.go +++ b/pkg/cli/config.go @@ -365,7 +365,7 @@ func (c *Config) Connect(ctx context.Context) (acct *account.Account, car *vehic return nil, nil, err } if skey != nil { - log.Info("Securing connction...") + log.Info("Securing connection...") domains, err := c.DomainNames.ToDomains() if err != nil { return nil, nil, err From e063a23789c40dd517e2db5fc7ceed3738902c3b Mon Sep 17 00:00:00 2001 From: Seth Terashima Date: Fri, 1 Mar 2024 13:10:26 -0800 Subject: [PATCH 3/5] Retry handshake when response times out The client discards handshake response messages that take too long to receive because a long delay prevents accurate synchronization between the client and vehicle clocks. However, the client still considered the handshake complete after receiving a delayed response. Vehicles send unsolicited handshake response messages when they detect a desychronization, and so the StartSession method relied on this automatic detection without verifying the result. This commit fixes the issue by having the client check that a session was successfully established and retrying as needed. --- internal/dispatcher/dispatcher.go | 65 +++++---- internal/dispatcher/dispatcher_test.go | 133 ++++++++++++++---- .../universalmessage/universal_message.pb.go | 5 - 3 files changed, 148 insertions(+), 55 deletions(-) diff --git a/internal/dispatcher/dispatcher.go b/internal/dispatcher/dispatcher.go index 07186835..0ac591ef 100644 --- a/internal/dispatcher/dispatcher.go +++ b/internal/dispatcher/dispatcher.go @@ -52,9 +52,6 @@ func New(conn connector.Connector, privateKey authentication.ECDHPrivateKey) (*D if _, err := rand.Read(dispatcher.address); err != nil { return nil, err } - // Only connections to these domains will be allowed - dispatcher.sessions[universal.Domain_DOMAIN_VEHICLE_SECURITY] = nil - dispatcher.sessions[universal.Domain_DOMAIN_INFOTAINMENT] = nil return &dispatcher, nil } @@ -71,6 +68,7 @@ func (d *Dispatcher) StartSession(ctx context.Context, domain universal.Domain) s, ok := d.sessions[domain] if !ok { d.sessions[domain], err = NewSession(d.privateKey, d.conn.VIN()) + s = d.sessions[domain] } else if s != nil && s.ctx != nil { log.Info("Session for %s loaded from cache", domain) sessionReady = true @@ -79,16 +77,27 @@ func (d *Dispatcher) StartSession(ctx context.Context, domain universal.Domain) if err != nil || sessionReady { return err } - recv, err := d.RequestSessionInfo(ctx, domain) - if err != nil { - return err - } - defer recv.Close() - select { - case reply := <-recv.Recv(): - return protocol.GetError(reply) - case <-ctx.Done(): - return ctx.Err() + for { + recv, err := d.RequestSessionInfo(ctx, domain) + if err != nil { + return err + } + defer recv.Close() + select { + case reply := <-recv.Recv(): + if err = protocol.GetError(reply); err != nil { + return err + } + case <-ctx.Done(): + return ctx.Err() + case <-s.readySignal: + return nil + } + select { + case <-time.After(d.conn.RetryInterval()): + case <-ctx.Done(): + return ctx.Err() + } } } @@ -153,6 +162,16 @@ func (d *Dispatcher) checkForSessionUpdate(message *universal.RoutableMessage, h return } + if d.privateKey == nil { + log.Warning("[%02x] Discarding session info because client does not have a private key", message.GetRequestUuid()) + return + } + + if handler.expired() { + log.Warning("[%02x] Discarding session info because it was received more than %s after request", message.GetRequestUuid(), sessionInfoRequestTimeout) + return + } + tag := message.GetSignatureData().GetSessionInfoTag().GetTag() if tag == nil { log.Warning("[%02x] Discarding unauthenticated session info", message.GetRequestUuid()) @@ -168,17 +187,8 @@ func (d *Dispatcher) checkForSessionUpdate(message *universal.RoutableMessage, h return } - if session == nil { - if session, err = NewSession(d.privateKey, d.conn.VIN()); err != nil { - log.Error("[%02x] Error creating new session: %s", message.GetRequestUuid(), err) - return - } - d.sessions[domain] = session - } - if err = session.ProcessHello(message.GetRequestUuid(), sessionInfo, tag); err != nil { log.Warning("[%02x] Session info error: %s", message.GetRequestUuid(), err) - d.sessions[domain] = nil return } log.Info("[%02x] Updated session info for %s", message.GetRequestUuid(), domain) @@ -238,9 +248,7 @@ func (d *Dispatcher) process(message *universal.RoutableMessage) { // have been a desync. This typically accompanies an error message, and so // the reply still needs to be passed down to the handler after updating // session info. - if !handler.expired() && d.privateKey != nil { - d.checkForSessionUpdate(message, handler) - } + d.checkForSessionUpdate(message, handler) select { case handler.ch <- message: @@ -350,8 +358,13 @@ func (d *Dispatcher) Send(ctx context.Context, message *universal.RoutableMessag if auth != connector.AuthMethodNone { d.sessionLock.Lock() session, ok := d.sessions[message.GetToDestination().GetDomain()] + if ok { + session.lock.Lock() + ok = session.ready + session.lock.Unlock() + } d.sessionLock.Unlock() - if !ok || session == nil { + if !ok { log.Warning("No session available for %s", message.GetToDestination().GetDomain()) return nil, protocol.ErrNoSession } diff --git a/internal/dispatcher/dispatcher_test.go b/internal/dispatcher/dispatcher_test.go index 79bec2e7..14c3d5a4 100644 --- a/internal/dispatcher/dispatcher_test.go +++ b/internal/dispatcher/dispatcher_test.go @@ -15,6 +15,7 @@ import ( "google.golang.org/protobuf/proto" "github.com/teslamotors/vehicle-command/internal/authentication" + "github.com/teslamotors/vehicle-command/pkg/protocol/protobuf/signatures" universal "github.com/teslamotors/vehicle-command/pkg/protocol/protobuf/universalmessage" ) @@ -82,10 +83,12 @@ func testCommand() *universal.RoutableMessage { type dummyConnector struct { inbox []*universal.RoutableMessage + callback func(*dummyConnector, *universal.RoutableMessage) ([]byte, bool) outbox chan []byte replies chan *universal.RoutableMessage lock sync.Mutex errorQueue []error + keyLock sync.Mutex keys map[universal.Domain]authentication.ECDHPrivateKey dropReplies bool AckRequests bool @@ -94,6 +97,7 @@ type dummyConnector struct { func newDummyConnector(t *testing.T) *dummyConnector { t.Helper() conn := dummyConnector{ + callback: handleSessionInfoRequests, outbox: make(chan []byte, 50), replies: make(chan *universal.RoutableMessage, 50), keys: make(map[universal.Domain]authentication.ECDHPrivateKey), @@ -120,8 +124,8 @@ func (d *dummyConnector) Wake() { } func (d *dummyConnector) domainKey(domain universal.Domain) authentication.ECDHPrivateKey { - d.lock.Lock() - defer d.lock.Unlock() + d.keyLock.Lock() + defer d.keyLock.Unlock() if key, ok := d.keys[domain]; ok { return key } @@ -184,16 +188,19 @@ func (d *dummyConnector) SessionInfoReply(rsp protocol.Receiver, publicKeyBytes return reply } -func (d *dummyConnector) handleAsync(message *universal.RoutableMessage) { - d.lock.Lock() - d.inbox = append(d.inbox, message) - d.lock.Unlock() - - var reply *universal.RoutableMessage +func initReply(message *universal.RoutableMessage) *universal.RoutableMessage { + var reply universal.RoutableMessage + reply.ToDestination = message.GetFromDestination() + reply.FromDestination = message.GetToDestination() + reply.RequestUuid = append([]byte{}, message.GetUuid()...) + reply.Uuid = testUUID() + return &reply +} +func handleSessionInfoRequests(d *dummyConnector, message *universal.RoutableMessage) ([]byte, bool) { req := message.GetSessionInfoRequest() if req == nil { - return + return nil, false } domain := message.GetToDestination().GetDomain() @@ -202,32 +209,33 @@ func (d *dummyConnector) handleAsync(message *universal.RoutableMessage) { panic(err) } - reply = &universal.RoutableMessage{} + reply := initReply(message) if err := verifier.SetSessionInfo(message.GetUuid(), reply); err != nil { panic(err) } - reply.ToDestination = message.GetFromDestination() - reply.FromDestination = message.GetToDestination() - reply.RequestUuid = message.GetUuid() - reply.Uuid = testUUID() - encoded, err := proto.Marshal(reply) - if err != nil { panic(err) } + return encoded, true +} + +func (d *dummyConnector) handleAsync(message *universal.RoutableMessage) { d.lock.Lock() defer d.lock.Unlock() - if d.dropReplies { + d.inbox = append(d.inbox, message) + if d.dropReplies || d.callback == nil { return } - select { - case d.outbox <- encoded: - return - default: - panic(errOutboxFull) + if responseBytes, shouldSend := d.callback(d, message); shouldSend { + select { + case d.outbox <- responseBytes: + return + default: + panic(errOutboxFull) + } } } @@ -449,7 +457,7 @@ func TestVehicleDropsReply(t *testing.T) { defer cancel() conn.Sleep() - dispatcher.sessions[testDomain] = nil + delete(dispatcher.sessions, testDomain) err := dispatcher.StartSession(ctx, testDomain) if err != context.DeadlineExceeded { t.Errorf("Expected timeout but got error: %s", err) @@ -626,6 +634,7 @@ func TestConnect(t *testing.T) { if err := dispatcher.Start(ctx); err != nil { t.Fatal(err) } + defer dispatcher.Stop() if err := dispatcher.StartSessions(ctx, nil); err != context.DeadlineExceeded { t.Fatalf("Unexpected error: %s", err) @@ -635,7 +644,7 @@ func TestConnect(t *testing.T) { defer cancel() if _, err := dispatcher.Send(ctx, testCommand(), connector.AuthMethodHMAC); err != protocol.ErrNoSession { - t.Errorf("Unexpected error: %s", err) + t.Fatalf("Unexpected error: %s", err) } ctx, cancel = context.WithTimeout(context.Background(), quiescentDelay) @@ -839,6 +848,82 @@ func TestRequestSessionWithoutKey(t *testing.T) { } } +func TestHandshakeWithoutKey(t *testing.T) { + conn := newDummyConnector(t) + defer conn.Close() + + dispatcher, err := New(conn, nil) + if err != nil { + t.Fatalf("Couldn't initialize dispatcher: %s", err) + } + + ctx, cancel := context.WithTimeout(context.Background(), quiescentDelay) + defer cancel() + + if err := dispatcher.Start(ctx); err != nil { + t.Fatal(err) + } + defer dispatcher.Stop() + + if err := dispatcher.StartSession(ctx, testDomain); !errors.Is(err, protocol.ErrRequiresKey) { + t.Errorf("Expected no key error but got %s", err) + } +} + +func TestNoValidHandshakeResponse(t *testing.T) { + conn := newDummyConnector(t) + defer conn.Close() + + key, err := authentication.NewECDHPrivateKey(rand.Reader) + if err != nil { + t.Fatal(err) + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + dispatcher, err := New(conn, key) + if err != nil { + t.Fatal(err) + } + if err := dispatcher.Start(ctx); err != nil { + t.Fatal(err) + } + defer dispatcher.Stop() + + const maxCallbacks = 5 + callbackCount := 0 + + conn.callback = func(d *dummyConnector, message *universal.RoutableMessage) ([]byte, bool) { + callbackCount++ // caller holds d.lock + reply := initReply(message) + reply.Payload = &universal.RoutableMessage_SessionInfo{} + reply.SubSigData = &universal.RoutableMessage_SignatureData{ + SignatureData: &signatures.SignatureData{ + SigType: &signatures.SignatureData_SessionInfoTag{ + SessionInfoTag: &signatures.HMAC_Signature_Data{ + Tag: []byte("swordfish"), + }, + }, + }, + } + if callbackCount == maxCallbacks { + reply.SignedMessageStatus = &universal.MessageStatus{ + SignedMessageFault: universal.MessageFault_E_MESSAGEFAULT_ERROR_UNKNOWN_KEY_ID, + } + } + encoded, err := proto.Marshal(reply) + if err != nil { + panic(err) + } + return encoded, true + } + + if err := dispatcher.StartSession(ctx, testDomain); !errors.Is(err, protocol.ErrKeyNotPaired) { + t.Errorf("Expected key not paired but got %s", err) + } +} + func TestCache(t *testing.T) { conn := newDummyConnector(t) key, err := authentication.NewECDHPrivateKey(rand.Reader) diff --git a/pkg/protocol/protobuf/universalmessage/universal_message.pb.go b/pkg/protocol/protobuf/universalmessage/universal_message.pb.go index c303197c..d39c8000 100644 --- a/pkg/protocol/protobuf/universalmessage/universal_message.pb.go +++ b/pkg/protocol/protobuf/universalmessage/universal_message.pb.go @@ -1,8 +1,3 @@ -//* -// This proto file defines the message protocol that is used to communicate from applications/clients to the car/servers -// -// - // Code generated by protoc-gen-go. DO NOT EDIT. // versions: // protoc-gen-go v1.28.1 From faa657a72d86c7e9c28f3b4f1902783aad7398a2 Mon Sep 17 00:00:00 2001 From: Seth Terashima Date: Fri, 1 Mar 2024 17:49:37 -0800 Subject: [PATCH 4/5] Allow configurable timeouts Timeouts were previously hard-coded. This change allows clients to specify timeouts dynamically. --- internal/dispatcher/dispatcher.go | 23 ++++++++++++++++++----- internal/dispatcher/dispatcher_test.go | 4 ++++ internal/dispatcher/receiver.go | 4 ++-- internal/dispatcher/session.go | 10 ++++++++-- pkg/connector/ble/ble.go | 7 ++++++- pkg/connector/connector.go | 14 ++++++++------ pkg/connector/inet/inet.go | 7 +++++++ pkg/vehicle/vehicle.go | 10 ++++++++++ pkg/vehicle/vehicle_test.go | 2 ++ 9 files changed, 65 insertions(+), 16 deletions(-) diff --git a/internal/dispatcher/dispatcher.go b/internal/dispatcher/dispatcher.go index 0ac591ef..ad6a9ea2 100644 --- a/internal/dispatcher/dispatcher.go +++ b/internal/dispatcher/dispatcher.go @@ -18,9 +18,6 @@ import ( universal "github.com/teslamotors/vehicle-command/pkg/protocol/protobuf/universalmessage" ) -var sessionInfoRequestTimeout = 5 * time.Second -var commandTimeout = 5 * time.Second - // Dispatcher objects send (encrypted) messages to a vehicle and route incoming messages to the // appropriate receiver object. type Dispatcher struct { @@ -28,6 +25,9 @@ type Dispatcher struct { privateKey authentication.ECDHPrivateKey address []byte + latencyLock sync.Mutex + maxLatency time.Duration + doneLock sync.Mutex terminate chan struct{} done chan bool @@ -43,6 +43,7 @@ type Dispatcher struct { func New(conn connector.Connector, privateKey authentication.ECDHPrivateKey) (*Dispatcher, error) { dispatcher := Dispatcher{ conn: conn, + maxLatency: conn.AllowedLatency(), address: make([]byte, addressLength), sessions: make(map[universal.Domain]*session), handlers: make(map[receiverKey]*receiver), @@ -55,6 +56,14 @@ func New(conn connector.Connector, privateKey authentication.ECDHPrivateKey) (*D return &dispatcher, nil } +func (d *Dispatcher) SetMaxLatency(latency time.Duration) { + if latency > 0 { + d.latencyLock.Lock() + d.maxLatency = latency + d.latencyLock.Unlock() + } +} + // RetryInterval fetches the transport-layer dependent recommended delay between retry attempts. func (d *Dispatcher) RetryInterval() time.Duration { return d.conn.RetryInterval() @@ -167,8 +176,12 @@ func (d *Dispatcher) checkForSessionUpdate(message *universal.RoutableMessage, h return } - if handler.expired() { - log.Warning("[%02x] Discarding session info because it was received more than %s after request", message.GetRequestUuid(), sessionInfoRequestTimeout) + d.latencyLock.Lock() + maxLatency := d.maxLatency + d.latencyLock.Unlock() + + if handler.expired(maxLatency) { + log.Warning("[%02x] Discarding session info because it was received more than %s after request", message.GetRequestUuid(), maxLatency) return } diff --git a/internal/dispatcher/dispatcher_test.go b/internal/dispatcher/dispatcher_test.go index 14c3d5a4..2069994a 100644 --- a/internal/dispatcher/dispatcher_test.go +++ b/internal/dispatcher/dispatcher_test.go @@ -137,6 +137,10 @@ func (d *dummyConnector) domainKey(domain universal.Domain) authentication.ECDHP return d.keys[domain] } +func (d *dummyConnector) AllowedLatency() time.Duration { + return time.Second +} + func (d *dummyConnector) RetryInterval() time.Duration { return time.Millisecond } diff --git a/internal/dispatcher/receiver.go b/internal/dispatcher/receiver.go index 0febe93b..4b4170a3 100644 --- a/internal/dispatcher/receiver.go +++ b/internal/dispatcher/receiver.go @@ -49,6 +49,6 @@ func (r *receiver) Close() { // expired returns true if the request was sent long enough ago that any included session info // should be discarded as stale. -func (r *receiver) expired() bool { - return time.Now().After(r.requestSentAt.Add(sessionInfoRequestTimeout)) +func (r *receiver) expired(lifetime time.Duration) bool { + return time.Now().After(r.requestSentAt.Add(lifetime)) } diff --git a/internal/dispatcher/session.go b/internal/dispatcher/session.go index e1515f2f..2173f875 100644 --- a/internal/dispatcher/session.go +++ b/internal/dispatcher/session.go @@ -12,6 +12,8 @@ import ( universal "github.com/teslamotors/vehicle-command/pkg/protocol/protobuf/universalmessage" ) +var defaultExpiration = 5 * time.Second + // CacheEntry contains information that allows a vehicle session to be resumed without a handshake // mesasge (SessionInfoRequest). type CacheEntry struct { @@ -44,6 +46,10 @@ func NewSession(private authentication.ECDHPrivateKey, vin string) (*session, er func (s *session) Authorize(ctx context.Context, command *universal.RoutableMessage, method connector.AuthMethod) error { var err error + lifetime := defaultExpiration + if deadline, ok := ctx.Deadline(); ok { + lifetime = time.Until(deadline) + } for { attempted := false select { @@ -56,9 +62,9 @@ func (s *session) Authorize(ctx context.Context, command *universal.RoutableMess case connector.AuthMethodNone: err = nil case connector.AuthMethodGCM: - err = s.ctx.Encrypt(command, commandTimeout) + err = s.ctx.Encrypt(command, lifetime) case connector.AuthMethodHMAC: - err = s.ctx.AuthorizeHMAC(command, commandTimeout) + err = s.ctx.AuthorizeHMAC(command, lifetime) default: return errors.New("unrecognized authentication method") } diff --git a/pkg/connector/ble/ble.go b/pkg/connector/ble/ble.go index 38ee5f85..ba931acb 100644 --- a/pkg/connector/ble/ble.go +++ b/pkg/connector/ble/ble.go @@ -17,7 +17,8 @@ import ( const maxBLEMessageSize = 1024 var ( - rxTimeout = time.Second + rxTimeout = time.Second // Timeout interval between receiving chunks of a mesasge + maxLatency = 4 * time.Second // Max allowed error when syncing vehicle clock ) var ( @@ -81,6 +82,10 @@ func (c *Connection) Close() { c.client.CancelConnection() } +func (c *Connection) AllowedLatency() time.Duration { + return maxLatency +} + func (c *Connection) rx(p []byte) { if time.Since(c.lastRx) > rxTimeout { c.inputBuffer = []byte{} diff --git a/pkg/connector/connector.go b/pkg/connector/connector.go index 726dbf2a..13d18109 100644 --- a/pkg/connector/connector.go +++ b/pkg/connector/connector.go @@ -34,12 +34,10 @@ type Connector interface { // Send sends a buffer to the vehicle. // - // Depending on the error, the vehicle may have received and even acted on - // the message. For some errors, such as network timeouts, the client will - // not be able to determine if this is the case. If the returned error - // implements the vehicle.Error interface, then the client may be able - // to determine if the message was received by using the appropriate - // methods. + // Depending on the error, the vehicle may have received and even acted on the message. For some + // errors, such as network timeouts, the client will not be able to determine if this is the + // case. If the returned error implements the vehicle.Error interface, then the client may be + // able to determine if the message was received by using the appropriate methods. // // Implementations must be thread safe. Send(ctx context.Context, buffer []byte) error @@ -59,6 +57,10 @@ type Connector interface { // RetryInterval returns the recommended wait time between transmission attempts. RetryInterval() time.Duration + + // AllowedLatency returns the maximum permitted delay between sending a request and receiving a + // response with an updated vehicle clock. + AllowedLatency() time.Duration } // FleetAPIConnector is a superset of Connector (which sends datagrams to vehicles) that also allows diff --git a/pkg/connector/inet/inet.go b/pkg/connector/inet/inet.go index a60f26bc..7fc6e6b9 100644 --- a/pkg/connector/inet/inet.go +++ b/pkg/connector/inet/inet.go @@ -18,6 +18,9 @@ import ( "github.com/teslamotors/vehicle-command/pkg/protocol" ) +// MaxLatency is the default maximum latency permitted when updating the vehicle clock estimate. +var MaxLatency = 10 * time.Second + func readWithContext(ctx context.Context, r io.Reader, p []byte) ([]byte, error) { bytesRead := 0 for { @@ -183,6 +186,10 @@ func (c *Connection) PreferredAuthMethod() connector.AuthMethod { return connector.AuthMethodHMAC } +func (c *Connection) AllowedLatency() time.Duration { + return MaxLatency +} + func (c *Connection) RetryInterval() time.Duration { return time.Second } diff --git a/pkg/vehicle/vehicle.go b/pkg/vehicle/vehicle.go index 3e8b2101..dc23c80f 100644 --- a/pkg/vehicle/vehicle.go +++ b/pkg/vehicle/vehicle.go @@ -52,6 +52,9 @@ type sender interface { // Returns the recommended retransmission interval for the Connector RetryInterval() time.Duration + + // Sets the maximum allowed clock error. + SetMaxLatency(time.Duration) } // A Vehicle represents a Tesla vehicle. @@ -59,6 +62,7 @@ type Vehicle struct { dispatcher sender Flags uint32 vin string + conn connector.Connector authMethod connector.AuthMethod @@ -89,6 +93,12 @@ func NewVehicle(conn connector.Connector, privateKey authentication.ECDHPrivateK return vehicle, nil } +// SetMaxLatency sets the threshold used by the client to discard clock-synchronization messages +// from the vehicle that take too long to arrive. +func (v *Vehicle) SetMaxLatency(latency time.Duration) { + v.dispatcher.SetMaxLatency(latency) +} + func (v *Vehicle) VIN() string { return v.vin } diff --git a/pkg/vehicle/vehicle_test.go b/pkg/vehicle/vehicle_test.go index cc4abb9a..7bb5379f 100644 --- a/pkg/vehicle/vehicle_test.go +++ b/pkg/vehicle/vehicle_test.go @@ -89,6 +89,8 @@ func (s *testSender) EnqueueResponse(t *testing.T, message *universal.RoutableMe } } +func (s *testSender) SetMaxLatency(latency time.Duration) {} + func newTestVehicle() (*Vehicle, *testSender) { dispatch := newTestSender() return &Vehicle{dispatcher: dispatch}, dispatch From dd3584d1fb2376c951b68d7dafca5116cb03926a Mon Sep 17 00:00:00 2001 From: Seth Terashima Date: Thu, 7 Mar 2024 16:10:14 -0800 Subject: [PATCH 5/5] Add timeout command-line options Adds timeout command-line options that override the defaults. --- cmd/tesla-control/main.go | 22 +++++++++++++--------- cmd/tesla-http-proxy/main.go | 4 ++++ pkg/proxy/proxy.go | 4 ++-- 3 files changed, 19 insertions(+), 11 deletions(-) diff --git a/cmd/tesla-control/main.go b/cmd/tesla-control/main.go index cc812671..cb670ac9 100644 --- a/cmd/tesla-control/main.go +++ b/cmd/tesla-control/main.go @@ -56,8 +56,8 @@ func Usage() { } } -func runCommand(acct *account.Account, car *vehicle.Vehicle, args []string) int { - ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) +func runCommand(acct *account.Account, car *vehicle.Vehicle, args []string, timeout time.Duration) int { + ctx, cancel := context.WithTimeout(context.Background(), timeout) defer cancel() if err := execute(ctx, acct, car, args); err != nil { @@ -73,7 +73,7 @@ func runCommand(acct *account.Account, car *vehicle.Vehicle, args []string) int return 0 } -func runInteractiveShell(acct *account.Account, car *vehicle.Vehicle) int { +func runInteractiveShell(acct *account.Account, car *vehicle.Vehicle, timeout time.Duration) int { scanner := bufio.NewScanner(os.Stdin) for fmt.Printf("> "); scanner.Scan(); fmt.Printf("> ") { args, err := shlex.Split(scanner.Text()) @@ -87,7 +87,7 @@ func runInteractiveShell(acct *account.Account, car *vehicle.Vehicle) int { writeErr("Invalid command: %s", err) continue } - runCommand(acct, car, args) + runCommand(acct, car, args, timeout) } if err := scanner.Err(); err != nil { writeErr("Error reading command: %s", err) @@ -103,8 +103,10 @@ func main() { }() var ( - debug bool - forceBLE bool + debug bool + forceBLE bool + commandTimeout time.Duration + connTimeout time.Duration ) config, err := cli.NewConfig(cli.FlagAll) if err != nil { @@ -114,6 +116,8 @@ func main() { flag.Usage = Usage flag.BoolVar(&debug, "debug", false, "Enable verbose debugging messages") flag.BoolVar(&forceBLE, "ble", false, "Force BLE connection even if OAuth environment variables are defined") + flag.DurationVar(&commandTimeout, "command-timeout", 5*time.Second, "Set timeout for commands sent to the vehicle.") + flag.DurationVar(&connTimeout, "connect-timeout", 20*time.Second, "Set timeout for establishing initial connection.") config.RegisterCommandLineFlags() flag.Parse() @@ -150,7 +154,7 @@ func main() { return } - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), connTimeout) defer cancel() acct, car, err := config.Connect(ctx) @@ -171,8 +175,8 @@ func main() { } if flag.NArg() > 0 { - status = runCommand(acct, car, flag.Args()) + status = runCommand(acct, car, flag.Args(), commandTimeout) } else { - status = runInteractiveShell(acct, car) + status = runInteractiveShell(acct, car, commandTimeout) } } diff --git a/cmd/tesla-http-proxy/main.go b/cmd/tesla-http-proxy/main.go index 80e12390..9fbac93f 100644 --- a/cmd/tesla-http-proxy/main.go +++ b/cmd/tesla-http-proxy/main.go @@ -7,6 +7,7 @@ import ( "fmt" "net/http" "os" + "time" "github.com/teslamotors/vehicle-command/internal/log" "github.com/teslamotors/vehicle-command/pkg/cli" @@ -43,6 +44,7 @@ func main() { verbose bool host string port int + timeout time.Duration ) config, err := cli.NewConfig(cli.FlagPrivateKey) @@ -64,6 +66,7 @@ func main() { flag.BoolVar(&verbose, "verbose", false, "Enable verbose logging") flag.StringVar(&host, "host", "localhost", "Proxy server `hostname`") flag.IntVar(&port, "port", defaultPort, "`Port` to listen on") + flag.DurationVar(&timeout, "timeout", proxy.DefaultTimeout, "Timeout interval when sending commands") flag.Usage = Usage config.RegisterCommandLineFlags() flag.Parse() @@ -97,6 +100,7 @@ func main() { if err != nil { return } + p.Timeout = timeout addr := fmt.Sprintf("%s:%d", host, port) log.Info("Listening on %s", addr) diff --git a/pkg/proxy/proxy.go b/pkg/proxy/proxy.go index 20a32257..a71c764c 100644 --- a/pkg/proxy/proxy.go +++ b/pkg/proxy/proxy.go @@ -22,7 +22,7 @@ import ( ) const ( - defaultTimeout = 10 * time.Second + DefaultTimeout = 10 * time.Second maxRequestBodyBytes = 512 vinLength = 17 proxyProtocolVersion = "tesla-http-proxy/1.0.0" @@ -90,7 +90,7 @@ func (p *Proxy) unlockVIN(vin string) { // command-authentication key, not a TLS key.) func New(ctx context.Context, skey protocol.ECDHPrivateKey, cacheSize int) (*Proxy, error) { return &Proxy{ - Timeout: defaultTimeout, + Timeout: DefaultTimeout, commandKey: skey, sessions: cache.New(cacheSize), }, nil