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>

diff --git a/lib/util/data_blob.c b/lib/util/data_blob.c
index 1b05809119dd986bb74b9897827d41075d342c85..3dccdc1c58ad8e88c529a53a6a7a0093f3fd5221 100644 (file)
--- 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 0f3eae16592185c8230b08e9ab40b6500b494010..44376b707765daaf743a0e2ba24536277295f8dc 100644 (file)
--- 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 369e3d94d3fa692586d9741780e8f461f9ca7c54..889e1e8acf0d2c373a171d33b339bd0cafc9db99 100644 (file)
--- 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 b1f4d9c2b04e4ec3e775ac483d45dce701341b2b..cc92c84cc0795d15dd1b3c1c581bd295b5d7173c 100644 (file)
--- 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 7978b20555cb1aab07c17c4c9e50c04124442a11..eab57da46509391c4f383f38298f7657694b7bc1 100644 (file)
--- 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;