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

Fix #1146 + some coding style cleanups #1156

Closed
wants to merge 2,605 commits into from
Closed

Fix #1146 + some coding style cleanups #1156

wants to merge 2,605 commits into from

Conversation

krizhanovsky
Copy link
Contributor

Fix #1146: ttls_update_checksum() can be called for chunk of ClientHello,
while we still don't know the cipher suite. So calculate two checksums
in parallel and copy SHA256 contex if necessary when ClientHello
sets xfrm.ciphersuite_info.

The rest of the patch, besides ttls_update_checksum() changes, is
coding style adjustments.

aleksostapenko and others added 30 commits July 14, 2018 14:37
Fix #1033: Change header name format in HTTP tables configuration.
 Fix #772: Change 'keepalive' and 'client_body' timeouts applying.
   The page is used later as skb page fragment in TLS handshake.
2. Cleanups in PEM decoder.
3. Remove DHM routines called only for FS IO (unused).
4. Revert DTLS routines (dirty code) as it's supposed to used them for QUIC.
Fix #900: Change some comments and add unit tests.
2. Make GFSM traverse skb list and call a FSM for each skb. TLS layer
   must collect skbs before decryption to have AEAD tag, so it sends
   list of decrypted skbs to HTTP layer by GFSM call. However, HTTP
   manages skb list on it's own.
3. Make ttls_decrypt() to work with plain buffers as well as fragmented
   skbs;
4. Some cleanups.
SIMD crypto algorithms won't be called through cryptd.
Some cleanups.
tfw_gfsm_switch() can be called multiple times during tfw_gfsm_move()
call. After each tfw_gfsm_switch() call current FSM is switched
to FSM stated in fsm_hooks. Thus FSM(st) actually points on
child FSM state, not parent one.
krizhanovsky and others added 19 commits January 10, 2019 17:11
Various backup file copies were accidentally added to the repo.
Sanitize Content-Type for multipart/form-data requests
* req->vhost is expected to be non-NULL, as it was checked before;
* disallow multiple instances of http_post_validate at topmost level;
* a typo.
Instead of distinct flag, TFW_HTTP_CONN_CLOSE flags is set
to close client connection. The reason for a change
is message adjusting on forwarding. The 'Connection: ' header
should be properly set.
ss_close_sync() was replaced by tfw_connection_close() in current master.
Fixes on serving requests with errors
…llo,

while we still don't know the cipher suite. So calculate two checksums
in parallel and copy SHA256 contex if necessary when ClientHello
sets xfrm.ciphersuite_info.

The rest of the patch, besides ttls_update_checksum() changes, is
coding style adjustments.
@vankoven
Copy link
Contributor

vankoven commented Jan 25, 2019

Didn't finish the review yet. But it seems that the issue is still here. I've tried to run tests on the CI, and crash on tls.test_tls_stress.StressTls.test_tls still happens. See http://93.115.28.191:4010/#/builders/5/builds/416 . The test was configured to 10 concurrent connections and 10 seconds duration.

@vankoven
Copy link
Contributor

May be related: #1157 . There also a lot of warnings in kernel log during the test. Please see shell_15 output on CI. It's not really readable, but there are some schedule while atomic warnings and a lot of more messages (partly garbaged).

Copy link
Contributor

@aleksostapenko aleksostapenko left a comment

Choose a reason for hiding this comment

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

Looks good to me, with minor cleanups.

sizeof(*sha256));
bzero_fast(&hs->ecdh_ctx, sizeof(ttls_ecdh_context));
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Redundant empty line.

* @key_cert - chosen key/cert pair (server);
* @sni_key_cert - key/cert list from SNI;
* @sni_ca_chain - trusted CAs from SNI callback;
* @sni_ca_crt - trusted CAs CRLs from SNI;
Copy link
Contributor

Choose a reason for hiding this comment

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

sni_ca_crt -> sni_ca_crl

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.

7 participants