Conversation
WalkthroughAdds JS-facing Telnet bindings and a public TelnetClient plus a new telnetmini utility library (Telnet I/O, ENCRYPT detection, NTLM/TNAP builders/parsers) and an SMB/NTLM packet toolkit; includes a JS integration test launching a telnetd Docker container for authentication/NTLM probing. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor JS_User
participant TelnetClient_JS as TelnetClient (JS)
participant Dialer as Dialer/ACL
participant Target as Telnet Server
Note over TelnetClient_JS: Connect(ctx, host, port, user, pass)
JS_User->>TelnetClient_JS: Connect(...)
TelnetClient_JS->>Dialer: allowlist check + dial(host:port)
Dialer-->>TelnetClient_JS: net.Conn
TelnetClient_JS->>Target: Telnet login flow (prompts, credentials)
Target-->>TelnetClient_JS: Banner / Prompts / Shell prompt
TelnetClient_JS-->>JS_User: (success bool, error)
sequenceDiagram
autonumber
actor JS_User
participant TelnetClient_JS as TelnetClient (JS)
participant Target as Telnet Server
Note over TelnetClient_JS: Info(ctx, host, port)
JS_User->>TelnetClient_JS: Info(...)
TelnetClient_JS->>Target: Probe ENCRYPT via IAC negotiation
Target-->>TelnetClient_JS: Banner + IAC options
TelnetClient_JS-->>JS_User: TelnetInfoResponse {SupportsEncryption, Banner, Options}
sequenceDiagram
autonumber
actor JS_User
participant TelnetClient_JS as TelnetClient (JS)
participant telnetmini as telnetmini library
participant Target as Telnet Server
Note over TelnetClient_JS: GetTelnetNTLMInfo(ctx, host, port)
JS_User->>TelnetClient_JS: GetTelnetNTLMInfo(...)
TelnetClient_JS->>telnetmini: CreateTNAPLoginPacket()
TelnetClient_JS->>Target: Send TNAP/NTLM Negotiate packet
Target-->>TelnetClient_JS: NTLM Type-2 challenge (raw)
TelnetClient_JS->>telnetmini: ParseNTLMResponse(raw)
telnetmini-->>TelnetClient_JS: NTLMInfoResponse
TelnetClient_JS-->>JS_User: NTLMInfoResponse
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.go📄 CodeRabbit inference engine (CLAUDE.md)
Files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (6)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (10)
pkg/utils/telnetmini/doc.go (1)
1-7: Polish package doc: tighten wording and capitalize consistently
- Start the second line with a capital letter and avoid the dangling "it supports".
- Consider merging the bullets into a single paragraph for godoc brevity, or keep bullets but align their phrasing.
Proposed tweak:
-// it supports +// It supports:pkg/utils/telnetmini/ntlm.go (2)
85-97: Misleading name: CalculateTimestampSkew returns Unix timestamp, not skewThe function returns the Unix timestamp derived from the NTLM timestamp. Either rename to NTLMTimestampToUnix or compute and return the actual skew (now - unixTimestamp).
Two options:
-// CalculateTimestampSkew calculates the time skew from NTLM timestamp -func CalculateTimestampSkew(ntlmTimestamp uint64) int64 { +// NTLMTimestampToUnix converts NTLM 100ns ticks since 1601 to Unix seconds. +func NTLMTimestampToUnix(ntlmTimestamp uint64) int64 {Or keep name and compute skew:
- unixTimestamp := int64(ntlmTimestamp/10000000) - 11644473600 - return unixTimestamp + unixTimestamp := int64(ntlmTimestamp/10000000) - 11644473600 + return time.Now().Unix() - unixTimestamp
181-191: Use bitwise OR for NTLM flags, not additionFlags are bitfields. Addition works only accidentally when bits don’t overlap. Use bitwise OR to communicate intent and safety.
-flags := uint32(0x00000001 + // Negotiate Unicode - 0x00000002 + // Negotiate OEM strings - 0x00000004 + // Request Target - 0x00000200 + // Negotiate NTLM - 0x00008000 + // Negotiate Always Sign - 0x00080000 + // Negotiate NTLM2 Key - 0x20000000 + // Negotiate 128 - 0x80000000) // Negotiate 56 +flags := uint32(0x00000001 | // Negotiate Unicode + 0x00000002 | // Negotiate OEM strings + 0x00000004 | // Request Target + 0x00000200 | // Negotiate NTLM + 0x00008000 | // Negotiate Always Sign + 0x00080000 | // Negotiate NTLM2 Key + 0x20000000 | // Negotiate 128 + 0x80000000) // Negotiate 56pkg/utils/telnetmini/telnet.go (1)
49-69: Reasonable defaults: add “Password:” capitalized and common promptsMinor but useful: include "Password:" and "Login:" variants in defaults to reduce case sensitivity edge cases where servers don’t lowercase prompts (we lowercase on compare, but vary punctuation/spacing).
- c.LoginPrompts = []string{"login:", "username:"} + c.LoginPrompts = []string{"login:", "Login:", "username:", "Username:"} @@ - c.PasswordPrompts = []string{"password:"} + c.PasswordPrompts = []string{"password:", "Password:"}pkg/js/libs/telnet/telnet.go (2)
10-14: Avoid import alias shadowing the current package nameYou import
services/telnetastelnetinside packagetelnet. This is legal but confusing. Use an alias likefpxTelnetto avoid ambiguity.- "github.com/praetorian-inc/fingerprintx/pkg/plugins/services/telnet" + fpxTelnet "github.com/praetorian-inc/fingerprintx/pkg/plugins/services/telnet"And update
telnet.TELNETPlugin{}tofpxTelnet.TELNETPlugin{}below.
16-30: Drop unused telnet constants from JS bridge or explain their API contractThese constants are duplicated from telnetmini and unused in this file. Keeping them risks drift. If they are part of the JS public surface, document that; otherwise, remove.
pkg/utils/telnetmini/smb.go (4)
118-151: SMB header field layout mostly fine; consider filling Signature explicitlyNot critical, but writing the 8-byte Signature explicitly (as zeros) avoids confusion. Current code relies on zeroed slice but skips indices [14:22]. Optional.
618-632: Placeholders for LM/NT responses could lead to confusionReturning zeroed 24-byte arrays is fine for scaffolding, but call that out clearly in doc comments, and ensure no production flow sends these as real auth. Consider renaming to
StubLMResponse/StubNTResponseor returning an error until implemented.
697-724: ParseSMBResponse ignores NetBIOS Session header; ensure callers strip itIf callers pass NetBIOS-wrapped messages, the first byte will be 0x00 (NetBIOS type), not 0xFF. Either strip NetBIOS before calling, or detect and skip the first 4 bytes here.
- if len(data) < 32 { + if len(data) < 32 { return nil, fmt.Errorf("SMB response too short: %d bytes", len(data)) } - header := &SMBHeader{} - // Check protocol ID - if data[0] != 0xFF || data[1] != 'S' || data[2] != 'M' || data[3] != 'B' { + // Skip NetBIOS session header if present (0x00 + 3-byte length) + if data[0] == 0x00 && len(data) >= 36 { + data = data[4:] + } + header := &SMBHeader{} + if data[0] != 0xFF || data[1] != 'S' || data[2] != 'M' || data[3] != 'B' { return nil, fmt.Errorf("invalid SMB protocol ID") }
1-74: Scope creep: do we need the SMB toolkit in this PR?Given the PR’s stated goals (telnet brute, encryption detection, NTLM info), the large SMB builder set may be unnecessary. It adds risk and maintenance. If not required now, consider moving into a separate follow-up PR with tests.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
pkg/js/generated/go/libtelnet/telnet.gois excluded by!**/generated/**pkg/js/generated/ts/telnet.tsis excluded by!**/generated/**
📒 Files selected for processing (5)
pkg/js/libs/telnet/telnet.go(3 hunks)pkg/utils/telnetmini/doc.go(1 hunks)pkg/utils/telnetmini/ntlm.go(1 hunks)pkg/utils/telnetmini/smb.go(1 hunks)pkg/utils/telnetmini/telnet.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
pkg/js/libs/telnet/telnet.go (5)
pkg/js/generated/ts/telnet.ts (6)
TelnetClient(24-74)Connect(38-40)Info(54-56)TelnetInfoResponse(103-110)GetTelnetNTLMInfo(71-73)NTLMInfoResponse(123-164)pkg/protocols/common/protocolstate/state.go (1)
GetDialersWithId(39-45)pkg/protocols/common/protocolstate/headless.go (2)
IsHostAllowed(124-150)ErrHostDenied(22-22)pkg/utils/telnetmini/telnet.go (2)
New(72-80)DetectEncryption(91-162)pkg/utils/telnetmini/ntlm.go (3)
NTLMInfoResponse(11-20)CreateTNAPLoginPacket(227-247)ParseNTLMResponse(24-83)
pkg/utils/telnetmini/telnet.go (1)
pkg/js/libs/telnet/telnet.go (6)
IAC(18-18)DO(21-21)ENCRYPT(29-29)WILL(19-19)SB(23-23)SE(24-24)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Tests (ubuntu-latest)
🔇 Additional comments (1)
pkg/js/libs/telnet/telnet.go (1)
99-110: Service banner extraction LGTMUsing fingerprintx TELNET plugin to detect banner looks sound, and you correctly close the connection.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
cmd/integration-test/javascript.go (3)
22-26: Reduce globals to limit cross-test coupling.Optional: allocate telnetResource within the test type (or a helper) rather than as a global; this mirrors best practices and makes reuse/parallelization safer.
105-133: Use dockertest.Retry correctly; return the real error to actually retry.Right now the closure always returns nil, so Retry doesn’t retry. Let Retry drive readiness and result assertion.
Apply this diff:
- errs := []error{} - for i := 0; i < defaultRetry; i++ { - results := []string{} - var err error - _ = pool.Retry(func() error { - //let telnet server start - time.Sleep(3 * time.Second) - results, err = testutils.RunNucleiTemplateAndGetResults(filePath, finalURL, debug) - return nil - }) - if err != nil { - return err - } - if err := expectResultsCount(results, 1); err == nil { - return nil - } else { - errs = append(errs, err) - } - } - return multierr.Combine(errs...) + errs := []error{} + for i := 0; i < defaultRetry; i++ { + var results []string + var runErr error + retryErr := pool.Retry(func() error { + // let telnet server start + time.Sleep(3 * time.Second) + results, runErr = testutils.RunNucleiTemplateAndGetResults(filePath, finalURL, debug) + if runErr != nil { + return runErr + } + return expectResultsCount(results, 1) + }) + if retryErr == nil { + return nil + } + errs = append(errs, retryErr) + } + return multierr.Combine(errs...)
201-217: Harden the container setup for reproducibility and cross-arch CI.
- Pin Alpine to a minor release to avoid breakage from latest.
- Drop Platform to allow native arch pulls (avoids QEMU on arm64 runners).
- Slightly extend expiry to reduce flakes.
Apply this diff:
- Repository: "alpine", - Tag: "latest", + Repository: "alpine", + Tag: "3.20", Cmd: []string{"sh", "-c", "apk add --no-cache busybox-extras shadow && useradd -m dev && echo 'dev:mysecret' | chpasswd && exec /usr/sbin/telnetd -F -p 23 -l /bin/login"}, - Platform: "linux/amd64", + // Platform omitted to allow native arch- if err := telnetResource.Expire(30); err != nil { + if err := telnetResource.Expire(60); err != nil {Run locally to verify telnetd and login availability on the pinned Alpine version.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
integration_tests/protocols/javascript/telnet-auth-test.yamlis excluded by!**/*.yaml
📒 Files selected for processing (1)
cmd/integration-test/javascript.go(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
cmd/integration-test/javascript.go (1)
pkg/testutils/integration.go (2)
TestCase(247-250)RunNucleiTemplateAndGetResults(30-32)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Tests (windows-latest)
- GitHub Check: Tests (macOS-latest)
- GitHub Check: Tests (ubuntu-latest)
🔇 Additional comments (1)
cmd/integration-test/javascript.go (1)
103-104: New test type definition looks good.
|
@coderabbitai resolve |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
pkg/utils/telnetmini/telnet.go (2)
91-156: Enforce a hard deadline and sanitize banner in DetectEncryption (prevents hang, avoids binary garbage).Loop resets the read deadline each iteration, defeating the overall timeout; chatty servers can keep this loop alive indefinitely. Also, banner concatenates raw bytes (including controls). Apply a hard deadline loop and ASCII sanitization.
func DetectEncryption(conn net.Conn, timeout time.Duration) (*EncryptionInfo, error) { if timeout == 0 { timeout = 7 * time.Second } - // Set connection timeout - _ = conn.SetDeadline(time.Now().Add(timeout)) + // Hard stop across the whole probe + deadline := time.Now().Add(timeout) + _ = conn.SetDeadline(deadline) @@ - // Read responses until we get encryption info or timeout - for { - _ = conn.SetReadDeadline(time.Now().Add(1 * time.Second)) + // Read responses until the hard deadline + for time.Now().Before(deadline) { + _ = conn.SetReadDeadline(time.Now().Add(1 * time.Second)) buffer := make([]byte, 1024) n, err := conn.Read(buffer) if err != nil { // Timeout or connection closed, break break } @@ - // Check if this contains banner text (non-IAC bytes) - for _, b := range data { - if b != IAC { - banner += string(b) - } - } + // Append printable ASCII only, skip IAC/control bytes + var sb strings.Builder + for _, b := range data { + if b != IAC && b >= 0x20 && b <= 0x7E { + sb.WriteByte(b) + } + } + banner += sb.String()
298-337: Handle IAC IAC (escaped 0xFF) and use protocol constants instead of magic numbers in readUntil.Without IAC-escaping support, literal 0xFF bytes get dropped. Also, replace raw numbers with named constants for clarity.
- // Telnet IAC (Interpret As Command) - if tmp[0] == 255 { // IAC + // Telnet IAC (Interpret As Command) + if tmp[0] == IAC { cmd, err := c.rd.ReadByte() if err != nil { return "", b.String(), err } + // Escaped 0xFF data byte (IAC IAC) + if cmd == IAC { + b.WriteByte(IAC) + lower := strings.ToLower(b.String()) + for i, n := range lowNeedles { + if strings.Contains(lower, n) { + return needles[i], b.String(), nil + } + } + if b.Len() > c.ReadCapBytes { + return "", b.String(), errors.New("prompt not found (read cap reached)") + } + continue + } switch cmd { - case 251, 252, 253, 254: // WILL, WONT, DO, DONT + case WILL, WONT, DO, DONT: opt, err := c.rd.ReadByte() if err != nil { return "", b.String(), err } // Politely refuse everything: DONT to WILL; WONT to DO. var reply []byte - if cmd == 251 { // WILL - reply = []byte{255, 254, opt} // DONT + if cmd == WILL { + reply = []byte{IAC, DONT, opt} } - if cmd == 253 { // DO - reply = []byte{255, 252, opt} // WONT + if cmd == DO { + reply = []byte{IAC, WONT, opt} } if len(reply) > 0 { c.setDeadlineFromCtx(ctx, true) _, _ = c.wr.Write(reply) _ = c.wr.Flush() } - case 250: // SB (subnegotiation): skip until SE + case SB: // subnegotiation: skip until SE for { bb, err := c.rd.ReadByte() if err != nil { return "", b.String(), err } - if bb == 255 { - if se, err := c.rd.ReadByte(); err == nil && se == 240 { // SE + if bb == IAC { + if se, err := c.rd.ReadByte(); err == nil && se == SE { break } } } default: // NOP for other commands (IAC NOP, GA, etc.) } continue }Also applies to: 323-334, 341-352
🧹 Nitpick comments (5)
pkg/utils/telnetmini/telnet.go (5)
164-202: processTelnetOptions: tolerate split SB blocks and duplicate command entries.Current parser assumes IAC, CMD, OPT always co-resident; SB blocks can span buffers. Consider buffering incomplete IAC sequences across calls and de-duplicating options to avoid unbounded growth.
Would you like me to sketch a small rolling parser state (prevIAC, prevCmd, inSB, sbOpt) to carry between reads?
225-240: Auth treats context deadline as success; differentiate ambiguous from success.If readUntil times out post-auth, you return success unless an explicit fail-banner was seen. This can yield false positives on slow servers.
- if err != nil && !errors.Is(err, context.DeadlineExceeded) { + if err != nil && !errors.Is(err, context.DeadlineExceeded) { return fmt.Errorf("post-auth read: %s (got: %s)", preview(got, 200), err) - } + } + if errors.Is(err, context.DeadlineExceeded) && match == "" { + return errors.New("authentication indeterminate (timeout)") + }
255-263: writeLine: ensure CRLF normalization only once per call.If callers pass a trailing newline, you'll send triple endlines. Trim trailing CR/LF before appending CRLF.
- if _, err := io.WriteString(c.wr, s+"\r\n"); err != nil { + s = strings.TrimRight(s, "\r\n") + if _, err := io.WriteString(c.wr, s+"\r\n"); err != nil { return err }
349-351: Off-by-one on ReadCapBytes check.Use >= so the cap is an exact limit.
- if b.Len() > c.ReadCapBytes { + if b.Len() >= c.ReadCapBytes { return "", b.String(), errors.New("prompt not found (read cap reached)") }
355-365: setDeadlineFromCtx: clear deadlines when ctx has no deadline.If a prior call set a deadline and the next ctx has none, deadlines persist. Clear them explicitly.
func (c *Client) setDeadlineFromCtx(ctx context.Context, write bool) { if ctx == nil { return } if dl, ok := ctx.Deadline(); ok { _ = c.Conn.SetReadDeadline(dl) if write { _ = c.Conn.SetWriteDeadline(dl) } + } else { + var zero time.Time + _ = c.Conn.SetReadDeadline(zero) + if write { + _ = c.Conn.SetWriteDeadline(zero) + } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
pkg/utils/telnetmini/telnet.go(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Tests (windows-latest)
- GitHub Check: Tests (macOS-latest)
- GitHub Check: Tests (ubuntu-latest)
✅ Actions performedComments resolved. |
dwisiswant0
force-pushed
the
feat-4834-js-telnet
branch
from
882463c to
075d0a3
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (9)
pkg/utils/telnetmini/ntlm.go (1)
53-69: NTLM strings require UTF-16LE decodingTargetName in NTLM Type 2 messages is UTF-16LE encoded. Casting raw bytes to
stringproduces garbage with interleaved NUL bytes. This also applies to the AV pair parsing inparseTargetInfo(lines 118-148).pkg/js/libs/telnet/telnet.go (3)
130-139: Usectxfor dialing and full target for policy check.The dial uses
context.TODO()instead of the providedctx, andIsHostAllowedreceives onlyhostinstead of the fullhost:porttarget.
246-255: TNAP packet needs Telnet SB/SE framing to be recognized by servers.The raw TNAP payload is written directly to the TCP stream without the required Telnet subnegotiation framing (IAC SB ... IAC SE).
233-241: Double-close: bothconnandclientare closed, butclientwrapsconn.Lines 233-235 defer
conn.Close()and lines 239-241 deferclient.Close(). Sincetelnetmini.New(conn)wrapsconnandclient.Close()callsconn.Close(), this results in a double-close. Remove one of the deferred closures.defer func() { _ = conn.Close() }() // Create telnet client using the telnetmini library client := telnetmini.New(conn) - defer func() { - _ = client.Close() - }()pkg/utils/telnetmini/telnet.go (3)
278-287: The 20-iteration limit will abort before matching real prompts.
maxIterations = 20counts bytes, not iterations over meaningful data chunks. Most login prompts exceed 20 bytes, causing premature return with empty match.
299-338: Missing handling for escaped IAC (IAC IAC = literal 0xFF data byte).Per Telnet protocol,
IAC IAC(0xFF 0xFF) represents a literal 0xFF data byte. Currently, this sequence is misinterpreted as a command.
91-156: DetectEncryption loop can exceed the intended timeout.The per-iteration
SetReadDeadlineresets the deadline each loop, effectively removing the hard timeout cap set at line 97.pkg/utils/telnetmini/smb.go (2)
232-257: Critical:buf.Next(n)returns bytes already written, not a writable buffer.
binary.LittleEndian.PutUint16(buf.Next(2), value)reads 2 bytes from the buffer (which don't exist yet), returning an empty or short slice. Writing to this slice does nothing. This pattern is used throughout the SMB packet builders and results in malformed packets.
101-116: NetBIOS session header length is incorrect.The length field spans bytes 1-3 (24-bit big-endian) and should include the SMB header + data, not just smbData. Current code only sets the low byte.
🧹 Nitpick comments (3)
pkg/js/libs/telnet/telnet.go (2)
16-30: Consider importing constants from telnetmini to avoid duplication.These Telnet protocol constants are duplicated in
pkg/utils/telnetmini/telnet.go. To reduce maintenance burden and ensure consistency, consider exporting them fromtelnetminiand importing here.
256-265: SingleReadmay not capture the complete NTLM response.A single
conn.Readcall may return partial data. For reliability, consider looping until sufficient data is received or a timeout occurs, especially for multi-packet NTLM exchanges.pkg/utils/telnetmini/telnet.go (1)
304-316: Use defined constants instead of magic numbers for Telnet commands.Replace literal values (251, 252, 253, 254, 255, 250, 240) with the constants defined at the top of this file (WILL, WONT, DO, DONT, IAC, SB, SE) for consistency and readability.
- if tmp[0] == 255 { // IAC + if tmp[0] == IAC { cmd, err := c.rd.ReadByte() if err != nil { return "", b.String(), err } switch cmd { - case 251, 252, 253, 254: // WILL, WONT, DO, DONT + case WILL, WONT, DO, DONT: opt, err := c.rd.ReadByte() if err != nil { return "", b.String(), err } // Politely refuse everything: DONT to WILL; WONT to DO. var reply []byte - if cmd == 251 { // WILL - reply = []byte{255, 254, opt} // DONT + if cmd == WILL { + reply = []byte{IAC, DONT, opt} } - if cmd == 253 { // DO - reply = []byte{255, 252, opt} // WONT + if cmd == DO { + reply = []byte{IAC, WONT, opt} }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
integration_tests/protocols/javascript/telnet-auth-test.yamlis excluded by!**/*.yamlpkg/js/generated/go/libtelnet/telnet.gois excluded by!**/generated/**pkg/js/generated/ts/telnet.tsis excluded by!**/generated/**
📒 Files selected for processing (6)
cmd/integration-test/javascript.go(3 hunks)pkg/js/libs/telnet/telnet.go(3 hunks)pkg/utils/telnetmini/doc.go(1 hunks)pkg/utils/telnetmini/ntlm.go(1 hunks)pkg/utils/telnetmini/smb.go(1 hunks)pkg/utils/telnetmini/telnet.go(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- pkg/utils/telnetmini/doc.go
🚧 Files skipped from review as they are similar to previous changes (1)
- cmd/integration-test/javascript.go
🧰 Additional context used
📓 Path-based instructions (3)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Format Go code usinggo fmt ./...
Run static analysis usinggo vet ./...on Go code
Files:
pkg/utils/telnetmini/ntlm.gopkg/utils/telnetmini/telnet.gopkg/js/libs/telnet/telnet.gopkg/utils/telnetmini/smb.go
pkg/js/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Use custom JavaScript runtime for code protocol templates with auto-generated bindings from pkg/js/generated/
Files:
pkg/js/libs/telnet/telnet.go
pkg/js/libs/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Use custom JavaScript library implementations located in pkg/js/libs/ for JavaScript protocol support
Files:
pkg/js/libs/telnet/telnet.go
🧬 Code graph analysis (1)
pkg/utils/telnetmini/telnet.go (1)
pkg/js/libs/telnet/telnet.go (6)
IAC(18-18)DO(21-21)ENCRYPT(29-29)WILL(19-19)SB(23-23)SE(24-24)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Lint
🔇 Additional comments (4)
pkg/utils/telnetmini/ntlm.go (1)
166-223: LGTM on the NTLM Negotiate blob construction.The flag composition and field layout align with NTLM Type 1 message structure. The use of bitwise OR for flags is correct.
pkg/utils/telnetmini/telnet.go (1)
49-69: LGTM on the default prompt patterns.The default patterns for login, password, failure, and shell prompts are reasonable and cover common cases.
pkg/utils/telnetmini/smb.go (2)
697-724: LGTM onParseSMBResponse.The parsing logic correctly validates the protocol signature and extracts header fields at the proper offsets.
9-74: LGTM on the SMB/NTLM constant definitions.The protocol constants are correctly defined and align with SMB1/NTLM specifications.
Signed-off-by: Dwi Siswanto <git@dw1.io>
Signed-off-by: Dwi Siswanto <git@dw1.io>
4 participants
Proposed changes
Closes #4834
Checklist
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.