[bug report] rxrpc: rxgk: Implement the yfs-rxgk security class (GSSAPI)

[Dan Carpenter]

Hello David Howells,

Commit 9d1d2b59341f ("rxrpc: rxgk: Implement the yfs-rxgk security
class (GSSAPI)") from Apr 11, 2025 (linux-next), leads to the
following (UNPUBLISHED) Smatch static checker warning:

	net/rxrpc/rxgk_app.c:65 rxgk_yfs_decode_ticket()
	warn: untrusted unsigned subtract. 'ticket_len - 10 * 4'

net/rxrpc/rxgk_app.c
    42 int rxgk_yfs_decode_ticket(struct rxrpc_connection *conn, struct sk_buff *skb,
    43                            unsigned int ticket_offset, unsigned int ticket_len,
    44                            struct key **_key)
    45 {
    46         struct rxrpc_key_token *token;
    47         const struct cred *cred = current_cred(); // TODO - use socket creds
    48         struct key *key;
    49         size_t pre_ticket_len, payload_len;
    50         unsigned int klen, enctype;
    51         void *payload, *ticket;
    52         __be32 *t, *p, *q, tmp[2];
    53         int ret;
    54 
    55         _enter("");
    56 
    57         /* Get the session key length */
    58         ret = skb_copy_bits(skb, ticket_offset, tmp, sizeof(tmp));
    59         if (ret < 0)
    60                 return rxrpc_abort_conn(conn, skb, RXGK_INCONSISTENCY, -EPROTO,
    61                                         rxgk_abort_resp_short_yfs_klen);
    62         enctype = ntohl(tmp[0]);
    63         klen = ntohl(tmp[1]);
    64 
--> 65         if (klen > ticket_len - 10 * sizeof(__be32))
                          ^^^^^^^^^^
I don't think we know that ticket_len is >= 40 so this bounds checking
might not work as intended.  Since it's unsigned then negative values
are treated as very high positive values.

    66                 return rxrpc_abort_conn(conn, skb, RXGK_INCONSISTENCY, -EPROTO,
    67                                         rxgk_abort_resp_short_yfs_key);
    68 

token_len is set in rxgk_verify_response().

net/rxrpc/rxgk.c
  1180  static int rxgk_verify_response(struct rxrpc_connection *conn,
  1181                                  struct sk_buff *skb)
  1182  {
  1183          const struct krb5_enctype *krb5;
  1184          struct rxrpc_key_token *token;
  1185          struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
  1186          struct rxgk_response rhdr;
  1187          struct rxgk_context *gk;
  1188          struct key *key = NULL;
  1189          unsigned int offset = sizeof(struct rxrpc_wire_header);
  1190          unsigned int len = skb->len - sizeof(struct rxrpc_wire_header);
  1191          unsigned int token_offset, token_len;
  1192          unsigned int auth_offset, auth_len;
  1193          __be32 xauth_len;
  1194          int ret, ec;
  1195  
  1196          _enter("{%d}", conn->debug_id);
  1197  
  1198          /* Parse the RXGK_Response object */
  1199          if (sizeof(rhdr) + sizeof(__be32) > len)
  1200                  goto short_packet;
  1201  
  1202          if (skb_copy_bits(skb, offset, &rhdr, sizeof(rhdr)) < 0)
  1203                  goto short_packet;
  1204          offset  += sizeof(rhdr);
  1205          len     -= sizeof(rhdr);
  1206  
  1207          token_offset    = offset;
  1208          token_len       = ntohl(rhdr.token_len);
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  1209          if (xdr_round_up(token_len) + sizeof(__be32) > len)
  1210                  goto short_packet;

There is an upper bounds check but no lower bounds check.

  1211  
  1212          trace_rxrpc_rx_response(conn, sp->hdr.serial, 0, sp->hdr.cksum, token_len);
  1213  
  1214          offset  += xdr_round_up(token_len);
  1215          len     -= xdr_round_up(token_len);
  1216  
  1217          if (skb_copy_bits(skb, offset, &xauth_len, sizeof(xauth_len)) < 0)
  1218                  goto short_packet;
  1219          offset  += sizeof(xauth_len);
  1220          len     -= sizeof(xauth_len);
  1221  
  1222          auth_offset     = offset;
  1223          auth_len        = ntohl(xauth_len);
  1224          if (auth_len < len)
  1225                  goto short_packet;
  1226          if (auth_len & 3)
  1227                  goto inconsistent;
  1228          if (auth_len < 20 + 9 * 4)
  1229                  goto auth_too_short;
  1230  
  1231          /* We need to extract and decrypt the token and instantiate a session
  1232           * key for it.  This bit, however, is application-specific.  If
  1233           * possible, we use a default parser, but we might end up bumping this
  1234           * to the app to deal with - which might mean a round trip to
  1235           * userspace.
  1236           */
  1237          ret = rxgk_extract_token(conn, skb, token_offset, token_len, &key);
                                                                  ^^^^^^^^^

  1238          if (ret < 0)
  1239                  goto out;
  1240  

regards,
dan carpenter