Added results of an internal review

Lib/Protocols/OpenSSL/IdOpenSSLContext.pas

@@ -111,7 +111,7 @@ implementation
   Math,
   SysUtils;
 
-function FreeString(const Value: string): Boolean;
+function TryFinalizeString(const Value: string): Boolean;
 var
   refCount: Integer;
 begin
@@ -154,13 +154,13 @@ function GetPasswordCallback(buf: PIdAnsiChar; size: TIdC_INT; rwflag: TIdC_INT;
 
   // Override memory to prevent reading password via memory dump
   FillChar(LPasswordBytes[0], LPasswordByteLength, 0);
-  FreeString(LPassword);
+  TryFinalizeString(LPassword);
 
   buf[size-1] := #0; // RLebeau: truncate the password if needed
   Result := IndyMin(LPasswordByteLength, size);
 end;
 
-function VerifyCallback(preverify_ok: TIdC_INT; x509_ctx: PX509_STORE_CTX): TIdC_INT; cdecl;
+function VerifyCallback(preverify_ok: TIdC_INT; x509_ctx: PX509_STORE_CTX): TIdC_INT; cdecl; //FI:C103
 var
   LCertOpenSSL: PX509;
   LCertIndy: TIdOpenSSLX509;
@@ -249,7 +249,6 @@ function TIdOpenSSLContext.Init(const AOptions: TIdOpenSSLOptionsBase): Boolean;
 
   FreeContext(FContext);
   FContext := SSL_CTX_new(TLS_method());
-//  FContext := SSL_CTX_new(TLS_method());
   if not Assigned(FContext) then
     EIdOpenSSLNewSSLCtxError.&Raise(RIdOpenSSLNewSSLCtxError);
 

Lib/Protocols/OpenSSL/IdOpenSSLContextServer.pas

@@ -53,6 +53,8 @@   TIdOpenSSLContextServer = class(TIdOpenSSLContext)
   protected
     function GetVerifyMode(const AOptions: TIdOpenSSLOptionsBase): TIdC_INT; override;
   public
+    constructor Create;
+
     function Init(const AOptions: TIdOpenSSLOptionsServer): Boolean;
     function CreateSocket: TIdOpenSSLSocket; override;
   end;
@@ -70,6 +72,12 @@ implementation
 
 { TIdOpenSSLContextServer }
 
+constructor TIdOpenSSLContextServer.Create;
+begin
+  inherited;
+  FSessionIdCtxFallback := -1;
+end;
+
 function TIdOpenSSLContextServer.CreateSocket: TIdOpenSSLSocket;
 begin
   Result := TIdOpenSSLSocketServer.Create(OpenSSLContext);
@@ -131,42 +139,40 @@ procedure TIdOpenSSLContextServer.SetSessionContext(
 var
   LBio: PBIO;
   LX509: PX509;
+  LSessionContext: PByte;
   LLen: Integer;
 begin
-  if ACAFile = '' then
-  begin
-    Randomize();
-    FSessionIdCtxFallback := Random(High(Integer));
-    if SSL_CTX_set_session_id_context(
-      AContext,
-      PByte(@FSessionIdCtxFallback),
-      SizeOf(FSessionIdCtxFallback)) <> 1 then
-    begin
-      EIdOpenSSLSessionIdContextError.&Raise();
-    end;
-    Exit;
-  end;
-
-  LBio := BIO_new_file(GetPAnsiChar(ACAFile), 'r');
-  if not Assigned(LBio) then
-    EIdOpenSSLSessionIdContextError.&Raise();
+  LX509 := nil;
   try
-    LX509 := PEM_read_bio_X509(LBio, nil, nil, nil);
-    if not Assigned(LX509) then
-      EIdOpenSSLSessionIdContextError.&Raise();
-    try
+    if ACAFile <> '' then
+    begin
+      LSessionContext := @FSessionIdCtx[0];
       LLen := SSL_MAX_SID_CTX_LENGTH;
-      FillChar(FSessionIdCtx[0], LLen, 0);
-      if X509_digest(LX509, EVP_sha1, @FSessionIdCtx[0], @LLen) <> 1 then
+
+      LBio := BIO_new_file(GetPAnsiChar(ACAFile), 'r');
+      if not Assigned(LBio) then
         EIdOpenSSLSessionIdContextError.&Raise();
 
-      if SSL_CTX_set_session_id_context(AContext, @FSessionIdCtx[0], LLen) <> 1 then
+      LX509 := PEM_read_bio_X509(LBio, nil, nil, nil);
+      if not Assigned(LX509) then
         EIdOpenSSLSessionIdContextError.&Raise();
-    finally
-      X509_free(LX509);
+
+      FillChar(FSessionIdCtx[0], LLen, 0);
+      if X509_digest(LX509, EVP_sha1, @FSessionIdCtx[0], @LLen) <> 1 then
+        EIdOpenSSLSessionIdContextError.&Raise();
+    end
+    else
+    begin
+      LSessionContext := @FSessionIdCtxFallback;
+      LLen := SizeOf(FSessionIdCtxFallback);
     end;
+
+    if SSL_CTX_set_session_id_context(AContext, LSessionContext, LLen) <> 1 then
+      EIdOpenSSLSessionIdContextError.&Raise();
   finally
-    BIO_free(LBio);
+    // Both are nil-safe
+    X509_free(LX509);
+    BIO_vfree(LBio);
   end;
 end;
 

Lib/Protocols/OpenSSL/IdOpenSSLLoader.pas

@@ -161,7 +161,7 @@ function TOpenSSLLoader.GetOpenSSLPath: string;
 function TOpenSSLLoader.Load: Boolean;
 var
   LLibCrypto, LLibSSL: TIdLibHandle;
-begin
+begin                                  //FI:C101
   Result := True;
   FLoadCount.Lock();
   try
@@ -256,7 +256,7 @@ procedure TOpenSSLLoader.SetOpenSSLPath(const Value: string);
 end;
 
 procedure TOpenSSLLoader.Unload;
-begin
+begin                            //FI:C101
   FLoadCount.Lock();
   try
     FLoadCount.Decrement();

Lib/Protocols/OpenSSL/IdOpenSSLUtils.pas

@@ -102,10 +102,10 @@ function BoolToInt(const ABool: Boolean): Integer;
 
 function IntToBool(const AInt: Integer): Boolean;
 begin
-  if BoolConverter[True] = AInt then
-    Result := True
+  if BoolConverter[False] = AInt then
+    Result := False
   else
-    Result := False;
+    Result := True;
 end;
 
 end.

Lib/Protocols/OpenSSL/IdOpenSSLX509.pas

@@ -304,43 +304,40 @@ function TIdOpenSSLX509Name.GetL: string;
 end;
 
 function TIdOpenSSLX509Name.GetNameEntry(const nid: TIdC_INT): string;
+
+  function TryGetPosition(const name: PX509_NAME; const nid: TIdC_INT; var lastpos: TIdC_INT): Boolean;
+  begin
+    lastpos := X509_NAME_get_index_by_NID(name, nid, lastpos);
+    Result := not (lastpos = -1);
+  end;
+
 var
   LPos: TIdC_INT;
   LEntry: PX509_NAME_ENTRY;
   LString: PASN1_STRING;
-  LBuffer: PPByte;
+  LBuffer: PByte;
   LReturn: TIdC_INT;
 begin
   Result := '';
-  New(LBuffer);
-  try
-    // First Entry
-    LPos := X509_NAME_get_index_by_NID(FName, nid, -1);
-    if LPos = -1 then
+
+  LPos := -1;
+  while TryGetPosition(FName, nid, LPos) do
+  begin
+    LEntry := X509_NAME_get_entry(FName, LPos);
+    if not Assigned(LEntry) then
       Exit;
+    LString := X509_NAME_ENTRY_get_data(LEntry);
 
-    repeat
-      LEntry := X509_NAME_get_entry(FName, LPos);
-      if not Assigned(LEntry) then
-        Exit;
-      LString := X509_NAME_ENTRY_get_data(LEntry);
-
-      LReturn := ASN1_STRING_to_UTF8(LBuffer, LString);
-      if LReturn < 0 then
-        Exit;
-      try
-        if not (Result = '') then
-          Result := Result + ' ';
-        Result := Result + GetString(PIdAnsiChar(LBuffer^));
-      finally
-        OPENSSL_free(LBuffer^);
-      end;
-
-      // Next Entry
-      LPos := X509_NAME_get_index_by_NID(FName, nid, LPos);
-    until LPos = -1;
-  finally
-    Dispose(LBuffer);
+    LReturn := ASN1_STRING_to_UTF8(@LBuffer, LString);
+    if LReturn < 0 then
+      Exit;
+    try
+      if not (Result = '') then
+        Result := Result + ' ';
+      Result := Result + GetString(PIdAnsiChar(LBuffer));
+    finally
+      OPENSSL_free(LBuffer);
+    end;
   end;
 end;
 

Lib/Protocols/OpenSSL/IntermediateCode/IdOpenSSLHeaders_dh.pas

@@ -84,15 +84,15 @@ interface
   EVP_PKEY_CTRL_DH_PAD                   = (EVP_PKEY_ALG_CTRL + 16);
 
 type
-  DH_meth_generate_key_cb = function(dh: PDH): TIdC_INT;
-  DH_meth_compute_key_cb = function(key: PByte; const pub_key: PBIGNUM; dh: PDH): TIdC_INT;
+  DH_meth_generate_key_cb = function(dh: PDH): TIdC_INT cdecl;
+  DH_meth_compute_key_cb = function(key: PByte; const pub_key: PBIGNUM; dh: PDH): TIdC_INT cdecl;
   DH_meth_bn_mod_exp_cb = function(
     const dh: PDH; r: PBIGNUM; const a: PBIGNUM;
     const p: PBIGNUM; const m: PBIGNUM;
-    ctx: PBN_CTX; m_ctx: PBN_MONT_CTX): TIdC_INT;
-  DH_meth_init_cb = function(dh: PDH): TIdC_INT;
-  DH_meth_finish_cb = function(dh: PDH): TIdC_INT;
-  DH_meth_generate_params_cb = function(dh: PDH; prime_len: TIdC_INT; generator: TIdC_INT; cb: PBN_GENCB): TIdC_INT;
+    ctx: PBN_CTX; m_ctx: PBN_MONT_CTX): TIdC_INT cdecl;
+  DH_meth_init_cb = function(dh: PDH): TIdC_INT cdecl;
+  DH_meth_finish_cb = function(dh: PDH): TIdC_INT cdecl;
+  DH_meth_generate_params_cb = function(dh: PDH; prime_len: TIdC_INT; generator: TIdC_INT; cb: PBN_GENCB): TIdC_INT cdecl;
 
 var
 {