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: Unwrap SSL socket before closing transport socket. #737

Merged
merged 2 commits into from
May 19, 2022

Conversation

maczal
Copy link
Contributor

@maczal maczal commented May 19, 2022

Unwrap the SSL socket before closing the underlying transport socket instead of just closing SSL socket, to avoid yielding a "The TLS connection was non-properly terminated" warning in gvmd.log on disconnect.

What:

Performing proper disconnect of a TLS connection to skip log warnings from gvmd.

Why:

TLSConnection uses a transport socket and TLS socket wrapped on top of it. When disconnecting, the TLS socket was simply closed. Such behavior caused a log warning on the gvmd side:

md   main:WARNING:2022-05-19 14h13.51 UTC:18703: read_from_client_tls: failed to read from client: The TLS connection was non-properly terminated.

How:

In the change, on disconnect, unwrap() is called first on the TLS socket that closes it and returns the underlying transport socket, then the underlying socket can be closed normally and no warning from the gvmd server is generated.

I tested it using following snippet and observing tail of gvmd.log:

from gvm.connections import TLSConnection
from gvm.transforms import EtreeCheckCommandTransform
from gvm.protocols.gmp import Gmp

conn = TLSConnection(hostname='127.0.0.1', port=9390, timeout=900)
transform=EtreeCheckCommandTransform()

with Gmp(connection=conn, transform=transform) as gmp:
    gmp.get_version()

Checklist:

  • Tests (N/A - needs gvmd to be tested)
  • CHANGELOG Improved socket handling for TLSConnection.disconnect()
  • Documentation N/A

Unwrap the SSL socket before closing the underlying transport socket instead of just closing SSL socket, to avoid yielding a "The TLS connection was non-properly terminated" warning in gvmd.log  on disconnect.
@maczal maczal requested a review from a team as a code owner May 19, 2022 14:34
@y0urself y0urself changed the title Update connections.py Fix: Unwrap SSL socket before closing transport socket. May 19, 2022
@y0urself y0urself enabled auto-merge (squash) May 19, 2022 14:37
gvm/connections.py Outdated Show resolved Hide resolved
auto-merge was automatically disabled May 19, 2022 14:40

Head branch was pushed to by a user without write access

@Dexus Dexus enabled auto-merge (squash) May 19, 2022 14:42
@bjoernricks bjoernricks requested a review from y0urself May 19, 2022 14:46
@y0urself y0urself added the make release To trigger GitHub release action. label May 19, 2022
@Dexus Dexus merged commit a1fc99e into greenbone:main May 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
make release To trigger GitHub release action.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants