Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <ecc2207e-4845-f467-1926-e675f78c6b2e@amd.com>
Date:   Mon, 10 Oct 2022 13:55:13 +0530
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Subject: Re: [PATCH] crypto: gcm - Provide minimal library implementation
Content-Language: en-US
To:     Ard Biesheuvel <ardb@kernel.org>
Cc:     linux-crypto@vger.kernel.org, herbert@gondor.apana.org.au,
        ebiggers@kernel.org, keescook@chromium.org, jason@zx2c4.com
References: <20221007152105.3057788-1-ardb@kernel.org>
 <3e395ad2-48f6-87f6-9042-a3ca21c0baba@amd.com>
 <CAMj1kXGNXYvS9_-RwzzO-VeZN8+ZErhxGiCbfbBA5thJ84RxYA@mail.gmail.com>
From:   "Nikunj A. Dadhania" <nikunj@amd.com>
In-Reply-To: <CAMj1kXGNXYvS9_-RwzzO-VeZN8+ZErhxGiCbfbBA5thJ84RxYA@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?RDRwbUxUNkEybHdEVkVCS0pieWVSd3JDNGpmZWc1clVuL3Y2Y0x3T2g2SzdN?=
 =?utf-8?B?bUFVei94M1d1S0ZNVWI3L293Rkd6UFJsb3lKMS9ROXhtVmNnMkUyRDBDaXA1?=
 =?utf-8?B?QXRheWlpbUZjYy80aWRSdVBhOE5YNHQrNUQvRUhURENhMzRDQ2EvSW1JdnFu?=
 =?utf-8?B?SnJnempuR21lQVJxRUFSV2N2aGY0ZHFFcW9nZE1mQ3pxdEVtcVByTWk3NzN4?=
 =?utf-8?B?ODVvOXZveld2SFBUNVpnUFpWKzhvOXZBYU9xaUxwRUExVkhROHNzR21lamRP?=
 =?utf-8?B?RUF4eGtXalRUR1hWTVo2bExBVkhWdC9EU2pHemVNUml0Smo2bDczelRRMWdG?=
 =?utf-8?B?WVk1Z0wzU3h4eWdqNnFaSkJCQldQZklMSUsrRVdaTTFtVDRoOTZXbG9WMzBs?=
 =?utf-8?B?T05qU2w1bDF6MWkyc0VON0Y0ZHZ3WTQ5WStDajRlRDJEN0NHUkZnbzcxTUEv?=
 =?utf-8?B?U1gvTThyZDdHNzFpWFMwNXUzb0hRNlNheWZWTENLQnAzcUgrWlp3NVh2L09n?=
 =?utf-8?B?Z3FJVFR6QXNtVEQrbXpkTHR4UzVHRjFESk4xeFNUSVF1TkdpTHBKNE9kWXUx?=
 =?utf-8?B?N0I1bE95ZHhMQU1zTXVocU5JdkQ0V3pIZ1Zmai9uRjNmV3JHTUVYQmMwMDhh?=
 =?utf-8?B?RHN5RzlYcjhwS083ei81Nk01MG1nQ0VOU3M1c2JkOFc2ZldCSitSMUhZci9V?=
 =?utf-8?B?enFhODFGV1QraWVPMUV2Y2NoVzNwOXlkY0huQitic2lUNjVRU2ZsZExkVEFk?=
 =?utf-8?B?aWlPZVE1Ni9hcjZZZTVJWFRjb2M5MFZlakZTSHdXRGFiM3psMmFOaDU0MzVU?=
 =?utf-8?B?cHNSbzRnT1BkMVd4UFAvK2tXaldXSVZMNlAwUW1Sd1pQZWtIYUdSK1QyeVUw?=
 =?utf-8?B?SkpHWU1sdldVTERSWUIrYnlqK0JpZzlkYUNvVm1UOVVvYklicnU4MDlXWEgr?=
 =?utf-8?B?MjdkVXE4T3pwQzNDZGQ2VjhJbFNzd01QVE5TblYvTTYyTXJUL1ZOSDZoZjFU?=
 =?utf-8?B?K0JleEduaVFvZzIyVXlrbC8zR0dYSWorWEVzUTR0dVpTR1oyWGdYM0VhTGtO?=
 =?utf-8?B?SndqekVKeTJQMnFyUlJseHhVOEc2MHZ0OEU1QmJPeGZJSjZBZExKVmVLQ2I1?=
 =?utf-8?B?ai91c2VRYkV1Yys2Ukc1Y0Q3bXBJanRaTHA2NGRSc3h1SUVpOUFXTHptRXlC?=
 =?utf-8?B?TGFmMWZnZWRQcGFGZnJIT1lzSloyRlVTNGUyMExMbDhZcDJXR0J2aFdLYXlt?=
 =?utf-8?B?MWd3ODl0b05uYjZpblUzeGZqckVhUy96WUlLV1g3RXRkYTZRTUV1c05BYjZh?=
 =?utf-8?B?dmNXbGtCZG1SbW51NGtnbDlNU042M0pGdThFOHd2SDhFSjJxdEZQVEkvejRv?=
 =?utf-8?B?VlhQU3BmS1FSU0UyTmJQdEVlOUM4TTNuZVFHZzQydXJYWStsQlBZMnVERml1?=
 =?utf-8?B?RENXZ2lxU0l0MkNRZ2pVQ09DanVjVndNQ2kzem9nRWtFNXM4QS9qWjQ5Z2dE?=
 =?utf-8?B?YVFtMVd4UkFsS1JuMkpxTTFxYmhwUEkwdHoxQzBDZHBOK3BlbjRFcHk0dlpi?=
 =?utf-8?B?QzFTd2VnMi9iL3lINWFYcDV0SWFrWGI5Tm5sTGhrS0QyRVdSRnY3Y1Y0eHdn?=
 =?utf-8?B?RWcvMlZEaHRFTnVYMXRKOWJYYkZsdHowdXhBTFJzZ1RmbitUWGtIWUtiejFR?=
 =?utf-8?B?TndNSU5TbkJseGZ4T2J6Zm9CUVpZSkthdnJHKzdKN25Mem91VENZVVRuZHg1?=
 =?utf-8?B?M0NKdDRMV0lIa0xScHFOQ1c2N24raXNXaG1KY2N2NGxPUGJvUkU2VURYYTEr?=
 =?utf-8?B?ZTZiL1ZWZVZQZ2VlOHpwekNYSFJqb2J0WTEvRHNGZ0JEUzQvb0tQcHd4YkIx?=
 =?utf-8?B?WmxiVVVkVGNpWUplSktzbmVtSUI0TDR2WUZEZnI4ampBcmcwQzAwUUdPWC9Y?=
 =?utf-8?B?RG1KUVk0OC9uWS9yVEN3VkgxSzRtTS9ZSlVWN2JaRFFuZHBwZkg2V2N6K1ZH?=
 =?utf-8?B?dVJ0eVp3NnJrUU5ienhGVWNwcTZmZ3pySWllZkJ2c0tmWVBNb2REWGR0S2hF?=
 =?utf-8?B?QkllOWJPN2U3cVhIZUxjRkU0Vkg5Q1dQY0R1dXkydE5ObHJkVm5PSG85Z0F3?=
 =?utf-8?Q?zvi7AuO/LjFf7QKtluS083tyT?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 21e8a462-cedc-43d9-47da-08daaa98feac
X-MS-Exchange-CrossTenant-AuthSource: DS7PR12MB6309.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Oct 2022 08:25:31.1581
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: TTYsm/Om7rP/ezZ1u3ESJQMEZgdwzwi3H1xIb9HgFgnXU1nf3pr4uAUJlqNbSsGwxahfn+V0ds0417Do4wD/UA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS7PR12MB5861
X-Spam-Status: No, score=-6.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-crypto.vger.kernel.org>
X-Mailing-List: linux-crypto@vger.kernel.org


On 10/10/22 13:45, Ard Biesheuvel wrote:
> On Mon, 10 Oct 2022 at 09:27, Nikunj A. Dadhania <nikunj@amd.com> wrote:
>>
>> Hi Ard,
>>
>> On 07/10/22 20:51, Ard Biesheuvel wrote:
>>> Implement a minimal library version of GCM based on the existing library
>>> implementations of AES and multiplication in GF(2^128). Using these
>>> primitives, GCM can be implemented in a straight-forward manner.
>>>
>>> GCM has a couple of sharp edges, i.e., the amount of input data
>>> processed with the same initialization vector (IV) should be capped to
>>> protect the counter from 32-bit rollover (or carry), and the size of the
>>> authentication tag should be fixed for a given key. [0]
>>>
>>> The former concern is addressed trivially, given that the function call
>>> API uses 32-bit signed types for the input lengths. It is still up to
>>> the caller to avoid IV reuse in general, but this is not something we
>>> can police at the implementation level.
>>>
>>> As for the latter concern, let's make the authentication tag size part
>>> of the key schedule, and only permit it to be configured as part of the
>>> key expansion routine.
>>>
>>> Note that table based AES implementations are susceptible to known
>>> plaintext timing attacks on the encryption key. The AES library already
>>> attempts to mitigate this to some extent, but given that the counter
>>> mode encryption used by GCM operates exclusively on known plaintext by
>>> construction (the IV and therefore the initial counter value are known
>>> to an attacker), let's take some extra care to mitigate this, by calling
>>> the AES library with interrupts disabled.
>>>
>>> [0] https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fnvlpubs.nist.gov%2Fnistpubs%2Flegacy%2Fsp%2Fnistspecialpublication800-38d.pdf&amp;data=05%7C01%7Cnikunj.dadhania%40amd.com%7C912c5b73be0146ca63db08daaa97ade4%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638009869061457627%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=sNHrpE3d7w%2BpT8dzAPiFbmvYLLIu9zsCnJ%2BFFxjzp%2Bk%3D&amp;reserved=0
>>>
>>> Cc: "Nikunj A. Dadhania" <nikunj@amd.com>
>>> Link: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fall%2Fc6fb9b25-a4b6-2e4a-2dd1-63adda055a49%40amd.com%2F&amp;data=05%7C01%7Cnikunj.dadhania%40amd.com%7C912c5b73be0146ca63db08daaa97ade4%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638009869061457627%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=oyUKkikTm6wsPy8vFHiS98TZpL4Hu%2BFz9aYX%2Fs37wk0%3D&amp;reserved=0
>>> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>>> ---
>>> +/**
>>> + * gcm_decrypt - Perform GCM decryption on a block of data
>>> + * @ctx:     The GCM key schedule
>>> + * @dst:     Pointer to the plaintext output buffer
>>> + * @src:     Pointer the ciphertext (may equal @dst for decryption in place)
>>> + * @crypt_len:       The size in bytes of the plaintext and ciphertext.
>>> + * @assoc:   Pointer to the associated data,
>>> + * @assoc_len:       The size in bytes of the associated data
>>> + * @iv:              The initialization vector (IV) to use for this block of data
>>> + *           (must be 12 bytes in size as per the GCM spec recommendation)
>>> + * @authtag: The address of the buffer in memory where the authentication
>>> + *           tag is stored.
>>> + *
>>> + * Returns 0 on success, or -EBADMSG if the ciphertext failed authentication.
>>> + * On failure, no plaintext will be returned.
>>> + */
>>> +int __must_check gcm_decrypt(const struct gcm_ctx *ctx, u8 *dst, const u8 *src,
>>> +                          int crypt_len, const u8 *assoc, int assoc_len,
>>> +                          const u8 iv[GCM_AES_IV_SIZE], const u8 *authtag)
>>> +{
>>> +     u8 tagbuf[AES_BLOCK_SIZE];
>>> +     __be32 ctr[4];
>>> +
>>> +     memcpy(ctr, iv, GCM_AES_IV_SIZE);
>>> +
>>> +     gcm_mac(ctx, src, crypt_len, assoc, assoc_len, ctr, tagbuf);
>>> +     if (crypto_memneq(authtag, tagbuf, ctx->authsize)) {
>>> +             memzero_explicit(tagbuf, sizeof(tagbuf));
>>> +             return -EBADMSG;
>>> +     }
>>
>> The gcm_mac computation seems to be broken in this version. When I receive the encrypted
>> packet back from the security processor the authtag does not match. Will debug further
>> and report back.
>>
> 
> Sorry to hear that. If you find out what's wrong, can you please
> provide a test vector that reproduces it so we can add it to the list?

My bad, it was wrong crypt_len that I was sending. Working fine now.

Tested-by: Nikunj A Dadhania <nikunj@amd.com>