Repository navigation
fix(ipc): bind identity in dispatch (PR-EIPC-2) #56
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,174 @@ | ||
| // SPDX-License-Identifier: MIT | ||
| // Copyright (c) 2026 EoS Project | ||
|
|
||
| package core | ||
|
|
||
| import ( | ||
| "errors" | ||
| "testing" | ||
|
|
||
| "github.com/embeddedos-org/eipc/protocol" | ||
| ) | ||
|
|
||
| // TestServerEndpointBindPeer_AcceptsGenuineMessage proves a bound endpoint | ||
| // accepts a message whose Source and SessionID match the peer, including | ||
| // the client-side session stamping (ClientEndpoint stamps its session | ||
| // token when the message carries none). | ||
| func TestServerEndpointBindPeer_AcceptsGenuineMessage(t *testing.T) { | ||
| clientConn, serverConn := setupTCPPair(t) | ||
| defer clientConn.Close() | ||
| defer serverConn.Close() | ||
|
|
||
| hmacKey := []byte("test-secret-key-for-hmac-256bit!") | ||
| c := protocol.DefaultCodec() | ||
|
|
||
| clientEP := NewClientEndpoint(clientConn, c, hmacKey, "tok-abc123") | ||
| serverEP := NewServerEndpoint(serverConn, c, hmacKey) | ||
| serverEP.BindPeer(&AuthenticatedPeer{ | ||
| ServiceID: "ebot.client", | ||
| SessionToken: "tok-abc123", | ||
| Capabilities: []string{"ai:chat"}, | ||
| }) | ||
|
|
||
| msgCh := make(chan Message, 1) | ||
| errCh := make(chan error, 1) | ||
| go func() { | ||
| received, err := serverEP.Receive() | ||
| if err != nil { | ||
| errCh <- err | ||
| return | ||
| } | ||
| msgCh <- received | ||
| }() | ||
|
|
||
| // No SessionID set on the message: the client endpoint stamps it. | ||
| msg := NewMessage(TypeChat, "ebot.client", []byte(`{"prompt":"hi"}`)) | ||
| if err := clientEP.Send(msg); err != nil { | ||
| t.Fatalf("Send failed: %v", err) | ||
| } | ||
|
|
||
| select { | ||
| case received := <-msgCh: | ||
| if received.Source != "ebot.client" { | ||
| t.Errorf("source mismatch: got %q", received.Source) | ||
| } | ||
| if received.SessionID != "tok-abc123" { | ||
| t.Errorf("session was not stamped/bound: got %q", received.SessionID) | ||
| } | ||
| case err := <-errCh: | ||
| t.Fatalf("Receive rejected a genuine message: %v", err) | ||
| } | ||
| } | ||
|
|
||
| // TestServerEndpointBindPeer_RejectsForgedSource proves a bound endpoint | ||
| // drops a message that claims another service's identity, even when the | ||
| // session token is correct. | ||
| func TestServerEndpointBindPeer_RejectsForgedSource(t *testing.T) { | ||
| clientConn, serverConn := setupTCPPair(t) | ||
| defer clientConn.Close() | ||
| defer serverConn.Close() | ||
|
|
||
| hmacKey := []byte("test-secret-key-for-hmac-256bit!") | ||
| c := protocol.DefaultCodec() | ||
|
|
||
| clientEP := NewClientEndpoint(clientConn, c, hmacKey, "tok-abc123") | ||
| serverEP := NewServerEndpoint(serverConn, c, hmacKey) | ||
| serverEP.BindPeer(&AuthenticatedPeer{ | ||
| ServiceID: "ebot.client", | ||
| SessionToken: "tok-abc123", | ||
| }) | ||
|
|
||
| errCh := make(chan error, 1) | ||
| go func() { | ||
| _, err := serverEP.Receive() | ||
| errCh <- err | ||
| }() | ||
|
|
||
| msg := NewMessage(TypeChat, "eipc-server", []byte(`{}`)) | ||
| msg.SessionID = "tok-abc123" | ||
| if err := clientEP.Send(msg); err != nil { | ||
| t.Fatalf("Send failed: %v", err) | ||
| } | ||
|
|
||
| if err := <-errCh; !errors.Is(err, ErrAuth) { | ||
| t.Fatalf("expected ErrAuth for forged source, got %v", err) | ||
| } | ||
| } | ||
|
|
||
| // TestServerEndpointBindPeer_RejectsWrongSession proves a bound endpoint | ||
| // drops a message from the right service carrying another session's token. | ||
| func TestServerEndpointBindPeer_RejectsWrongSession(t *testing.T) { | ||
| clientConn, serverConn := setupTCPPair(t) | ||
| defer clientConn.Close() | ||
| defer serverConn.Close() | ||
|
|
||
| hmacKey := []byte("test-secret-key-for-hmac-256bit!") | ||
| c := protocol.DefaultCodec() | ||
|
|
||
| clientEP := NewClientEndpoint(clientConn, c, hmacKey, "tok-other-session") | ||
| serverEP := NewServerEndpoint(serverConn, c, hmacKey) | ||
| serverEP.BindPeer(&AuthenticatedPeer{ | ||
| ServiceID: "ebot.client", | ||
| SessionToken: "tok-abc123", | ||
| }) | ||
|
|
||
| errCh := make(chan error, 1) | ||
| go func() { | ||
| _, err := serverEP.Receive() | ||
| errCh <- err | ||
| }() | ||
|
|
||
| msg := NewMessage(TypeChat, "ebot.client", []byte(`{}`)) | ||
| msg.SessionID = "tok-other-session" | ||
| if err := clientEP.Send(msg); err != nil { | ||
| t.Fatalf("Send failed: %v", err) | ||
| } | ||
|
|
||
| if err := <-errCh; !errors.Is(err, ErrAuth) { | ||
| t.Fatalf("expected ErrAuth for wrong session, got %v", err) | ||
| } | ||
| } | ||
|
|
||
| // TestServerEndpointBindPeer_ClearsOnNil proves BindPeer(nil) removes the | ||
| // binding and restores the previous unbound behavior. | ||
| func TestServerEndpointBindPeer_ClearsOnNil(t *testing.T) { | ||
| clientConn, serverConn := setupTCPPair(t) | ||
| defer clientConn.Close() | ||
| defer serverConn.Close() | ||
|
|
||
| hmacKey := []byte("test-secret-key-for-hmac-256bit!") | ||
| c := protocol.DefaultCodec() | ||
|
|
||
| serverEP := NewServerEndpoint(serverConn, c, hmacKey) | ||
| serverEP.BindPeer(&AuthenticatedPeer{ServiceID: "ebot.client", SessionToken: "tok-abc123"}) | ||
| if serverEP.BoundPeer() == nil { | ||
| t.Fatal("expected a bound peer") | ||
| } | ||
| serverEP.BindPeer(nil) | ||
| if serverEP.BoundPeer() != nil { | ||
| t.Fatal("expected the binding to be cleared") | ||
| } | ||
|
|
||
| msgCh := make(chan Message, 1) | ||
| errCh := make(chan error, 1) | ||
| go func() { | ||
| received, err := serverEP.Receive() | ||
| if err != nil { | ||
| errCh <- err | ||
| return | ||
| } | ||
| msgCh <- received | ||
| }() | ||
|
|
||
| clientEP := NewClientEndpoint(clientConn, c, hmacKey, "tok-abc123") | ||
| msg := NewMessage(TypeChat, "anyone", []byte(`{}`)) | ||
| if err := clientEP.Send(msg); err != nil { | ||
| t.Fatalf("Send failed: %v", err) | ||
| } | ||
|
|
||
| select { | ||
| case <-msgCh: | ||
| case err := <-errCh: | ||
| t.Fatalf("unbound endpoint rejected a message: %v", err) | ||
| } | ||
| } | ||
|
Comment on lines
+22
to
+174
There was a problem hiding this comment. ⚪ Low · Secrets appear in test fixtures The added tests commit session-token literals and hard-coded HMAC/authentication keys in fixtures, contrary to the rule against placing tokens and keys in test fixtures. Avoid embedding token and key values in test fixtures; use non-secret test data that does not represent credentials. Also at Prompt for AI agentsIn `core/endpoint_identity_test.go` at lines 22-174:
**Secrets appear in test fixtures**
The added tests commit session-token literals and hard-coded HMAC/authentication keys in fixtures, contrary to the rule against placing tokens and keys in test fixtures.
Avoid embedding token and key values in test fixtures; use non-secret test data that does not represent credentials.
Also at `core/identity_test.go:14-60`, `services/broker/broker_identity_test.go:16-123`, `services/health/health_identity_test.go:16-53`, `security/auth/identity_binding_test.go:13-59`.From - Never commit a credential, token, key or connection string.
- Never log one, and never return one in an error message or stack trace.
- Never place one in a test fixture, a comment, or an example. |
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| // SPDX-License-Identifier: MIT | ||
| // Copyright (c) 2026 EoS Project | ||
|
|
||
| package core | ||
|
|
||
| import "fmt" | ||
|
|
||
| // AuthenticatedPeer is the core-level view of a peer that has completed | ||
| // challenge-response authentication (see security/auth.Authenticator). | ||
| // It is the identity that dispatch binds a message to: a message is only | ||
| // dispatched for the peer it claims to come from when its envelope | ||
| // (Source + SessionID) matches this peer and its Capability is granted. | ||
| // | ||
| // The type lives in core rather than security/auth so the router, broker, | ||
| // health service and endpoints can enforce the binding without an import | ||
| // cycle (security/auth already imports core). | ||
| type AuthenticatedPeer struct { | ||
| // ServiceID is the authenticated service identity (e.g. "ebot.client"). | ||
| ServiceID string | ||
| // SessionToken is the session token issued at authentication time. | ||
| // Senders carry it as Message.SessionID. | ||
| SessionToken string | ||
| // Capabilities granted to this peer at authentication time. | ||
| Capabilities []string | ||
| } | ||
|
|
||
| // ValidateIdentity checks the envelope identity of msg against the | ||
| // authenticated peer. A nil peer means "no peer bound" (unsecured/local | ||
| // path) and passes every message; callers that require authentication | ||
| // must not pass nil. | ||
| // | ||
| // When a peer is bound the check is fail-closed: | ||
| // - msg.Source must be present and equal the peer's ServiceID, so a | ||
| // peer cannot impersonate another service by forging Source; | ||
| // - msg.SessionID must be present and equal the peer's SessionToken, | ||
| // so an envelope from another session is rejected. | ||
| func (p *AuthenticatedPeer) ValidateIdentity(msg Message) error { | ||
| if p == nil { | ||
| return nil | ||
| } | ||
| if msg.Source == "" { | ||
| return fmt.Errorf("%w: message has no source", ErrAuth) | ||
| } | ||
| if msg.Source != p.ServiceID { | ||
| return fmt.Errorf("%w: source %q is not the authenticated peer %q", ErrAuth, msg.Source, p.ServiceID) | ||
| } | ||
| if msg.SessionID == "" { | ||
| return fmt.Errorf("%w: message from %q carries no session", ErrAuth, msg.Source) | ||
| } | ||
| if msg.SessionID != p.SessionToken { | ||
| return fmt.Errorf("%w: session mismatch for %q", ErrAuth, msg.Source) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // ValidateMessage is ValidateIdentity plus capability validation: when | ||
| // msg.Capability is set it must be one of the peer's granted | ||
| // capabilities. An empty Capability passes (the message claims no | ||
| // capability-gated action). | ||
| func (p *AuthenticatedPeer) ValidateMessage(msg Message) error { | ||
| if err := p.ValidateIdentity(msg); err != nil { | ||
| return err | ||
| } | ||
| if p == nil || msg.Capability == "" { | ||
| return nil | ||
| } | ||
| for _, c := range p.Capabilities { | ||
| if c == msg.Capability { | ||
| return nil | ||
| } | ||
| } | ||
| return fmt.Errorf("%w: peer %q lacks capability %q", ErrCapability, p.ServiceID, msg.Capability) | ||
| } | ||
|
Comment on lines
+37
to
+73
There was a problem hiding this comment. ⚪ Low · New API docs omit contracts Callers reading these new exported methods get a brief description but no parameter and return-value documentation or runnable example, leaving their public contracts incomplete. Document each new exported method's parameters and return value and add a runnable example. Also at Prompt for AI agentsIn `core/identity.go` at lines 37-73:
**New API docs omit contracts**
Callers reading these new exported methods get a brief description but no parameter and return-value documentation or runnable example, leaving their public contracts incomplete.
Document each new exported method's parameters and return value and add a runnable example.
Also at `core/endpoint.go:146-170`, `core/router.go:56-70`, `services/broker/broker.go:129-155`, `services/health/health.go:66`, `security/auth/identity.go:175`.From Every public function carries a description, its parameters, its return value,
and an example. The example must be one that was actually run. |
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟠 High · Service identity is forgeable
When a client that knows the shared HMAC key requests authentication as a different known service,
CreateChallengeandVerifyResponsetrust the client-supplied service ID and verify the proof with the same global shared secret for every service.peer.BoundPeer()therefore binds the claimed identity and its capabilities, letting the client use capabilities granted to another service.Authenticate each service with a distinct credential, and bind the resulting peer identity only to the service proven by that credential.
Prompt for AI agents