Skip to content

Commit

Permalink
Fixes a SharedDirectoryAnnounce incompatibility (#25931)
Browse files Browse the repository at this point in the history
* Fixes a SharedDirectoryAnnounce incompatibility

The SharedDirectoryAnnounce message was changed to remove an unnecessary
completionID field, however this change caused an incompatibility between
the proxy and previous versions of the wds. This commit reverts that specific
change and adds notes to the code explaining the situation.

The original change is here: https://github.com/gravitational/teleport/pull/25260/files#diff-98a4bee57beb7f007614e4810d2cf8413bddf48484c8aad6dd0756a218797c36R677-R678

* changes completionId to discard

* adds discard bit to sda.Encode to make fuzz tests happy
  • Loading branch information
ibeckermayer committed May 10, 2023
1 parent 2568bff commit e42b072
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 4 deletions.
12 changes: 11 additions & 1 deletion lib/srv/desktop/tdp/proto.go
Original file line number Diff line number Diff line change
Expand Up @@ -667,6 +667,8 @@ type SharedDirectoryAnnounce struct {
func (s SharedDirectoryAnnounce) Encode() ([]byte, error) {
buf := new(bytes.Buffer)
buf.WriteByte(byte(TypeSharedDirectoryAnnounce))
// TODO(isaiah): The discard here allows fuzz tests to succeed, it should eventually be done away with.
writeUint32(buf, 0) // discard
writeUint32(buf, s.DirectoryID)
if err := encodeString(buf, s.Name); err != nil {
return nil, trace.Wrap(err)
Expand All @@ -675,8 +677,16 @@ func (s SharedDirectoryAnnounce) Encode() ([]byte, error) {
}

func decodeSharedDirectoryAnnounce(in io.Reader) (SharedDirectoryAnnounce, error) {
// TODO(isaiah): The discard here is a copy-paste error, but we need to keep it
// for now in order that the proxy stay compatible with previous versions of the wds.
var discard uint32
err := binary.Read(in, binary.BigEndian, &discard)
if err != nil {
return SharedDirectoryAnnounce{}, trace.Wrap(err)
}

var directoryID uint32
err := binary.Read(in, binary.BigEndian, &directoryID)
err = binary.Read(in, binary.BigEndian, &directoryID)
if err != nil {
return SharedDirectoryAnnounce{}, trace.Wrap(err)
}
Expand Down
2 changes: 1 addition & 1 deletion web/packages/teleport/src/lib/tdp/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,7 @@ export default class Client extends EventEmitterWebAuthnSender {
name = this.sdManager.getName();
this.send(
this.codec.encodeSharedDirectoryAnnounce({
completionId: 0, // This is always the first request.
discard: 0, // This is always the first request.
// Hardcode directoryId for now since we only support sharing 1 directory.
// We're using 2 because the smartcard device is hardcoded to 1 in the backend.
directoryId: 2,
Expand Down
8 changes: 6 additions & 2 deletions web/packages/teleport/src/lib/tdp/codec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,10 @@ export type MfaJson = {
};

// | message type (11) | completion_id uint32 | directory_id uint32 | name_length uint32 | name []byte |
// TODO(isaiah): The discard here is a copy-paste error, but we need to keep it
// for now in order that the proxy stay compatible with previous versions of the wds.
export type SharedDirectoryAnnounce = {
completionId: number;
discard: number;
directoryId: number;
name: string;
};
Expand Down Expand Up @@ -601,7 +603,9 @@ export default class Codec {
let offset = 0;

view.setUint8(offset++, MessageType.SHARED_DIRECTORY_ANNOUNCE);
view.setUint32(offset, sharedDirAnnounce.completionId);
// TODO(isaiah): The discard here is a copy-paste error, but we need to keep it
// for now in order that the proxy stay compatible with previous versions of the wds.
view.setUint32(offset, sharedDirAnnounce.discard);
offset += uint32Length;
view.setUint32(offset, sharedDirAnnounce.directoryId);
offset += uint32Length;
Expand Down

0 comments on commit e42b072

Please sign in to comment.