Jump to content

Ralf Junker

Members
  • Content Count

    13
  • Joined

  • Last visited

Community Reputation

0 Neutral
  1. Agreed. But maybe they are used or will be used somewhere else?
  2. EVP_PKEY_paramgen() pkey should be a double pointer parameter. OverbyteIcsLIBEAY.pas line 1752, SVN 2063: f_EVP_PKEY_paramgen: function(pctx: PEVP_PKEY_CTX; pkey: PEVP_PKEY): Integer; cdecl = Nil; Notice the two ** asterisks in the OpenSSL's evp.h C declaration: int EVP_PKEY_paramgen(EVP_PKEY_CTX *ctx, EVP_PKEY **ppkey); Possible fixes: { More C like: Better if ppkey may be nil. } f_EVP_PKEY_paramgen: function(pctx: PEVP_PKEY_CTX; ppkey: PPEVP_PKEY): Integer; cdecl = Nil; { More Pascal like. Notice that nil cannot be passed this way. } f_EVP_PKEY_paramgen: function(pctx: PEVP_PKEY_CTX; var pkey: PEVP_PKEY): Integer; cdecl = Nil; Reference: https://www.openssl.org/docs/man1.1.1/man3/EVP_PKEY_paramgen.html Ralf
  3. f_CRYPTO_get_ex_data should return Pointer, not Integer. The code in question is at OverbyteIcsLIBEAY.pas line 1847, SVN 1476: f_CRYPTO_get_ex_data: function(r: PCRYPTO_EX_DATA; idx: integer): integer; cdecl = Nil; The corresponding C declaration from OpenSSL's crypto.h: void *CRYPTO_get_ex_data(const CRYPTO_EX_DATA *ad, int idx); Suggested solution: f_CRYPTO_get_ex_data: function(r: PCRYPTO_EX_DATA; idx: integer): Pointer; cdecl = Nil; Reference: https://www.openssl.org/docs/man1.1.1/man3/CRYPTO_get_ex_data.html Ralf
  4. At the time of this writing, TCryptoExNewFunc is declared as a function while it should be a procedure. OverbyteIcsLIBEAY.pas line 1752, SVN 1476: TCryptoExNewFunc = function(parent: Pointer; ptr: Pointer; ad: PCRYPTO_EX_DATA; idx: integer; argl: DWord; argp: Pointer): Integer; cdecl; This is the corresponding C declaration from OpenSSL's crypto.h, documented here: https://www.openssl.org/docs/man1.1.1/man3/CRYPTO_EX_new.html typedef void CRYPTO_EX_new (void *parent, void *ptr, CRYPTO_EX_DATA *ad, int idx, long argl, void *argp); This is the Pascal translation matching the C declaration: TCryptoExNewFunc = procedure(parent: Pointer; ptr: Pointer; ad: PCRYPTO_EX_DATA; idx: integer; argl: DWord; argp: Pointer); cdecl; Ralf
  5. In OverbyteIcsWSocket.pas, PunyServerName is always filled with 8-bit data, even if it is a 16-bit UnicodeString on Unicode Delphis. This happens in OverbyteIcsWSocket.pas line 15916 (SVN 1469): Move(DataExt[5], Ws.FCliHelloData.PunyServerName[1], Slen); The line above works well with non Unicode Delphis, but copies incorrect data in Unicode enabled Delphis. Cause is the declaration of PunyServerName (line 3488): TClientHelloData = record ServerName: String; PunyServerName: String; // <-- Problem here. ... Later on, this results in incorrect conversion to ServerName. See that the OverbyteIcsSslWebServ demo outputs garbled SNI and Server Name instead of localhost: HTTPS Server is waiting for connections https://localhost/demo.html HTTP Server is waiting for connections http://localhost/demo.html [10:17:36 127.0.0.1] SNI "潬慣桬獯t Client Hello: Server Name: 潬慣桬獯t [10:18:15 127.0.0.1] SNI "潬慣桬獯t Client Hello: Server Name: 潬慣桬獯t A quick remedy is to declare PunyServerName as AnsiString and work around the warning "OverbyteIcsWSocket.pas(15968): W1057 Implicit string cast from 'AnsiString' to 'string'", possibly be adding a typecast. TClientHelloData = record ServerName: String; PunyServerName: AnsiString; // <-- 8-bit string, even in Unicode Delphis. ... Ralf
  6. OverbyteIcsLIBEAY.pas line 1994 (SVN 1469) declares f_EVP_DigestSignInit() like this: f_EVP_DigestSignInit: function(ctx: PEVP_MD_CTX; pctx: PEVP_PKEY_CTX; etype: PEVP_MD; impl: PEngine; pkey: PEVP_PKEY): Integer; cdecl = Nil; In Pascal, pctx is a single pointer, but C declares pctx as a double pointer: int EVP_DigestSignInit(EVP_MD_CTX *ctx, EVP_PKEY_CTX **pctx, const EVP_MD *type, ENGINE *e, EVP_PKEY *pkey); Following nearby declarations, Pascal should probably change pctx to a var parameter. Reference: https://www.openssl.org/docs/man1.1.1/man3/EVP_DigestSignInit.html Ralf
  7. Thanks for the explanation. Removing outdated files seems reasonable as they likely confuse first time ICS users looking at the samples for good inspiration but end up confused. Until removal, might I suggest just to delete the offending line? It has no functional value anyway, its sole purpose is informational. This it would allow users of older Delphis to compile the OverbyteIcsHttpsServer1 sample without confusion. Ralf
  8. OverbyteIcsSSLEAY.pas line 2160 (SVN 1464) declares f_SSL_set_msg_callback() to use a TProto_msg_cb callback. TProto_msg_cb is defined in line 1270 as a function: TProto_msg_cb = function (write_p, version, content_type: integer; buf: PAnsiChar; size_t: integer; ssl: PSSL; arg: Pointer): Integer; cdecl; { V8.40 handshake protocol message callback } This is different from the OpenSSL definition. The C header declares the callback as a procedure which does not return a value (void): void SSL_CTX_set_msg_callback(SSL_CTX *ctx, void (*cb)(int write_p, int version, int content_type, const void *buf, size_t len, SSL *ssl, void *arg)); Reference: https://www.openssl.org/docs/man1.1.1/man3/SSL_set_msg_callback.html Ralf
  9. OverbyteIcsSSLEAY.pas line 2097 (SVN 1464) declares f_SSL_clear() like this: f_SSL_clear: procedure(S: PSSL); cdecl = nil; This is different from the OpenSSL definition. The Pascal declaration is a procedure, but the C declaration a function with an Integer return value: int SSL_clear(SSL *ssl); Reference: https://www.openssl.org/docs/man1.1.1/man3/SSL_clear.html Ralf
  10. OverbyteIcsSSLEAY.pas line 2095 (SVN 1464) declares f_SSL_bytes_to_cipher_list() like this: f_SSL_bytes_to_cipher_list: function(s: PSSL; cbytes: PAnsiChar; len: size_t; isv2format: Boolean; sk, scvsvs: PSTACK_OF_SSL_CIPHER): LongInt; cdecl = nil; { V8.64 } This is different from the OpenSSL definition. The Pascal declaration passes sk and scvsvs as simple pointers, instead of pointer addresses or var parameters. In contrast, the C header uses a double pointer for both parameters: int SSL_bytes_to_cipher_list(SSL *s, const unsigned char *bytes, size_t len, int isv2format, STACK_OF(SSL_CIPHER) **sk, STACK_OF(SSL_CIPHER) **scsvs); Reference: https://www.openssl.org/docs/man1.1.1/man3/SSL_bytes_to_cipher_list.html Ralf
  11. Samples\Delphi\SslInternet\OverbyteIcsHttpsServer1.pas line 415 has a reference to OverbyteIcsWSocket.SslWSocketCopyRight which does not exist: Display(' ' + Trim(OverbyteIcsWSocket.SslWSocketCopyRight)); As a result, the OverbyteIcsHttpsServer1.pas unit does not compile. Ralf
  12. OverbyteIcsLIBEAY.pas line 2336 (SVN 1464) declares f_X509_check_ip_asc() like this: f_X509_check_ip_asc: function(Cert: PX509; Paddress: PAnsiChar; namelen: size_t; flags: Cardinal): Integer; cdecl = nil; This is different from the OpenSSL definition. The Pascal declaration has an extra namelen parameter not present in the C header: int X509_check_ip_asc(X509 *, const char *address, unsigned int flags); Reference: https://www.openssl.org/docs/man1.1.1/man3/X509_check_ip_asc.html Ralf
  13. OverbyteIcsWSocket.pas has at least one small memory leak. In line 17009 (SVN 1464), an X509Obj is duplicated before its is added to the list. However, TX509Base.Create() duplicates the duplication once more before TX509List.Add() stores it to the list. The first duplicated object is never freed, resulting in the leak. This is the line in question: CertList.Add(f_X509_dup(f_X509_OBJECT_get0_X509 (MyX509Obj))); The fix is to remove f_X509_dup(), so the line looks like this: CertList.Add(f_X509_OBJECT_get0_X509 (MyX509Obj)); Code analysis suggests that a similar leak is present in line 15012, but I have not been able to trigger this code running one of the Sample projects: Cert.AddToInters(f_X509_dup(f_X509_OBJECT_get0_X509 (MyX509Obj))); Ralf
×