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

crypto/tls: alertCloseNotify should be sent with alert level warning, not fatal #3413

Closed
gopherbot opened this issue Mar 27, 2012 · 4 comments
Closed
Milestone

Comments

@gopherbot
Copy link

@gopherbot gopherbot commented Mar 27, 2012

by blake.mizerany:

What steps will reproduce the problem?
If possible, include a link to a program on play.golang.org.

    You could try (I haven't):
        1. Sending bytes, from a crypto/tls.Conn, to a server that uses OpenSSL and watching the log.

    Or:
        1. Install postgres 9.1.3 with SSL support (run on default ip/port)
            http://www.postgresql.org/docs/9.1/static/ssl-tcp.html
        2. Create a database and a user, both called 'pqgotest'
        3. Run `go test` for https://github.com/bmizerany/pq
        4. See "SSL error: SSL error code 336151528" in postgres log (explained below)

What is the expected output?
    Not an error.

What do you see instead?
    In postgres log: "SSL error: SSL error code 336151528"

Which compiler are you using (5g, 6g, 8g, gccgo)?
    $ go tool
    6a
    6c
    6g
    6l
    ...

Which operating system are you using?
    $ uname -a
    Darwin Blakes-MacBook-Air.local 11.3.0 Darwin Kernel Version 11.3.0: Thu Jan 12 18:47:41 PST 2012; root:xnu-1699.24.23~1/RELEASE_X86_64 x86_64


Which version are you using?  (run 'go version')
    $ go version
    go version weekly.2012-03-13 +a2a95054f3bd

Please provide any additional information below.
    I started with:
    https://groups.google.com/d/topic/mailing.openssl.users/o6G1X1FedkE/discussion

    With that response, I learned Nginx issues with this same error:
    http://www.ruby-forum.com/topic/140712
    The authors imply it's their fault at the end, not OpenSSL.

    I am unable to reproduce in portgres with the `psql` client with SSL on.

    This leads me to believe `if c.handshakeComplete {` in tls.Conn.Close is incorrectly yielding `true`.

    My apologies for not being very helpful. I don't understood TLS enough too.
    If there is anything else I can do to help, I'm loaded and ready.
@benburkert

This comment has been minimized.

Copy link
Contributor

@benburkert benburkert commented Mar 28, 2012

Comment 1:

The postgres warning appears to be caused by the tls.Conn.Close method sending an
alertCloseNotify via the tls.Conn.sendAlert function. The tls.Conn.sendAlertLocked
function is hard coded to send any alerts with an alert level of fatal, except for
alertNoRenegotiation. OpenSSL expects a close_notify alert to have an alert level of
warning, unless an error has occurred:
https://github.com/mkrautz/openssl-mirror/blob/master/ssl/d1_pkt.c#L1020-1027
@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Mar 28, 2012

Comment 2:

Thanks for the great diagnosis, Ben.

Labels changed: added priority-later, removed priority-triage.

Status changed to Accepted.

@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Sep 12, 2012

Comment 3:

Labels changed: added go1.1.

@agl

This comment has been minimized.

Copy link
Contributor

@agl agl commented Oct 16, 2012

Comment 4:

This issue was closed by revision cfa1ba3.

Status changed to Fixed.

@rsc rsc added this to the Go1.1 milestone Apr 14, 2015
@rsc rsc removed the go1.1 label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 24, 2016
FiloSottile pushed a commit to FiloSottile/go that referenced this issue Oct 12, 2018
The RFC doesn't actually have an opinion on whether this is a fatal or
warning level alert, but common practice suggests that it should be a
warning.

This involves rebasing most of the tests.

Fixes golang#3413.

R=golang-dev, shanemhansen, rsc
CC=golang-dev
https://golang.org/cl/6654050
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.