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

SslSocketTest.RsaPrivateKeyProviderSyncSignFailure flaky #8255

Closed
alyssawilk opened this issue Sep 16, 2019 · 2 comments · Fixed by #8264
Closed

SslSocketTest.RsaPrivateKeyProviderSyncSignFailure flaky #8255

alyssawilk opened this issue Sep 16, 2019 · 2 comments · Fixed by #8264
Assignees

Comments

@alyssawilk
Copy link
Contributor

Ismo, I think you have the most context on this test - would you be able to take a look?

I can repro locally with
bazel test //test/extensions/transport_sockets/tls:ssl_socket_test --runs_per_test=1000
but if you have trouble the extra flags in test/integration/README.md deflaking section might help.

Sample failure:
https://circleci.com/gh/envoyproxy/envoy/276851?utm_campaign=workflow-failed&utm_medium=email&utm_source=notification

[ RUN ] IpVersions/SslSocketTest.RsaPrivateKeyProviderSyncSignFailure/IPv6
unknown file: Failure

Unexpected mock function call - returning directly.
Function call: onEvent(4-byte object <02-00 00-00>)
Google Mock tried the following 1 expectation, but it didn't match:

test/extensions/transport_sockets/tls/ssl_socket_test.cc:416: EXPECT_CALL(server_connection_callbacks, onEvent(options.expectedServerCloseEvent()))...
Expected arg #0: is equal to 4-byte object <00-00 00-00>
Actual: 4-byte object <02-00 00-00>
Expected: to be called once
Actual: never called - unsatisfied and active
Stack trace:
0x1c62f29: testing::internal::GoogleTestFailureReporter::ReportFailure()
0x4c85c5: testing::internal::Expect()
0x1c770a5: testing::internal::UntypedFunctionMockerBase::UntypedInvokeWith()
0x4ec4d0: testing::internal::FunctionMocker<>::Invoke()
0x4ec3e3: Envoy::Network::MockConnectionCallbacks::onEvent()
0x127a906: Envoy::Network::ConnectionImpl::raiseEvent()
0x56c636: Envoy::Extensions::TransportSockets::Tls::SslSocket::doHandshake()
0x56d202: Envoy::Extensions::TransportSockets::Tls::SslSocket::doWrite()
0x127ade9: Envoy::Network::ConnectionImpl::onWriteReady()
0x127c879: Envoy::Network::ConnectionImpl::onFileEvent()
0x127ec7e: Envoy::Network::ConnectionImpl::ConnectionImpl()::$_2::operator()()
0x127eb31: std::_Function_handler<>::_M_invoke()
0x9dbbfa: std::function<>::operator()()
0x1271e47: Envoy::Event::FileEventImpl::assignEvents()::$_0::operator()()
0x1271c78: Envoy::Event::FileEventImpl::assignEvents()::$_0::__invoke()
0x1b947eb: event_process_active_single_queue
0x1b912b0: event_base_loop
0x13085bc: Envoy::Event::LibeventScheduler::run()
0x126bfa8: Envoy::Event::DispatcherImpl::run()
0x43b47f: Envoy::Extensions::TransportSockets::Tls::(anonymous namespace)::testUtil()
0x467adb: Envoy::Extensions::TransportSockets::Tls::SslSocketTest_RsaPrivateKeyProviderSyncSignFailure_Test::TestBody()
0x1c5b51e: testing::internal::HandleSehExceptionsInMethodIfSupported<>()
0x1c4b1ae: testing::internal::HandleExceptionsInMethodIfSupported<>()
0x1c35023: testing::Test::Run()
0x1c35c35: testing::TestInfo::Run()
... Google Test internal frames ...

unknown file: Failure

Unexpected mock function call - returning directly.
Function call: onEvent(4-byte object <02-00 00-00>)
Google Mock tried the following 1 expectation, but it didn't match:

test/extensions/transport_sockets/tls/ssl_socket_test.cc:414: EXPECT_CALL(client_connection_callbacks, onEvent(Network::ConnectionEvent::RemoteClose))...
Expected arg #0: is equal to 4-byte object <00-00 00-00>
Actual: 4-byte object <02-00 00-00>
Expected: to be called once
Actual: never called - unsatisfied and active
Stack trace:
0x1c62f29: testing::internal::GoogleTestFailureReporter::ReportFailure()
0x4c85c5: testing::internal::Expect()
0x1c770a5: testing::internal::UntypedFunctionMockerBase::UntypedInvokeWith()
0x4ec4d0: testing::internal::FunctionMocker<>::Invoke()
0x4ec3e3: Envoy::Network::MockConnectionCallbacks::onEvent()
0x127a906: Envoy::Network::ConnectionImpl::raiseEvent()
0x56c636: Envoy::Extensions::TransportSockets::Tls::SslSocket::doHandshake()
0x56d202: Envoy::Extensions::TransportSockets::Tls::SslSocket::doWrite()
0x127ade9: Envoy::Network::ConnectionImpl::onWriteReady()
0x127c879: Envoy::Network::ConnectionImpl::onFileEvent()
0x127ec7e: Envoy::Network::ConnectionImpl::ConnectionImpl()::$_2::operator()()
0x127eb31: std::_Function_handler<>::_M_invoke()
0x9dbbfa: std::function<>::operator()()
0x1271e47: Envoy::Event::FileEventImpl::assignEvents()::$_0::operator()()
0x1271c78: Envoy::Event::FileEventImpl::assignEvents()::$_0::__invoke()
0x1b947eb: event_process_active_single_queue
0x1b912b0: event_base_loop
0x13085bc: Envoy::Event::LibeventScheduler::run()
0x126bfa8: Envoy::Event::DispatcherImpl::run()
0x43b47f: Envoy::Extensions::TransportSockets::Tls::(anonymous namespace)::testUtil()
0x467adb: Envoy::Extensions::TransportSockets::Tls::SslSocketTest_RsaPrivateKeyProviderSyncSignFailure_Test::TestBody()
0x1c5b51e: testing::internal::HandleSehExceptionsInMethodIfSupported<>()
0x1c4b1ae: testing::internal::HandleExceptionsInMethodIfSupported<>()
0x1c35023: testing::Test::Run()
0x1c35c35: testing::TestInfo::Run()
... Google Test internal frames ...

Terminated
-- Test timed out at 2019-09-16 13:29:33 UTC --

@ipuustin
Copy link
Member

@alyssawilk: Thanks, I can reproduce the issue, and I have a potential fix to the test (it seems that the method for generating an artificial crypto error wasn't very robust). I'll try to prepare a PR when I get the 1000-test run passing.

@alyssawilk
Copy link
Contributor Author

Awesome, thanks so much for diving on this!

lizan pushed a commit that referenced this issue Sep 18, 2019
Description:

Fix a test flake.

If crypto error option is set in the signing test, just leave the digest to zeroes instead of trying to modify it. The digest modification might just cause ASN object structure changes.

BoringSSL source appears to have a similar test where the ASN structure is parsed before modification:
https://github.com/google/boringssl/blob/a7d9ac2af4684747c4524cbeba9737b04dce3e3e/crypto/fipsmodule/ecdsa/ecdsa_test.cc#L143

Risk Level: low
Testing: `bazel test //test/extensions/transport_sockets/tls:ssl_socket_test --runs_per_test=1000`
Docs Changes: N/A
Release Notes: N/A
Fixes: #8255 

Signed-off-by: Ismo Puustinen <[email protected]>
danzh2010 pushed a commit to danzh2010/envoy that referenced this issue Sep 24, 2019
Description:

Fix a test flake.

If crypto error option is set in the signing test, just leave the digest to zeroes instead of trying to modify it. The digest modification might just cause ASN object structure changes.

BoringSSL source appears to have a similar test where the ASN structure is parsed before modification:
https://github.com/google/boringssl/blob/a7d9ac2af4684747c4524cbeba9737b04dce3e3e/crypto/fipsmodule/ecdsa/ecdsa_test.cc#L143

Risk Level: low
Testing: `bazel test //test/extensions/transport_sockets/tls:ssl_socket_test --runs_per_test=1000`
Docs Changes: N/A
Release Notes: N/A
Fixes: envoyproxy#8255 

Signed-off-by: Ismo Puustinen <[email protected]>
danzh2010 pushed a commit to danzh2010/envoy that referenced this issue Oct 4, 2019
Description:

Fix a test flake.

If crypto error option is set in the signing test, just leave the digest to zeroes instead of trying to modify it. The digest modification might just cause ASN object structure changes.

BoringSSL source appears to have a similar test where the ASN structure is parsed before modification:
https://github.com/google/boringssl/blob/a7d9ac2af4684747c4524cbeba9737b04dce3e3e/crypto/fipsmodule/ecdsa/ecdsa_test.cc#L143

Risk Level: low
Testing: `bazel test //test/extensions/transport_sockets/tls:ssl_socket_test --runs_per_test=1000`
Docs Changes: N/A
Release Notes: N/A
Fixes: envoyproxy#8255 

Signed-off-by: Ismo Puustinen <[email protected]>
danzh2010 pushed a commit to danzh2010/envoy that referenced this issue Oct 4, 2019
Description:

Fix a test flake.

If crypto error option is set in the signing test, just leave the digest to zeroes instead of trying to modify it. The digest modification might just cause ASN object structure changes.

BoringSSL source appears to have a similar test where the ASN structure is parsed before modification:
https://github.com/google/boringssl/blob/a7d9ac2af4684747c4524cbeba9737b04dce3e3e/crypto/fipsmodule/ecdsa/ecdsa_test.cc#L143

Risk Level: low
Testing: `bazel test //test/extensions/transport_sockets/tls:ssl_socket_test --runs_per_test=1000`
Docs Changes: N/A
Release Notes: N/A
Fixes: envoyproxy#8255 

Signed-off-by: Ismo Puustinen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants