ndr_claims: only use compression if it actually reduces the size

I have captures showing that claims compression depends on the payload
itself and how well it compresses, instead of the pure length of the
payload.

E.g. a single string claim with a value of 68 'a'
characters has an unpressed size of 336
and compressed size is 335.
While a single string with random string s1
has an unpressed size of 504 and it's still
uncompressed on the wire.
A different random string s2 also has an unpressed
size of 504, but it is compressed into a size of 502.

So it really depends if the compression makes it actually
smaller than the uncompressed version.

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
Autobuild-User(master): Ralph Böhme <slow@samba.org>
Autobuild-Date(master): Fri Feb 14 11:56:49 UTC 2025 on atb-devel-224

diff --git a/librpc/idl/claims.idl b/librpc/idl/claims.idl
index c81d4718ce0976e3e27db5f50804e2114054bb23..64df11c858b56117b560b8453f228d8976fe1ecc 100644 (file)
--- a/librpc/idl/claims.idl
+++ b/librpc/idl/claims.idl
@@ -138,8 +138,8 @@ interface claims
                 * disabled compression entirely.
                 */
                [value(ndr_claims_actual_wire_compression_alg(r->compression_format,
-                                                             ndr_size_CLAIMS_SET_NDR(claims_set,
-                                                                                     ndr->flags)))] CLAIMS_COMPRESSION_FORMAT compression_format;
+                                                             claims_set,
+                                                             ndr->flags))] CLAIMS_COMPRESSION_FORMAT compression_format;
                [value(ndr_size_CLAIMS_SET_NDR(claims_set,
                                               ndr->flags))] uint32 uncompressed_claims_set_size;
                uint16 reserved_type;

diff --git a/librpc/ndr/ndr_claims.c b/librpc/ndr/ndr_claims.c
index bb77cf38a5b0c398c2427280b09ecbc96374a8f0..b8f5fae9f054988b0627bf6f2b3689cb91123bec 100644 (file)
--- a/librpc/ndr/ndr_claims.c
+++ b/librpc/ndr/ndr_claims.c
@@ -23,29 +23,24 @@ enum ndr_compression_alg ndr_claims_compression_alg(enum CLAIMS_COMPRESSION_FORM
        return NDR_COMPRESSION_INVALID;
 }
 
-
-enum CLAIMS_COMPRESSION_FORMAT ndr_claims_actual_wire_compression_alg(enum CLAIMS_COMPRESSION_FORMAT specified_compression,
-                                                                     size_t uncompressed_claims_size) {
-       if (uncompressed_claims_size < CLAIM_UPPER_COMPRESSION_THRESHOLD) {
-               return CLAIMS_COMPRESSION_FORMAT_NONE;
-       }
-
-       return specified_compression;
-}
-
-size_t ndr_claims_compressed_size(struct CLAIMS_SET_NDR *claims_set,
-                                 enum CLAIMS_COMPRESSION_FORMAT wire_alg,
-                                 int flags)
+static void ndr_claims_compressed_sizes(struct CLAIMS_SET_NDR *claims_set,
+                                       enum CLAIMS_COMPRESSION_FORMAT wire_alg,
+                                       int flags,
+                                       ssize_t *_uncompressed_size,
+                                       enum CLAIMS_COMPRESSION_FORMAT *_used_alg,
+                                       ssize_t *_compressed_size)
 {
        TALLOC_CTX *frame = NULL;
        DATA_BLOB tmp_blob;
        uint8_t * tmp_compressed;
        ssize_t compressed_size;
        enum ndr_err_code ndr_err;
-       enum CLAIMS_COMPRESSION_FORMAT actual_wire_alg;
 
        if (claims_set == NULL) {
-               return 0;
+               *_uncompressed_size = 0;
+               *_used_alg = CLAIMS_COMPRESSION_FORMAT_NONE;
+               *_compressed_size = 0;
+               return;
        }
 
        frame = talloc_stackframe();
@@ -56,17 +51,28 @@ size_t ndr_claims_compressed_size(struct CLAIMS_SET_NDR *claims_set,
                                       (ndr_push_flags_fn_t)ndr_push_CLAIMS_SET_NDR);
        if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
                DBG_ERR("Failed to push claims while determining compressed size\n");
+               *_uncompressed_size = -1;
+               *_used_alg = CLAIMS_COMPRESSION_FORMAT_NONE;
+               *_compressed_size = -1;
                TALLOC_FREE(frame);
-               return 0;
+               return;
        }
 
-       actual_wire_alg = ndr_claims_actual_wire_compression_alg(wire_alg,
-                                                                tmp_blob.length);
+       if (tmp_blob.length < CLAIM_UPPER_COMPRESSION_THRESHOLD) {
+               *_uncompressed_size = tmp_blob.length;
+               *_used_alg = CLAIMS_COMPRESSION_FORMAT_NONE;
+               *_compressed_size = tmp_blob.length;
+               TALLOC_FREE(frame);
+               return;
+       }
 
-       switch (actual_wire_alg) {
+       switch (wire_alg) {
        case CLAIMS_COMPRESSION_FORMAT_NONE:
+               *_uncompressed_size = tmp_blob.length;
+               *_used_alg = CLAIMS_COMPRESSION_FORMAT_NONE;
+               *_compressed_size = tmp_blob.length;
                TALLOC_FREE(frame);
-               return tmp_blob.length;
+               return;
 
        case CLAIMS_COMPRESSION_FORMAT_XPRESS_HUFF:
                compressed_size = lzxpress_huffman_compress_talloc(frame,
@@ -74,19 +80,76 @@ size_t ndr_claims_compressed_size(struct CLAIMS_SET_NDR *claims_set,
                                                                   tmp_blob.length,
                                                                   &tmp_compressed);
 
-               TALLOC_FREE(frame);
-
                if (compressed_size < 0) {
                        DBG_ERR("Failed to compress claims (for determining compressed size)\n");
-                       return 0;
+                       *_uncompressed_size = -1;
+                       *_used_alg = CLAIMS_COMPRESSION_FORMAT_NONE;
+                       *_compressed_size = -1;
+                       TALLOC_FREE(frame);
+                       return;
+               }
+               if (compressed_size >= tmp_blob.length) {
+                       *_uncompressed_size = tmp_blob.length;
+                       *_used_alg = CLAIMS_COMPRESSION_FORMAT_NONE;
+                       *_compressed_size = tmp_blob.length;
+                       TALLOC_FREE(frame);
+                       return;
                }
-               return compressed_size;
 
-       default:
+               *_uncompressed_size = tmp_blob.length;
+               *_used_alg = CLAIMS_COMPRESSION_FORMAT_XPRESS_HUFF;
+               *_compressed_size = compressed_size;
                TALLOC_FREE(frame);
+               return;
+
+       default:
                DBG_ERR("Invalid chosen compression algorithm while determining compressed claim size\n");
+               *_uncompressed_size = -1;
+               *_used_alg = CLAIMS_COMPRESSION_FORMAT_NONE;
+               *_compressed_size = -1;
+               TALLOC_FREE(frame);
+               return;
+       }
+}
+
+enum CLAIMS_COMPRESSION_FORMAT ndr_claims_actual_wire_compression_alg(enum CLAIMS_COMPRESSION_FORMAT specified_compression,
+                                                                     struct CLAIMS_SET_NDR *claims_set,
+                                                                     int flags)
+{
+       ssize_t uncompressed_size = -1;
+       enum CLAIMS_COMPRESSION_FORMAT used_alg = CLAIMS_COMPRESSION_FORMAT_NONE;
+       ssize_t compressed_size = -1;
+
+       ndr_claims_compressed_sizes(claims_set,
+                                   specified_compression,
+                                   flags,
+                                   &uncompressed_size,
+                                   &used_alg,
+                                   &compressed_size);
+
+       return used_alg;
+}
+
+size_t ndr_claims_compressed_size(struct CLAIMS_SET_NDR *claims_set,
+                                 enum CLAIMS_COMPRESSION_FORMAT wire_alg,
+                                 int flags)
+{
+       ssize_t uncompressed_size = -1;
+       enum CLAIMS_COMPRESSION_FORMAT used_alg = CLAIMS_COMPRESSION_FORMAT_NONE;
+       ssize_t compressed_size = -1;
+
+       ndr_claims_compressed_sizes(claims_set,
+                                   wire_alg,
+                                   flags,
+                                   &uncompressed_size,
+                                   &used_alg,
+                                   &compressed_size);
+       if (uncompressed_size == -1) {
+               DBG_ERR("Failed to push claims while determining compressed size\n");
                return 0;
        }
+
+       return compressed_size;
 }
 
 _PUBLIC_ enum ndr_err_code ndr_push_claims_tf_rule_set(struct ndr_push *ndr, ndr_flags_type ndr_flags, const struct claims_tf_rule_set *r)

diff --git a/librpc/ndr/ndr_claims.h b/librpc/ndr/ndr_claims.h
index 03f40466762c7e96d3810bfdfb1e28f3b8c42b38..add60403a6268afad2140caa8223f9360274b937 100644 (file)
--- a/librpc/ndr/ndr_claims.h
+++ b/librpc/ndr/ndr_claims.h
@@ -24,7 +24,8 @@
 
 enum ndr_compression_alg ndr_claims_compression_alg(enum CLAIMS_COMPRESSION_FORMAT wire_alg);
 enum CLAIMS_COMPRESSION_FORMAT ndr_claims_actual_wire_compression_alg(enum CLAIMS_COMPRESSION_FORMAT specified_compression,
-                                                                     size_t uncompressed_claims_size);
+                                                                     struct CLAIMS_SET_NDR *claims_set,
+                                                                     int flags);
 
 size_t ndr_claims_compressed_size(struct CLAIMS_SET_NDR *claims_set,
                                 enum CLAIMS_COMPRESSION_FORMAT wire_alg,