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

OpenSSL 3.2.0 - sessions, time, signed vs unsigned, failure with negative session timeout values #703

Open
MSP-Greg opened this issue Nov 25, 2023 · 6 comments

Comments

@MSP-Greg
Copy link
Contributor

Very recently MSYS2 (Windows ucrt & mingw builds) upgraded their OpenSSL package from OpenSSL 3.1.4 to 3.2.0.

As we've seen, there are issues with a net-http test failing, which is setting http.ssl_timeout to -1.

A commit in OpenSSL (openssl/openssl@f0131dc) updates SSL_CTX_set_timeout in ssl/ssl_sess.c, and the commit only affects 3.2.0.

Some of the code seems to imply uint64_t for timeout values.

Another day where I'm AFK on and off...

@MSP-Greg
Copy link
Contributor Author

I just checked this issue, and it seems to behave the same for TLSv1.3 & TLSv1.2. See below, a negative ctx.timeout doesn't work with OpenSSL 3.2.0. JFYI, I patched a Puma test file, as I'm familiar with its session tests...

ctx.timeout -2      session.timeout: 1_266_874_887
ctx.timeout -1      session.timeout: 1_266_874_888
ctx.timeout  0      session.timeout: 7200  (ignored, 7_200 is default timeout)
ctx.timeout  1      session.timeout: 1
ctx.timeout  2      session.timeout: 2
ctx.timeout  1_000  session.timeout: 1_000

@MSP-Greg
Copy link
Contributor Author

Looked at this a bit more, and OpenSSL 1.1.x allows storing a negative session timeout, as noted in the data below:

  • OpenSSL 1.1.1f Ubuntu 20.04

    ctx.timeout   -2   session.timeout   -2
    ctx.timeout   -1   session.timeout   -1
    ctx.timeout    0   session.timeout 7200
    ctx.timeout    1   session.timeout    1
    ctx.timeout    2   session.timeout    2
    ctx.timeout    3   session.timeout    3
    ctx.timeout 1000   session.timeout 1000
    
  • OpenSSL 3.1.4? Windows mswin, Ubuntu 22.04

    ctx.timeout   -2   session.timeout    3
    ctx.timeout   -1   session.timeout    3
    ctx.timeout    0   session.timeout 7200
    ctx.timeout    1   session.timeout    1
    ctx.timeout    2   session.timeout    2
    ctx.timeout    3   session.timeout    3
    ctx.timeout 1000   session.timeout 1000
    
  • OpenSSL 3.2.0 Windows ucrt

    ctx.timeout   -2   session.timeout 1266874887
    ctx.timeout   -1   session.timeout 1266874888
    ctx.timeout    0   session.timeout 7200
    ctx.timeout    1   session.timeout    1
    ctx.timeout    2   session.timeout    2
    ctx.timeout    3   session.timeout    3
    ctx.timeout 1000   session.timeout 1000
    

net/http contains the following code:

if @ssl_session and
   Process.clock_gettime(Process::CLOCK_REALTIME) < @ssl_session.time.to_f + @ssl_session.timeout
  s.session = @ssl_session
end

It's been a while since I've reviewed session negotiation, etc, and that was mostly TLSv1.3 & TLSv1.2. I thought timeout was normally associated with cache mechanisms, and hence, used on the server, not the client.

The net/http code is using it to determine whether to use the session in a client request, so its use of session.timeout has nothing to do with OpenSSL functionality?

@rhenium
Copy link
Member

rhenium commented Nov 30, 2023

Thank you for digging into this!

IIUC OpenSSL 3.2.0 didn't intend to break backwards-compatibility, but setting the timeout value to a negative number doesn't seem like a feature.

The SSLeay documentation from 1997 (or even before) suggests that it was actually intended to be unsigned long.

https://github.com/openssl/openssl/blob/0c106d75e38032d97d29f864bb772454beb5632f/doc/session.doc#L178-L184

You can also use
SSL_CTX_set_timeout(SSL_CTX *ctx,unsigned long t) and
SSL_CTX_get_timeout(SSL_CTX *ctx) to manipulate the default timeouts for
all SSL connections created against a SSL_CTX. If you set a timeout in
an SSL_CTX, all new SSL's created will inherit the timeout. It can be over
written by the SSL_set_timeout(SSL *s,unsigned long t) function call.
If you 'set' the timeout back to 0, the system default will be used.

(As with many other accessor functions in SSLeay/OpenSSL, the actual backing field on SSL_CTX was a signed long, however.)

The current manpages don't say anything about when setting it to zero or a negative value.

@MSP-Greg
Copy link
Contributor Author

Thank you for digging into this!

That's about as far I feel comfortable going.

I thought maybe throwing an error if the value is set to a negative (and maybe zero) value would be helpful. I can't recall if somewhere in the OpenSSL code I saw something that took a zero value as 'use the default', not sure.

but setting the timeout value to a negative number doesn't seem like a feature.

Agreed. But, net/http seems to be making use of it to determine whether to reuse a session. Hence, the PR there that allows it work by using the context (negative) value, then using the session value if the context value is nil. If the context value is positive, it's used as intended. I added a comment that hopefully was clear.

I think that made sense...

@MSP-Greg
Copy link
Contributor Author

@rhenium

JFYI, MSFT/vcpkg just updated to OpenSSL 3.2.0. So, all Windows head builds are using 3.2.0, as MSYS2 previously updated to it.

Happy Holidays / Merry Christmas

Greg

@MSP-Greg
Copy link
Contributor Author

@rhenium

I just updated build tools locally, and noticed that all the Ruby head builds are using OpenSSL OpenSSL 3.3.0 9 Apr 2024...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants