BLu85 / AES-GCM-128-192-256-bits

Configurable AES-GCM IP (128, 192, 256 bits)
19 stars 5 forks source link

Masking of AAD Input Value #4

Closed seceng-jan closed 8 months ago

seceng-jan commented 12 months ago

HI!

Thanks for uploading the code, it helped me a lot! I believe there is a bug with the masking of the AAD input values. I found that when

aes_ghash_aad <= x"ffff0000000000000000000000000000";
aes_ghash_aad_bval <= x"c000";

the GCM tag is different to when I use

aes_ghash_aad <= x"ffff0000000000000000000000000001";
aes_ghash_aad_bval <= x"c000";

The first example returns the correct tag, the second one not. Here is my full testbench file:

library ieee;
use ieee.std_logic_1164.all;
use ieee.numeric_std.all;

use work.gcm_pkg.all;
use work.aes_pkg.all;

entity aes_tb is
end entity;

architecture default of aes_tb is
    constant t: time := 5 ns;
    signal clk, reset, done : std_logic;
    signal enable : std_logic;

    signal aes_mode: std_logic_vector(1 downto 0);
    signal aes_enc_dec : std_logic;
    signal aes_pipe_reset : std_logic;
    signal aes_key_word_val : std_logic_vector(3 downto 0);
    signal aes_key_word : std_logic_vector(AES_256_KEY_WIDTH_C-1 downto 0);
    signal aes_iv_val : std_logic;
    signal aes_iv : std_logic_vector(GCM_ICB_WIDTH_C-1 downto 0);
    signal aes_icb_start_cnt : std_logic;
    signal aes_icb_stop_cnt : std_logic;
    signal aes_ghash_pkt_val : std_logic;
    signal aes_ghash_aad_bval : std_logic_vector(NB_STAGE_C-1 downto 0);
    signal aes_ghash_aad : std_logic_vector(GCM_DATA_WIDTH_C-1 downto 0);
    signal aes_data_in_bval : std_logic_vector(NB_STAGE_C-1 downto 0);
    signal aes_data_in : std_logic_vector(AES_DATA_WIDTH_C-1 downto 0);

    -- Output
    signal aes_ready : std_logic;
    signal aes_data_out_val : std_logic;
    signal aes_data_out_bval :  std_logic_vector(NB_STAGE_C-1 downto 0);
    signal aes_data_out : std_logic_vector(AES_DATA_WIDTH_C-1 downto 0);
    signal aes_ghash_tag_val : std_logic;
    signal aes_ghash_tag_o :  std_logic_vector(GCM_DATA_WIDTH_C-1 downto 0);
    signal aes_overflow : std_logic;

begin
        uut: entity work.top_aes_gcm
        port map(
            enable => enable,
            rst_i => reset,
            clk_i => clk,
            aes_gcm_mode_i => aes_mode,
            aes_gcm_enc_dec_i => aes_enc_dec,
            aes_gcm_pipe_reset_i => aes_pipe_reset,
            aes_gcm_key_word_val_i => aes_key_word_val,
            aes_gcm_key_word_i => aes_key_word,
            aes_gcm_iv_val_i => aes_iv_val,
            aes_gcm_iv_i => aes_iv,
            aes_gcm_icb_start_cnt_i => aes_icb_start_cnt,
            aes_gcm_icb_stop_cnt_i => aes_icb_stop_cnt,
            aes_gcm_ghash_pkt_val_i => aes_ghash_pkt_val,
            aes_gcm_ghash_aad_bval_i => aes_ghash_aad_bval,
            aes_gcm_ghash_aad_i  => aes_ghash_aad,
            aes_gcm_data_in_bval_i => aes_data_in_bval,
            aes_gcm_data_in_i => aes_data_in,
            aes_gcm_ready_o  => aes_ready,
            aes_gcm_data_out_val_o => aes_data_out_val, 
            aes_gcm_data_out_bval_o  => aes_data_out_bval,
            aes_gcm_data_out_o  => aes_data_out, 
            aes_gcm_ghash_tag_val_o => aes_ghash_tag_val, 
            aes_gcm_ghash_tag_o  => aes_ghash_tag_o,
            aes_gcm_icb_cnt_overflow_o => aes_overflow
        );

    clk_gen: process
    begin
        clk <= '1';
        wait for t / 2;
        clk <= '0';
        wait for t / 2;
        if done = '1' then
            wait;
        end if;
    end process;

    test: process
        variable correct: boolean := true;
    begin
        enable <= '1';
        done <= '0';
        reset <= '1';
        aes_mode <= AES_MODE_128_C;
        aes_key_word_val <= "0000";
        aes_pipe_reset <= '1';
        aes_iv_val <= '0';
        aes_ghash_pkt_val <= '0';
        aes_ghash_aad_bval <= (others => '0');
        aes_data_in_bval <= (others => '0');
        aes_enc_dec <= '0';
        aes_icb_start_cnt <= '0';
        aes_icb_stop_cnt <= '0';
        wait for t;
        reset <= '0';
        aes_pipe_reset <= '0';
        wait for t;
        -- Load the key
        aes_key_word <= x"affeaffeaffeaffeaffeaffeaffeaffe00000000000000000000000000000000";
        aes_key_word_val(2) <= '1';
        wait for t;
        aes_key_word_val(2) <= '0';
        aes_key_word <= x"aaaeaffeaffeaffeaffeaffeaffeaffe00000000000000000000000000000000";
        wait for t;
        -- Load the IV
        aes_iv <= x"d2423c7f670eb2ad17469b86";
        aes_iv_val <= '1';
        wait for t;
        aes_iv_val <= '0';
        aes_icb_start_cnt <= '1';
        wait for t;
        aes_icb_start_cnt <= '0';
        wait until aes_ready = '1';

        aes_ghash_pkt_val <= '1';
        wait for 10*t;
        aes_ghash_aad <= x"ffff0000000000000000000000000000";
        aes_ghash_aad_bval <= x"c000";
        wait for t;
        aes_ghash_aad_bval <= x"0000";
        aes_data_in <= x"fffefdfcfbfaf9f8f7f6f5f4f3f2f1f0";
        aes_data_in_bval <= x"FFFF";
        wait for t;

        aes_data_in <= x"efeeedecebeae9e8e7e6e50000000001";
        aes_data_in_bval <= x"ffe0";
        wait for 3*t/2;
        aes_data_in_bval <= x"0000";
        aes_ghash_pkt_val <= '0';
        wait for t;
        wait for 100*t;
        done <= '1';
        wait;
    end process;

end architecture;
BLu85 commented 11 months ago

Hi @seceng-jan

Thanks for trying the code and reporting this.

I honestly don't think that this is an issue. If you take a look at the NIST 800-38D, you will notice that the AAD is formed with the number of data to authenticate and a padding of zeros to reach the block size.

...In Steps 4 and 5, the AAD and the ciphertext are each appended with the minimum number of ‘0’ bits, possibly none, so that the bit lengths of the resulting strings are multiples of the block size.

This means that the user should supply the AAD already padded.

The reason why this doesn't happen for the data, is because I wrongly masked them. In file gcm_gctr.vhd line 162: gctr_data_out <= (gctr_data_in_i xor aes_ecb_data) and gctr_data_mask;

should actually be: gctr_data_out <= gctr_data_in_i xor (aes_ecb_data and gctr_data_mask);

Hope this makes sense. Luca

seceng-jan commented 11 months ago

Hi Luca!

Thanks for the response. That does make sense. If you like, you can close this issue - don't know if you want to change gcm_gctr.vhd first.

Best regards, Jan

BLu85 commented 8 months ago

Although this wasn't an issue, I preferred to add the logic that masks the AAD and Data that are supplied to the GHASH block. This will avoid other users to end up into this issue. Commit can be found here