Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

#77 : Implement a handler for iSCSI. #165

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

#77 : Implement a handler for iSCSI. #165

wants to merge 3 commits into from
+279 −1

Conversation

namay26
Copy link

@namay26 namay26 commented Jan 28, 2025

No description provided.

@namay26
Copy link
Author

namay26 commented Jan 28, 2025

@glaslos Kindly review.

glaslos
glaslos requested changes Jan 28, 2025
View reviewed changes
config/rules.yaml Outdated Show resolved Hide resolved
protocols/tcp/iscsi.go Outdated Show resolved Hide resolved
protocols/tcp/iscsi.go Outdated Show resolved Hide resolved
protocols/tcp/iscsi.go Outdated Show resolved Hide resolved
protocols/tcp/iscsi.go Show resolved Hide resolved
protocols/tcp/iscsi.go Show resolved Hide resolved
protocols/tcp/iscsi.go Outdated Show resolved Hide resolved
@glaslos
Copy link
Member

glaslos commented Jan 28, 2025

Nice start. Don't worry about the amount of feedback, there is quite a bit of undocumented convention in this project.
Please add some unit tests, specifically for the parser.
If you expect multiple messages to be sent between the client and server, I suggest adding a protocol server. You can think of it as a state handler.

@namay26
Copy link
Author

namay26 commented Jan 31, 2025

@glaslos Made the changes. Kindly review!

glaslos
glaslos requested changes Jan 31, 2025
View reviewed changes
Copy link
Member

@glaslos glaslos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you tried connecting to the honeypot with an ISCSI client?

config/rules.yaml Outdated Show resolved Hide resolved
protocols/tcp/iscsi.go Outdated Show resolved Hide resolved
protocols/tcp/iscsi.go Show resolved Hide resolved
protocols/tcp/iscsi.go Outdated Show resolved Hide resolved
protocols/tcp/iscsi/iscsi.go Outdated Show resolved Hide resolved
protocols/tcp/iscsi/iscsi_test.go Show resolved Hide resolved
protocols/tcp/iscsi/iscsi_test.go Outdated Show resolved Hide resolved
@namay26
Copy link
Author

namay26 commented Feb 5, 2025

@glaslos Can you check it now?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@glaslos glaslos glaslos requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@namay26 @glaslos