cisco / libest

Other
97 stars 94 forks source link

Fix content-length error on /serverkeyen response #85

Closed JamesGibo closed 4 years ago

JamesGibo commented 4 years ago

The response did not include the length of the headers for each part in the response, meaning that the content-length header was shorter than the content by 245 bytes. This meant that the response was truncated on clients like Python Requests or curl. The response was also incorrectly formatted meaning the header for the private key was not parsed correctly.

The only functional changes are to the est_handle_server_keygen function in est_server.c the rest are whitespace issues

rpb5bnc commented 4 years ago

Hi,

Thanks for uncovering this issue. I'm sorry to hear about this one, but I'm also not totally surprised by this either. Server Key Gen (SKG) was added at the request of a product team and then they never ended up making use of it. We have a fair amount of internal testing we do against all our features within our common security modules, but in cases like this we were effectively testing against ourselves. At the moment there's still no product team making use of SKG so there's been no independent testing that we usually get from the product teams.

I see the changes and they seem good. What I'll do is pull them into the internal version of CiscoEST, run it through our internal Development tests. If everything goes smoothly I'll push these back out here to Github. Unfortunately, I may not be able to pick up all these trailing spaces issues. I'll try to include those though and see what happens.

Thanks very much for doing the debug and providing the specific fix. Pete

rpb5bnc commented 4 years ago

Hi James,

It had been decided to address the issue you found in a slightly different way. Instead of creating the constants for the additional headers and then calculating the total Content-Length using these, the headers were created in separate buffers and then their lengths obtained and added up. Tests were run to confirm that the same lengths were obtained with your proposed changes and this other method. Hopefully, this alternative method provides the correct results for you as well.

Thank you again for uncovering this issue and providing the PR. We appreciate it. If you run into anything else just let us know.

Take care, Pete