Pennyw0rth
[SMB] Switch signing:true to signing:Req and signing:false to signing:NotReq
This PR switches the signing output to something clearer:
That way we know exactly what the old signing:True means.
Due to all the things in the group chat i think we should at first leave it at that until we have the distinction between notReq and None. At least my opinion
Due to all the things in the group chat i think we should at first leave it at that until we have the distinction between
notReqandNone. At least my opinion
Thoughts everyone? Should i close it for now?
One night later, I think we could at least change signing to signingreq. That way we know for sure what signing:true means
One night later, I think we could at least change signing to signingreq. That way we know for sure what signing:true means
I think that is a good compromise until we have signing None/Not-Req.
How about signReq to make it shorter? Or is that too short? Kinda worried the output line gets too long.
I think signing:Req and signing:NotReq should be fine. It's not too short and too long.
But I have crazy idea, what if only showing
signing with color, without req, notreq, or true,false? Green Signing and Red Signing. Everyone can understand what it means green and red lol.
I like the idea of removing True/False because to be honest, that's a lot of useless characters but we still need a string marker to allow grepping.
So we could have something like:
- SigningReq (in green)
- SigningNotReq (in red)
Something like
And since we are thinking reducing the output of each output lines, I propose to remove the SMBv1:False flag and only print it when SMBv1 is activated. The same we do with NullAuth. @NeffIsBack
Actually we can even think of removing everything if it is correctly configured. Like who cares about signing:True computers. What we look for during internal assessment is signing:False. Same for SMBv1, same for channel binding
My thoughts regarding the two ideas:
Removing True/False: Although that would reduce characters in the info header there are a few problems with that:
- On terminals that does not support color the signingReq vs signingNotReq is pretty easy to miss imo. At least it is much harder to just look for False/True (also applies to terminals with color)
- This would be inconsistent to other flags such as channel binding for example where we would have key:value (channel binding:None/When supported/Required). This is especially true if we would like to add signing:None/Req/NotReq in the future and we would have to change it back.
- This would be inconsistent to other parts of the application like the nxcdb. There it is a column with signing True/False so you can export it to a csv and filter it or parse the data in other applications
TLDR; Not worth the trade of imo when it comes to readability/consistency or generally making clear what the value is.
Hide attributes that are False: While it is clear to us what attributes are checked and how netexec displays different misconfiguration's I think it could be quite confusing for people that aren't that deep into netexec or start getting used to it. Having an attribute displayed that is either False or True makes it pretty clear if it is correctly configured or not. Otherwise people might wonder if the flag was removed or moved to an arg/config check or just failed.
Regarding NTLM disabled and null auth, I think the only reason why it was not added as a permanently flag is that it is either just too rare to be worth added all the time, or in the case of null auth the output header would just get too long.
However, I would be fine with removing the SMBv1 output or flag in general. There is no attack chain/scenario of which I would be aware of where this info would help, but just something that I add in reports. We could for example disable it by default and make a flag/config check to enable it, which would reduce the header length. Or just trade the SMBv1 output with the null auth output (much more interesting imo).
I'd keep the smbv1:true but remove the false. Because having SMBv1 enabled can mean a lot of things like not up to date server or missing GPO. At least if I see one smbv1 among 10's of servers without it, I'd go directly toward that one server because it is missing something the others are not.
But yeah the smbv1:False flag is useless.
Aggree to keep signing:Req/NotReq until we have modified Impaket so that it has got all values (disabled, not req, req)
Could we update the signing to be (Signing:True [Req]) or (Signing:True - Req)/(Signing:True-Req) (or an alternative deliminator) if it's required? That way it won't break greps searching for Signing:True, but it gives the additional information we want.
However, I would be fine with removing the SMBv1 output or flag in general.
agreed, just like we do with ntlm flag, only if True
So are we good if I comit the following:
- SigningReq:True/False
- SMBv1 only printed if True
?
So are we good if I comit the following:
SigningReq:True/False
SMBv1 only printed if True
?
Sounds good to me. How about we also add a config option to be able to disable the SMBv1 check entirely, similar to the guest auth?
At the end SigningReq:True/False is the same as Signing:True/False the wiki explain it pretty well https://www.netexec.wiki/smb-protocol/enumeration/smb-signing-not-required
Come on mate, who reads wikis ?
Actually quite a lot^^ 3.3k views over the past 30 days on that site alone
Okay you got me 🫡 But still I'd rather people know exactly what that means without having yo go on the wiki
Alright I have switched the code as discussed:
I took the liberty of making the base code easier to read / extend :P