From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 99FBE21ECCB15 for ; Wed, 20 Sep 2017 05:42:57 -0700 (PDT) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga104.jf.intel.com with ESMTP; 20 Sep 2017 05:46:02 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.42,421,1500966000"; d="scan'208,217";a="1174124506" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by orsmga001.jf.intel.com with ESMTP; 20 Sep 2017 05:46:02 -0700 Received: from fmsmsx153.amr.corp.intel.com (10.18.125.6) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 20 Sep 2017 05:46:02 -0700 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by FMSMSX153.amr.corp.intel.com (10.18.125.6) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 20 Sep 2017 05:46:01 -0700 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.213]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.159]) with mapi id 14.03.0319.002; Wed, 20 Sep 2017 20:45:59 +0800 From: "Long, Qin" To: Laszlo Ersek , "Ye, Ting" , "Zhang, Chao B" CC: "edk2-devel@lists.01.org" Thread-Topic: [edk2] [PATCH] CryptoPkg: Add new API to retrieve commonName of X.509 certificate Thread-Index: AQHTMglNUx8vAVtuzEmMXOYg/8wBZ6K9snpQ Date: Wed, 20 Sep 2017 12:45:59 +0000 Message-ID: References: <20170919033840.3012-1-qin.long@intel.com> <179d82f6-df16-ffbf-f813-f2a2cc39f0df@redhat.com> In-Reply-To: <179d82f6-df16-ffbf-f813-f2a2cc39f0df@redhat.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 X-Content-Filtered-By: Mailman/MimeDel 2.1.22 Subject: Re: [PATCH] CryptoPkg: Add new API to retrieve commonName of X.509 certificate X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 20 Sep 2017 12:42:57 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Laszlo. It's one good feedback. This is one historical design issue. We choose to use simple BOOLEAN as the= return value, because OpenSSL has complicated return data (reason) with ex= tra api (e.g. ERR_get_error()...). It's hard to map these error messages di= rectly, then we just used one simplest way before, and always kept this kin= d of API style in late updates. I also think the return value (true/false) in current BaseCryptLib is reall= y ambiguous to tell any more useful information. RETURN_xxx is more valuabl= e in this new-added case. I would like to update the patch per your suggest= ion. Thanks for raising this. Best Regards & Thanks, LONG, Qin From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Lasz= lo Ersek Sent: Wednesday, September 20, 2017 8:09 PM To: Long, Qin ; Ye, Ting ; Zhang, Ch= ao B Cc: edk2-devel@lists.01.org Subject: Re: [edk2] [PATCH] CryptoPkg: Add new API to retrieve commonName o= f X.509 certificate Hello Qin, On 09/19/17 05:38, Long Qin wrote: > Add one new API (X509GetCommonName()) to retrieve the subject commonName > string from one X.509 certificate. > > Cc: Ting Ye > > Cc: Chao Zhang > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Qin Long > > --- > CryptoPkg/Application/Cryptest/RsaVerify2.c | 17 ++++ > CryptoPkg/Include/Library/BaseCryptLib.h | 32 ++++++++ > CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c | 93 ++++++++++++++++= ++++++ > CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c | 32 ++++++++ > .../Pk/CryptX509Null.c | 34 +++++++- > 5 files changed, 207 insertions(+), 1 deletion(-) > diff --git a/CryptoPkg/Include/Library/BaseCryptLib.h b/CryptoPkg/Include= /Library/BaseCryptLib.h > index 9c5ffcd9cf..d861be6725 100644 > --- a/CryptoPkg/Include/Library/BaseCryptLib.h > +++ b/CryptoPkg/Include/Library/BaseCryptLib.h > @@ -2171,6 +2171,38 @@ X509GetSubjectName ( > IN OUT UINTN *SubjectSize > ); > > +/** > + Retrieve the common name (CN) string from one X.509 certificate. > + > + If Cert or CommonNameSize is NULL, then return FALSE. > + If this interface is not supported, then return FALSE. > + > + @param[in] Cert Pointer to the DER-encoded X509 certif= icate. > + @param[in] CertSize Size of the X509 certificate in bytes. > + @param[out] CommonName Buffer to contain the retrieved certif= icate common > + name string. At most CommonNameSize by= tes will be > + written and the string will be null te= rminated. May be > + NULL in order to determine the size bu= ffer needed. > + @param[in,out] CommonNameSize The size in bytes of the CommonName bu= ffer on input, > + and the size of buffer returned Common= Name on output. > + if CommonName is NULL then the amount = of space needed > + in buffer (including the final null) i= s returned. > + > + @retval TRUE The certificate CommonName retrieved successfully. > + @retval FALSE Invalid certificate, or CommonNameSize is NULL, > + or no CommonName entry exists. > + @retval FALSE This interface is not supported. > + > +**/ > +BOOLEAN > +EFIAPI > +X509GetCommonName ( > + IN CONST UINT8 *Cert, > + IN UINTN CertSize, > + OUT CHAR8 *CommonName, > + IN OUT UINTN *CommonNameSize > + ); > + > /** > Verify one X509 certificate was issued by the trusted CA. > I hope my questions / suggestions aren't unwelcome (or misguided) -- have you considered returning RETURN_STATUS from this function? Currently FALSE is returned for several error cases, but we have good RETURN_xxx macros for telling them apart: - RETURN_BUFFER_TOO_SMALL: "The buffer was not large enough to hold the requested data. The required buffer size is returned in the appropriate parameter when this error occurs." - RETURN_UNSUPPORTED: "The operation is not supported." - RETURN_NOT_FOUND: "The item was not found." -- this can be used for "no CommonName entry exists". - RETURN_INVALID_PARAMETER: "The parameter was incorrect." -- this can be used for "CommonNameSize is NULL", and likely for "Invalid certificate" as well. If you don't want to update the interface, I'm OK with that of course; I just figured I'd raise the question. Thanks! Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel