Git Product home page Git Product logo

Comments (11)

dhbaird avatar dhbaird commented on August 9, 2024

Thanks a lot for this report, @giuliomoro. It sounds like we will need to add masking as a standard feature to easywsclient. We'll get on that. What server do you connect to, which has the masking requirement?

Per your recommendation, in 7cbf090, I've added CRTSECURE_NO_WARNINGS, fixed the type cast in ::send() (cast to char* instead of int8_t*), and added WSAStartup() and WSACleanup() in the -cpp11 example.

from easywsclient.

giuliomoro avatar giuliomoro commented on August 9, 2024

It is a custom local test server of mine, that's how I managed to disable the masking requirement.

from easywsclient.

giuliomoro avatar giuliomoro commented on August 9, 2024

According to the RFC 6455 specifications ( http://tools.ietf.org/html/rfc6455#section-5.1 ) and if I didn't get it wrong, and unless the anticipated "relaxed rules in future specification" has been issued, a client MUST mask all frames. Therefore, it seems that easywsclient is not behaving after the specifics as it does not send masked frames. Or is it built with a different specification in mind?
" In the WebSocket Protocol, data is transmitted using a sequence of
frames. To avoid confusing network intermediaries (such as
intercepting proxies) and for security reasons that are further
discussed in Section 10.3, a client MUST mask all frames that it
sends to the server (see Section 5.3 for further details). (Note
that masking is done whether or not the WebSocket Protocol is running
over TLS.) The server MUST close the connection upon receiving a
frame that is not masked. In this case, a server MAY send a Close
frame with a status code of 1002 (protocol error) as defined in
Section 7.4.1. A server MUST NOT mask any frames that it sends to
the client. A client MUST close a connection if it detects a masked
frame. In this case, it MAY use the status code 1002 (protocol
error) as defined in Section 7.4.1. (These rules might be relaxed in
a future specification.)"

from easywsclient.

dhbaird avatar dhbaird commented on August 9, 2024

@giuliomoro - you are correct. 2cac70b now contains extremely basic masking support. The from_url() factory now uses masking by default. If no masking (the old behavior) is desired, from_url_no_mask() may be used.

Caveat: The mask is not based on a good random number source (yet), and therefore can be exploited to confuse non-compliant intermediaries (the whole reason masking exists in the first place). I hard coded a masking key for now, while we look into a good way to get the random numbers.

Some background on the project may help - easywsclient was born from projects which used WebSocket on localhost or local networks to integrate software and hardware components via a web server. Because masking represented extra overhead and did not provide any benefit in that scenario, masking was not implemented (because there were no non-compliant intermediaries). But now it is supported :)

from easywsclient.

giuliomoro avatar giuliomoro commented on August 9, 2024

Thank you. Howevere there are still a couple of bugs in the files you updated:

  1. this is my fault as I told you to include #define CRTSECURE_NO_WARNINGS while the correct line is
    #define _CRT_SECURE_NO_WARNINGS
  2. such line should be in the .hpp file otherwise if Visual Studio compiles example-client.cpp before easyws-client.cpp, it wouldn't realize that CRT_SECURE_NO_WARNINGS has been defined and che compiler would complain about this.
    3)you forgot about one casting on line 186 in easywsclient.cpp
    ret = recv(sockfd, (int8_t
    )&rxbuf[0] + N, 1500, 0);
    should instead be
    ret = recv(sockfd, (char_)&rxbuf[0] + N, 1500, 0);

Thannk you

from easywsclient.

dhbaird avatar dhbaird commented on August 9, 2024
  1. Ah, I see - I just double checked (my fault too, for not double checking before) the correct spelling is #define _CRT_SECURE_NO_WARNINGS (github markdown ate the underscores). Just fixed it.
  2. My intuition says that it smells fishy to put that flag in the header file, because then it is forced on the users inadvertently who want to use that header. For this reason, I would like to hold back on putting it in the .hpp file until I can understand better what the problem is. This is probably a limitation in my knowledge of MSVC compilers. Is the problem a side effect of using precompiled headers (stdafx.h)...?
  3. Great, fixed it!

from easywsclient.

giuliomoro avatar giuliomoro commented on August 9, 2024

I am not using any precompiled headers.
The compiler complains because sscanf is "potentially" unsafe because it could try to access memory non previously allocated, similarly to what happens to strcpy ( http://stackoverflow.com/questions/4012222/c-strcpy-gives-a-warning-c4996 ) .
This is the error message
Error 5 error C4996: 'sscanf': This function or variable may be unsafe. Consider using sscanf_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
Warning C4996 is classified by Microsoft like this: http://msdn.microsoft.com/en-us/library/ttcz0bys.aspx
It should be a warning for deprecated functions, but in Visual Studio 2013 express is actually an error and does not allow me to compile unless I #define __CRT_SECURE_NO_WARNINGS . Maybe the documentation is outdated or I must tick some options somewhere

from easywsclient.

dhbaird avatar dhbaird commented on August 9, 2024

We'll sscanf carefully. A check has been added to ensure that the source strings are no longer than the destination strings when doing sscanf in fc12f28.

There are two ways I understand to do _CRT_SECURE_NO_WARNINGS:

  1. Set _CRT_SECURE_NO_WARNINGS in your project settings, via via "Project"->"Properties"->"Configuration properties"->"C/C++"->"Preprocessor"->"Preprocessor definitions": http://stackoverflow.com/questions/119578/disabling-warnings-generated-via-crt-secure-no-deprecate
  2. Set _CRT_SECURE_NO_WARNINGS at the top of any .cpp file that uses sscanf, before including the offending headers.

However, I don't understand the rationale to put _CRT_SECURE_NO_WARNINGS in a .hpp file.

from easywsclient.

dhbaird avatar dhbaird commented on August 9, 2024

The masking calculation in send() was being done wrong (used wrong offset in a buffer). Fixed in d678234.

from easywsclient.

giuliomoro avatar giuliomoro commented on August 9, 2024

The only reason why I'd put it in the .hpp file is that it is included by both easywsclient.cpp and the example-client.cpp and so it would be in just one place. I don't know if it is good or bad practice.
Regarding your options, in my opinion number 1. would be a good option if the user would get such hint in "readme for windows" file, or if a .sln Visual Studio solution file was bundled with easywsclient.
By the way I don't know if such issue is also to be found in previous versions of visual studio
so we coult put a check in the preprocessor instructions remembering that
_MSC_VER == 1800 (Visual Studio 2013)

from easywsclient.

dhbaird avatar dhbaird commented on August 9, 2024

I'm happy with things now, concerning _CRT_SECURE_NO_WARNINGS and sscanf(), since there is an option to resolve the issue. It seems like putting it a #define _CRT_SECURE_NO_WARNINGS at the tops of .cpp files should also fix the issue, so I'm confused why that's not working; But at least there's a workaround via "Project"->"Properties". I'll go ahead and close the ticket for now, but feel free to reopen it if you come up with new discoveries.

from easywsclient.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.