r19502: fixed the RPC-SECRETS test with kerberos. Andrew, can you look at this

as well?

The server side change is needed to fix a valgrind error, which was
possibly exploitable if the client sent deliberately bad data
(This used to be commit e3c04cf165fe15739197b2713e78046399aa7653)

diff --git a/source4/librpc/rpc/dcerpc.c b/source4/librpc/rpc/dcerpc.c
index 28e48c4a5aeceae02c864e45bc39f7365e58fcf7..bda07066ffbd6f1a8acb28e813e9b300926e93f3 100644 (file)
--- a/source4/librpc/rpc/dcerpc.c
+++ b/source4/librpc/rpc/dcerpc.c
@@ -272,7 +272,6 @@ static NTSTATUS ncacn_pull_request_auth(struct dcerpc_connection *c, TALLOC_CTX
                return status;
        }
        
-       
        /* check signature or unseal the packet */
        switch (c->security_state.auth_info->auth_level) {
        case DCERPC_AUTH_LEVEL_PRIVACY:
@@ -433,6 +432,13 @@ static NTSTATUS ncacn_push_request_sign(struct dcerpc_connection *c,
                        return status;
                }
                dcerpc_set_auth_length(blob, creds2.length);
+               if (c->security_state.auth_info->credentials.length == 0) {
+                       /* this is needed for krb5 only, to correct the total packet
+                          length */
+                       dcerpc_set_frag_length(blob, 
+                                              dcerpc_get_frag_length(blob)
+                                              +creds2.length);
+               }
                break;
 
        case DCERPC_AUTH_LEVEL_INTEGRITY:
@@ -454,6 +460,13 @@ static NTSTATUS ncacn_push_request_sign(struct dcerpc_connection *c,
                        return status;
                }
                dcerpc_set_auth_length(blob, creds2.length);
+               if (c->security_state.auth_info->credentials.length == 0) {
+                       /* this is needed for krb5 only, to correct the total packet
+                          length */
+                       dcerpc_set_frag_length(blob, 
+                                              dcerpc_get_frag_length(blob)
+                                              +creds2.length);
+               }
                break;
 
        case DCERPC_AUTH_LEVEL_CONNECT:

diff --git a/source4/rpc_server/dcesrv_auth.c b/source4/rpc_server/dcesrv_auth.c
index e6e9bb7fc5ce21fc7742e73c2d9b3e835ecb6e53..b73143ce34c3442570a8c067aefbb11df6df7871 100644 (file)
--- a/source4/rpc_server/dcesrv_auth.c
+++ b/source4/rpc_server/dcesrv_auth.c
@@ -470,19 +470,14 @@ BOOL dcesrv_auth_response(struct dcesrv_call_state *call,
                                            &creds2);
 
                if (NT_STATUS_IS_OK(status)) {
-                       status = data_blob_realloc(call, blob,
-                                                  blob->length - dce_conn->auth_state.auth_info->credentials.length + 
-                                                  creds2.length);
-               }
-
-               if (NT_STATUS_IS_OK(status)) {
-                       memcpy(blob->data + blob->length - dce_conn->auth_state.auth_info->credentials.length,
-                              creds2.data, creds2.length);
+                       blob->length -= dce_conn->auth_state.auth_info->credentials.length;
+                       status = data_blob_append(call, blob, creds2.data, creds2.length);
                }
 
                /* If we did AEAD signing of the packet headers, then we hope
                 * this value didn't change... */
                dcerpc_set_auth_length(blob, creds2.length);
+               dcerpc_set_frag_length(blob, dcerpc_get_frag_length(blob)+creds2.length);
                data_blob_free(&creds2);
                break;
 
@@ -495,20 +490,14 @@ BOOL dcesrv_auth_response(struct dcesrv_call_state *call,
                                            blob->length - dce_conn->auth_state.auth_info->credentials.length,
                                            &creds2);
                if (NT_STATUS_IS_OK(status)) {
-                       status = data_blob_realloc(call, blob,
-                                                  blob->length - dce_conn->auth_state.auth_info->credentials.length + 
-                                                  creds2.length);
-               }
-
-               if (NT_STATUS_IS_OK(status)) {
-                       memcpy(blob->data + blob->length - dce_conn->auth_state.auth_info->credentials.length,
-                              creds2.data, creds2.length);
+                       blob->length -= dce_conn->auth_state.auth_info->credentials.length;
+                       status = data_blob_append(call, blob, creds2.data, creds2.length);
                }
 
                /* If we did AEAD signing of the packet headers, then we hope
                 * this value didn't change... */
                dcerpc_set_auth_length(blob, creds2.length);
-
+               dcerpc_set_frag_length(blob, dcerpc_get_frag_length(blob)+creds2.length);
                data_blob_free(&creds2);
                break;