fffonion / lua-resty-openssl

FFI-based OpenSSL binding for OpenResty
BSD 2-Clause "Simplified" License
130 stars 43 forks source link

EVP_PKEY_derive error on 32-bit systems due to incorrect type casting #180

Closed OmentaElvis closed 1 month ago

OmentaElvis commented 1 month ago

When running OpenResty on a 32-bit system, I encountered an error originating from lua-resty-sessions, which originated from lua-resty-openssl. The specific error message is:

/usr/local/openresty/site/lualib/resty/openssl/kdf.lua:281: bad argument #3 to 'EVP_PKEY_derive' (cannot convert 'uint64_t' to 'unsigned int *')

The error occurs on invokation of EVP_PKEY_derive

After tracing the parameters, I found that the issue is caused by the outlen variable in kdf.lua being initialized as

local outlen = ctypes.ptr_of_uint64()

However, the target FFI function EVP_PKEY_derive expects a size_t* argument, which is 32-bit on a 32-bit machine.

The problem arises because size_t is architecture-dependent, and the current implementation assumes a 64-bit system. To ensure cross-compatibility, the outlen variable should be initialized as

local outlen = ctypes.ptr_of_size_t()

since there is already a ptr_of_size_t() in ctypes definitions.

Steps to reproduce:

  1. Run OpenResty on a 32-bit system.
  2. Use lua-resty-session, which depends on lua-resty-openssl.
  3. Run this example from lua-resty-session documentation.
    
    worker_processes  1;

events { worker_connections 1024; }

http { init_by_lua_block { require "resty.session".init({ remember = true, audience = "demo", secret = "RaJKp8UQW1", storage = "cookie", }) }

server { listen 8080; server_name localhost; default_type text/html;

location /start {
  content_by_lua_block {
    local session = require "resty.session".new()
    session:set_subject("OpenResty Fan")
    session:set("quote", "The quick brown fox jumps over the lazy dog")
    local ok, err = session:save()

    ngx.say(string.format([[
      <html>
      <body>
        <p>Session started (%s)</p>
        <p><a href=/started>Check if it really was</a></p>
      </body>
      </html>
    ]], err or "no error"))
  }
}

} }


**Expected behavior:**

The code should run without errors on 32-bit systems.

**Proposed fix:**

Change the initialization of `outlen` to `local outlen = ctypes.ptr_of_size_t()` in `kdf.lua`

## Info
```bash
$ openresty -V 2>&1 | sed s/--/\\n--/g

nginx version: openresty/1.25.3.2
built by gcc 12.2.0 (Debian 12.2.0-14) 
built with OpenSSL 3.0.9 30 May 2023
TLS SNI support enabled
configure arguments: 
--prefix=/usr/local/openresty/nginx 
--with-cc-opt=-O2 
--add-module=../ngx_devel_kit-0.3.3 
--add-module=../echo-nginx-module-0.63 
--add-module=../xss-nginx-module-0.06 
--add-module=../ngx_coolkit-0.2 
--add-module=../set-misc-nginx-module-0.33 
--add-module=../form-input-nginx-module-0.12 
--add-module=../encrypted-session-nginx-module-0.09 
--add-module=../srcache-nginx-module-0.33 
--add-module=../ngx_lua-0.10.26 
--add-module=../ngx_lua_upstream-0.07 
--add-module=../headers-more-nginx-module-0.37 
--add-module=../array-var-nginx-module-0.06 
--add-module=../memc-nginx-module-0.20 
--add-module=../redis2-nginx-module-0.15 
--add-module=../redis-nginx-module-0.3.9 
--add-module=../rds-json-nginx-module-0.16 
--add-module=../rds-csv-nginx-module-0.09 
--add-module=../ngx_stream_lua-0.0.14 
--with-ld-opt=-Wl,-rpath,/usr/local/openresty/luajit/lib 
--with-stream 
--without-pcre2 
--with-stream_ssl_module 
--with-stream_ssl_preread_module 
--with-http_ssl_module
$ lscpu
Architecture:        armv8l
Byte Order:          Little Endian
CPU(s):              8
On-line CPU(s) list: 0-7
Vendor ID:           ARM
Model name:          Cortex-A53
Model:               4
Thread(s) per core:  1
Core(s) per socket:  4
Socket(s):           2
Stepping:            r0p4
CPU(s) scaling MHz:  82%
CPU max MHz:         2301.0000
CPU min MHz:         400.0000
BogoMIPS:            26.00
Flags:               half thumb fastmult vfp edsp neon vfpv3 tls vfpv4 idiva idivt lpae evtstrm aes pmull sha1 sha2 crc32
$ ldd $(which openresty)
    libcrypt.so.1 => /lib/arm-linux-gnueabihf/libcrypt.so.1 (0xec880000)
    libluajit-5.1.so.2 => /usr/local/openresty/luajit/lib/libluajit-5.1.so.2 (0xec82d000)
    libm.so.6 => /lib/arm-linux-gnueabihf/libm.so.6 (0xec7ec000)
    libpcre.so.3 => /lib/arm-linux-gnueabihf/libpcre.so.3 (0xec770000)
    libssl.so.3 => /lib/arm-linux-gnueabihf/libssl.so.3 (0xec706000)
    libcrypto.so.3 => /lib/arm-linux-gnueabihf/libcrypto.so.3 (0xec47e000)
    libz.so.1 => /lib/arm-linux-gnueabihf/libz.so.1 (0xec440000)
    libc.so.6 => /lib/arm-linux-gnueabihf/libc.so.6 (0xec329000)
    /lib/ld-linux-armhf.so.3 (0x0f000000)
    libgcc_s.so.1 => /lib/arm-linux-gnueabihf/libgcc_s.so.1 (0xec2f0000)

Note I know little about cryptography but I am a c dev and I believe this was not the intended behaviour. I tried to patch my copy and it worked but restrained myself from making a pull request to avoid breaking stuff.

fffonion commented 1 month ago

Hi @OmentaElvis we currently doesn't support this module on 32bit. All cdef is assuming it's 64 bit. PR's are welcomed!

OmentaElvis commented 1 month ago

I would still recommend use of correct types as requested by external c functions regardless of the system. Using different types may lead to undefined behaviour even if it matches the desired bit width. In this case EVP_PKEY_derive expects a size_t* and the ctypes has the defination for ptr_of_size_t which is the correct type required.

That will ensure correctness and making it more platform independent without worrying about compatibility in the first place.

fffonion commented 1 month ago

ptr_of_size_t is actually ffi.typeof("size_t[1]") which is the correct way to create a size_t* and at same time initialize the address of the pointer that points to.

OmentaElvis commented 1 month ago

Is that not the intended type? I followed through the ffi code

local outlen = ctypes.ptr_of_size_t() -- size_t*
outlen[0] = 32 -- assign example value

C.EVP_PKEY_derive(ctx, buf, outlen) -- pass the ptr to size_t
fffonion commented 1 month ago

I see your point, you are mentioning the error in kdf, but i was looking at the usage of EVP_PKEY_derive in pkey.