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.

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

implement custom protobuf debug string creation #4532

Merged
merged 6 commits into from
Feb 23, 2022

Conversation

ebbit1q
Copy link
Member

@ebbit1q ebbit1q commented Jan 19, 2022

Short roundup of the initial problem

cockatrice generates a lot of debug output, which is great, but some things should not be logged, namely passwords and lengthy pictures/decklists

What will change with this Pull Request?

  • creates function that uses protobuf's text format functionality to extend the normal logging with protective measures
  • apply safe logger on all instances of getting a pb message debug string

example log:

IN "62 bytes message_type: SESSION_EVENT session_event { [Event_ServerIdentification.ext] { server_name: \"Rooster Range\" server_version: \"2.8.1-custom(e845c95) (2021-12-27)\" protocol_version: 14 server_options: SupportsPasswordHash } } "
OUT "15 bytes cmd_id: 0 session_command { [Command_RequestPasswordSalt.ext] { user_name: \"ebbit\" } } "
IN "29 bytes message_type: RESPONSE response { cmd_id: 0 response_code: RespOk [Response_PasswordSalt.ext] { password_salt: \" ---value expunged--- \" } } "
OUT "352 bytes cmd_id: 1 session_command { [Command_Login.ext] { user_name: \"ebbit\" clientid: \"1cc5de561762aac\" clientver: \"2.8.1-beta.6 (2022-01-16)\" clientfeatures: \"2.7.0_min_version\" clientfeatures: \"2.8.0_min_version\" clientfeatures: \"client_id\" clientfeatures: \"client_ver\" clientfeatures: \"client_warnings\" clientfeatures: \"feature_set\" clientfeatures: \"forgot_password\" clientfeatures: \"idle_client\" clientfeatures: \"mod_log_lookup\" clientfeatures: \"room_chat_history\" clientfeatures: \"user_ban_history\" clientfeatures: \"websocket\" hashed_password: \" ---value expunged--- \" } } "
IN "2100 bytes message_type: RESPONSE response { cmd_id: 1 response_code: RespOk [Response_Login.ext] { user_info { name: \"ebbit\" user_level: 19 avatar_bmp: \"RIFF\\010\\005\\000\\000WEBPVP8X\\n\\000\\000\\0000\\000\\000\\000\\233\\000\\000;\\000\\000ICCPL\\002\\000\\000\\000\\000\\002Llcms\\0040\\000\\000mntrRGB XYZ \\007\\345\\000\\003\\000\\005\\000\\004\\000\\000\\000\\nacspAPPL\\000\\000\\000\\000\\000\\000\\000\\000\\000\\000\\000\\000\\000\\000\\000\\000\\000\\000\\000\\000\\000\\000\\000\\000\\000\\000\\366\\326\\000\\001\\000\\000\\000\\000... ---snip--- (65535 bytes total) \" accountage_secs: 179313183 privlevel: \"VIP\" } } "

OUT "128 bytes cmd_id: 12 session_command { [Command_AccountPassword.ext] { old_password: \" ---value expunged--- \" hashed_new_password: \" ---value expunged--- \" } } "
IN "8 bytes message_type: RESPONSE response { cmd_id: 12 response_code: RespPasswordTooShort } "

@ebbit1q ebbit1q changed the title implement custom protobuf debug log string creation implement custom protobuf debug string creation Jan 19, 2022
@ebbit1q
Copy link
Member Author

ebbit1q commented Jan 19, 2022

I changed the logging behavior a bit so it no longer adds so many backslashes:

IN 74 bytes message_type: SESSION_EVENT session_event { [Event_ServerIdentification.ext] { server_name: "TEST SERVER PLEASE IGNORE" server_version: "2.8.1-custom(32df434) (2022-01-19)" protocol_version: 14 server_options: SupportsPasswordHash } } 
OUT 14 bytes cmd_id: 0 session_command { [Command_RequestPasswordSalt.ext] { user_name: "bbbb" } } 
IN 29 bytes message_type: RESPONSE response { cmd_id: 0 response_code: RespOk [Response_PasswordSalt.ext] { password_salt: " ---value expunged--- " } } 
OUT 360 bytes cmd_id: 1 session_command { [Command_Login.ext] { user_name: "bbbb" clientid: "75c7fc34cca019e" clientver: "2.8.1-custom(32df434) (2022-01-19)" clientfeatures: "2.7.0_min_version" clientfeatures: "2.8.0_min_version" clientfeatures: "client_id" clientfeatures: "client_ver" clientfeatures: "client_warnings" clientfeatures: "feature_set" clientfeatures: "forgot_password" clientfeatures: "idle_client" clientfeatures: "mod_log_lookup" clientfeatures: "room_chat_history" clientfeatures: "user_ban_history" clientfeatures: "websocket" hashed_password: " ---value expunged--- " } } 
IN 65586 bytes message_type: RESPONSE response { cmd_id: 1 response_code: RespOk [Response_Login.ext] { user_info { name: "bbbb" user_level: 3 real_name: "bbbb" country: "bb" avatar_bmp: "\377\330\377\340\000\020JFIF\000\001\001\001\000H\000H\000\000\377\342\014XICC_PROFILE\000\001\001\000\000\014HLino\002\020\000\000mntrRGB XYZ \007\316\000\002\000\t\000\006\0001\000\000acspMSFT\000\000\000\000IEC sRGB\000\000\000\000\000\000\000\000\000\000\000\000\000\000\366\326\000\001\000\000\000\000\323-HP  \000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\021cprt\000\000\001P\000\000\0003desc\000\000\001\204\000\000\000lwtpt\000\000\001\360\000\000\000\024bkpt\000\000\002\004\000\000\000\024rXYZ\000\000\002\030\000\000\000\024gXYZ\000\000\002,\000\000\000\024bXYZ\000\000\002@\000\000\000\024d... ---snip--- (65535 bytes total) " accountage_secs: 360608 privlevel: "NONE" } } }

@ebbit1q ebbit1q merged commit 7108eb4 into Cockatrice:master Feb 23, 2022
@ebbit1q ebbit1q deleted the debug_pb_message branch February 23, 2022 22:46
ebbit1q added a commit that referenced this pull request Oct 2, 2023
this was implemented in #4532, removing this line makes this entire file pointless

the original internal function this used has been placed in absl
instead, this is why it no longer works in protobuf 23+

I've chosen to remove this small optimisation and instead go with the
simpler solution, escaping the hard coded strings as well, see the code
on protobuf's source for reference:
https://github.com/protocolbuffers/protobuf/blob/81068e8e8cf00f25559112a562661064d59bfa87/src/google/protobuf/text_format.cc#L1648
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants