lib/util: Change function to data_blob_equal_const_time()

Since data_blob_cmp_const_time() doesn't act as an exact replacement for
data_blob_cmp(), and its return value is only ever compared with zero,
simplify it and emphasize the intention of checking equality by
returning a bool instead.

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

5 files changed, 17 insertions, 17 deletions

diff --git a/lib/util/data_blob.c b/lib/util/data_blob.c
index 1b05809119d..3dccdc1c58a 100644
--- a/lib/util/data_blob.c
+++ b/lib/util/data_blob.c
@@ -134,23 +134,23 @@ _PUBLIC_ int data_blob_cmp(const DATA_BLOB *d1, const DATA_BLOB *d2)
 check if two data blobs are equal, where the time taken should not depend on the
 contents of either blob.
 **/
-_PUBLIC_ int data_blob_cmp_const_time(const DATA_BLOB *d1, const DATA_BLOB *d2)
+_PUBLIC_ bool data_blob_equal_const_time(const DATA_BLOB *d1, const DATA_BLOB *d2)
 {
 	int ret;
 	if (d1->data == NULL && d2->data != NULL) {
-		return -1;
+		return false;
 	}
 	if (d1->data != NULL && d2->data == NULL) {
-		return 1;
+		return false;
 	}
-	if (d1->data == d2->data) {
-		return d1->length - d2->length;
+	if (d1->length != d2->length) {
+		return false;
 	}
-	ret = memcmp_const_time(d1->data, d2->data, MIN(d1->length, d2->length));
-	if (ret == 0) {
-		return d1->length - d2->length;
+	if (d1->data == d2->data) {
+		return true;
 	}
-	return ret;
+	ret = memcmp_const_time(d1->data, d2->data, d1->length);
+	return ret == 0;
 }
 
 /**
diff --git a/lib/util/data_blob.h b/lib/util/data_blob.h
index 0f3eae16592..44376b70776 100644
--- a/lib/util/data_blob.h
+++ b/lib/util/data_blob.h
@@ -90,7 +90,7 @@ _PUBLIC_ int data_blob_cmp(const DATA_BLOB *d1, const DATA_BLOB *d2);
 check if two data blobs are equal, where the time taken should not depend on the
 contents of either blob.
 **/
-_PUBLIC_ int data_blob_cmp_const_time(const DATA_BLOB *d1, const DATA_BLOB *d2);
+_PUBLIC_ bool data_blob_equal_const_time(const DATA_BLOB *d1, const DATA_BLOB *d2);
 
 /**
 print the data_blob as hex string
diff --git a/libcli/auth/netlogon_creds_cli.c b/libcli/auth/netlogon_creds_cli.c
index 369e3d94d3f..889e1e8acf0 100644
--- a/libcli/auth/netlogon_creds_cli.c
+++ b/libcli/auth/netlogon_creds_cli.c
@@ -630,7 +630,7 @@ bool netlogon_creds_cli_validate(struct netlogon_creds_cli_context *context,
 	DATA_BLOB blob2;
 	NTSTATUS status;
 	enum ndr_err_code ndr_err;
-	int cmp;
+	bool equal;
 
 	status = netlogon_creds_cli_get(context, frame, &creds2);
 	if (!NT_STATUS_IS_OK(status)) {
@@ -652,11 +652,11 @@ bool netlogon_creds_cli_validate(struct netlogon_creds_cli_context *context,
 		return false;
 	}
 
-	cmp = data_blob_cmp_const_time(&blob1, &blob2);
+	equal = data_blob_equal_const_time(&blob1, &blob2);
 
 	TALLOC_FREE(frame);
 
-	return (cmp == 0);
+	return equal;
 }
 
 static NTSTATUS netlogon_creds_cli_store_internal(
diff --git a/source3/rpc_server/netlogon/srv_netlog_nt.c b/source3/rpc_server/netlogon/srv_netlog_nt.c
index b1f4d9c2b04..cc92c84cc07 100644
--- a/source3/rpc_server/netlogon/srv_netlog_nt.c
+++ b/source3/rpc_server/netlogon/srv_netlog_nt.c
@@ -1577,7 +1577,7 @@ NTSTATUS _netr_ServerPasswordSet2(struct pipes_struct *p,
 	confounder_len = 512 - new_password.length;
 	enc_blob = data_blob_const(r->in.new_password->data, confounder_len);
 	dec_blob = data_blob_const(password_buf.data, confounder_len);
-	if (confounder_len > 0 && data_blob_cmp_const_time(&dec_blob, &enc_blob) == 0) {
+	if (confounder_len > 0 && data_blob_equal_const_time(&dec_blob, &enc_blob)) {
 		DBG_WARNING("Confounder buffer not encrypted Length[%zu]\n",
 			    confounder_len);
 		TALLOC_FREE(creds);
@@ -1592,7 +1592,7 @@ NTSTATUS _netr_ServerPasswordSet2(struct pipes_struct *p,
 				   new_password.length);
 	dec_blob = data_blob_const(password_buf.data + confounder_len,
 				   new_password.length);
-	if (data_blob_cmp_const_time(&dec_blob, &enc_blob) == 0) {
+	if (data_blob_equal_const_time(&dec_blob, &enc_blob)) {
 		DBG_WARNING("Password buffer not encrypted Length[%zu]\n",
 			    new_password.length);
 		TALLOC_FREE(creds);
diff --git a/source4/rpc_server/netlogon/dcerpc_netlogon.c b/source4/rpc_server/netlogon/dcerpc_netlogon.c
index 7978b20555c..eab57da4650 100644
--- a/source4/rpc_server/netlogon/dcerpc_netlogon.c
+++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c
@@ -873,7 +873,7 @@ static NTSTATUS dcesrv_netr_ServerPasswordSet2(struct dcesrv_call_state *dce_cal
 	confounder_len = 512 - new_password.length;
 	enc_blob = data_blob_const(r->in.new_password->data, confounder_len);
 	dec_blob = data_blob_const(password_buf.data, confounder_len);
-	if (confounder_len > 0 && data_blob_cmp_const_time(&dec_blob, &enc_blob) == 0) {
+	if (confounder_len > 0 && data_blob_equal_const_time(&dec_blob, &enc_blob)) {
 		DBG_WARNING("Confounder buffer not encrypted Length[%zu]\n",
 			    confounder_len);
 		return NT_STATUS_WRONG_PASSWORD;
@@ -887,7 +887,7 @@ static NTSTATUS dcesrv_netr_ServerPasswordSet2(struct dcesrv_call_state *dce_cal
 				   new_password.length);
 	dec_blob = data_blob_const(password_buf.data + confounder_len,
 				   new_password.length);
-	if (data_blob_cmp_const_time(&dec_blob, &enc_blob) == 0) {
+	if (data_blob_equal_const_time(&dec_blob, &enc_blob)) {
 		DBG_WARNING("Password buffer not encrypted Length[%zu]\n",
 			    new_password.length);
 		return NT_STATUS_WRONG_PASSWORD;