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,41 @@ 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;
+ LBio := 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
 {