public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-devel] setting TLS ciphers is broken (openssl 3?)
@ 2023-09-27  8:38 Gerd Hoffmann
  2023-09-27 17:30 ` Yao, Jiewen
  2023-09-28  9:11 ` Laszlo Ersek
  0 siblings, 2 replies; 9+ messages in thread
From: Gerd Hoffmann @ 2023-09-27  8:38 UTC (permalink / raw)
  To: devel

  Hi,

I've noticed that setting chipers for TLS stopped working in ovmf, most
likely due to the openssl 3.0 update.

Test case: try http boot from https server, set ciphers on the qemu
command line using:
    -object tls-cipher-suites,id=tls-cipher0,priority=@SYSTEM
    -fw_cfg name=etc/edk2/https/ciphers,gen_id=tls-cipher0

OvmfPkg/Library/TlsAuthConfigLib will read it from fwcfg and set
EDKII_HTTP_TLS_CIPHER_LIST_VARIABLE.

CryptoPkg/Library/TlsLib/TlsConfig.c will read the variable, map the IDs
to strings and call SSL_set_cipher_list() with the result.

Later on the tls handshake fails.  From the log:

[ ... ]
TlsDxe:TlsSetCipherList: CipherString={
  ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-ECDSA-AES128-GC
  M-SHA256:AES256-SHA:AES128-SHA:DES-CBC3-SHA:DHE-RSA-AES256-GCM-SHA384:DHE-RSA-A
  ES256-SHA:DHE-RSA-AES128-SHA:DHE-RSA-DES-CBC3-SHA
  }
[ ... ]
TlsDoHandshake SSL_HANDSHAKE_ERROR State=0x10 SSL_ERROR_SSL
TlsDoHandshake ERROR 0x308010C=L6:R8010C
TlsDoHandshake ERROR 0xA0C0103=L14:RC0103
[ ... ]

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109114): https://edk2.groups.io/g/devel/message/109114
Mute This Topic: https://groups.io/mt/101613778/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [edk2-devel] setting TLS ciphers is broken (openssl 3?)
  2023-09-27  8:38 [edk2-devel] setting TLS ciphers is broken (openssl 3?) Gerd Hoffmann
@ 2023-09-27 17:30 ` Yao, Jiewen
  2023-09-28  1:32   ` Li, Yi
  2023-09-28  9:11 ` Laszlo Ersek
  1 sibling, 1 reply; 9+ messages in thread
From: Yao, Jiewen @ 2023-09-27 17:30 UTC (permalink / raw)
  To: devel@edk2.groups.io, kraxel@redhat.com

Hi Gerd
Thanks for the reporting. 

We will look into that. Is below text full reproduce steps? Which server you are using? Which TLS version is configured?
Please provide as detail as possible, if you could.


One more thing: We are going to have 1 week National Holiday since Tomorrow.
If we cannot nail down shortly, that would be next next week.

Thank you
Yao, Jiewen



> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gerd
> Hoffmann
> Sent: Wednesday, September 27, 2023 4:39 PM
> To: devel@edk2.groups.io
> Subject: [edk2-devel] setting TLS ciphers is broken (openssl 3?)
> 
>   Hi,
> 
> I've noticed that setting chipers for TLS stopped working in ovmf, most
> likely due to the openssl 3.0 update.
> 
> Test case: try http boot from https server, set ciphers on the qemu
> command line using:
>     -object tls-cipher-suites,id=tls-cipher0,priority=@SYSTEM
>     -fw_cfg name=etc/edk2/https/ciphers,gen_id=tls-cipher0
> 
> OvmfPkg/Library/TlsAuthConfigLib will read it from fwcfg and set
> EDKII_HTTP_TLS_CIPHER_LIST_VARIABLE.
> 
> CryptoPkg/Library/TlsLib/TlsConfig.c will read the variable, map the IDs
> to strings and call SSL_set_cipher_list() with the result.
> 
> Later on the tls handshake fails.  From the log:
> 
> [ ... ]
> TlsDxe:TlsSetCipherList: CipherString={
>   ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-AES256-GCM-
> SHA384:ECDHE-ECDSA-AES128-GC
>   M-SHA256:AES256-SHA:AES128-SHA:DES-CBC3-SHA:DHE-RSA-AES256-GCM-
> SHA384:DHE-RSA-A
>   ES256-SHA:DHE-RSA-AES128-SHA:DHE-RSA-DES-CBC3-SHA
>   }
> [ ... ]
> TlsDoHandshake SSL_HANDSHAKE_ERROR State=0x10 SSL_ERROR_SSL
> TlsDoHandshake ERROR 0x308010C=L6:R8010C
> TlsDoHandshake ERROR 0xA0C0103=L14:RC0103
> [ ... ]
> 
> take care,
>   Gerd
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109128): https://edk2.groups.io/g/devel/message/109128
Mute This Topic: https://groups.io/mt/101613778/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [edk2-devel] setting TLS ciphers is broken (openssl 3?)
  2023-09-27 17:30 ` Yao, Jiewen
@ 2023-09-28  1:32   ` Li, Yi
  0 siblings, 0 replies; 9+ messages in thread
From: Li, Yi @ 2023-09-28  1:32 UTC (permalink / raw)
  To: devel@edk2.groups.io, Yao, Jiewen, kraxel@redhat.com

Hi Gerd,

We have validated HTTPs boot and WIFI with EAP-TLS, where the code consumed openssl3.0 TLS lib API.

So we cannot reproduce this issue. Could you provide detail test steps to me, I will look into it.

Thanks,
Yi  

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao, Jiewen
Sent: Thursday, September 28, 2023 1:31 AM
To: devel@edk2.groups.io; kraxel@redhat.com
Subject: Re: [edk2-devel] setting TLS ciphers is broken (openssl 3?)

Hi Gerd
Thanks for the reporting. 

We will look into that. Is below text full reproduce steps? Which server you are using? Which TLS version is configured?
Please provide as detail as possible, if you could.


One more thing: We are going to have 1 week National Holiday since Tomorrow.
If we cannot nail down shortly, that would be next next week.

Thank you
Yao, Jiewen



> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gerd 
> Hoffmann
> Sent: Wednesday, September 27, 2023 4:39 PM
> To: devel@edk2.groups.io
> Subject: [edk2-devel] setting TLS ciphers is broken (openssl 3?)
> 
>   Hi,
> 
> I've noticed that setting chipers for TLS stopped working in ovmf, 
> most likely due to the openssl 3.0 update.
> 
> Test case: try http boot from https server, set ciphers on the qemu 
> command line using:
>     -object tls-cipher-suites,id=tls-cipher0,priority=@SYSTEM
>     -fw_cfg name=etc/edk2/https/ciphers,gen_id=tls-cipher0
> 
> OvmfPkg/Library/TlsAuthConfigLib will read it from fwcfg and set 
> EDKII_HTTP_TLS_CIPHER_LIST_VARIABLE.
> 
> CryptoPkg/Library/TlsLib/TlsConfig.c will read the variable, map the 
> IDs to strings and call SSL_set_cipher_list() with the result.
> 
> Later on the tls handshake fails.  From the log:
> 
> [ ... ]
> TlsDxe:TlsSetCipherList: CipherString={
>   ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-AES256-GCM-
> SHA384:ECDHE-ECDSA-AES128-GC
>   M-SHA256:AES256-SHA:AES128-SHA:DES-CBC3-SHA:DHE-RSA-AES256-GCM-
> SHA384:DHE-RSA-A
>   ES256-SHA:DHE-RSA-AES128-SHA:DHE-RSA-DES-CBC3-SHA
>   }
> [ ... ]
> TlsDoHandshake SSL_HANDSHAKE_ERROR State=0x10 SSL_ERROR_SSL 
> TlsDoHandshake ERROR 0x308010C=L6:R8010C TlsDoHandshake ERROR 
> 0xA0C0103=L14:RC0103 [ ... ]
> 
> take care,
>   Gerd
> 
> 
> 
> 
> 








-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109131): https://edk2.groups.io/g/devel/message/109131
Mute This Topic: https://groups.io/mt/101613778/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [edk2-devel] setting TLS ciphers is broken (openssl 3?)
  2023-09-27  8:38 [edk2-devel] setting TLS ciphers is broken (openssl 3?) Gerd Hoffmann
  2023-09-27 17:30 ` Yao, Jiewen
@ 2023-09-28  9:11 ` Laszlo Ersek
  2023-09-28 14:25   ` Gerd Hoffmann
  1 sibling, 1 reply; 9+ messages in thread
From: Laszlo Ersek @ 2023-09-28  9:11 UTC (permalink / raw)
  To: Gerd Hoffmann, edk2-devel-groups-io, Li, Yi, Jiewen Yao

On 9/27/23 10:38, kraxel at redhat.com (Gerd Hoffmann) wrote:
>   Hi,
>
> I've noticed that setting chipers for TLS stopped working in ovmf,
> most likely due to the openssl 3.0 update.
>
> Test case: try http boot from https server, set ciphers on the qemu
> command line using:
>     -object tls-cipher-suites,id=tls-cipher0,priority=@SYSTEM
>     -fw_cfg name=etc/edk2/https/ciphers,gen_id=tls-cipher0
>
> OvmfPkg/Library/TlsAuthConfigLib will read it from fwcfg and set
> EDKII_HTTP_TLS_CIPHER_LIST_VARIABLE.
>
> CryptoPkg/Library/TlsLib/TlsConfig.c will read the variable, map the
> IDs to strings and call SSL_set_cipher_list() with the result.
>
> Later on the tls handshake fails.  From the log:
>
> [ ... ]
> TlsDxe:TlsSetCipherList: CipherString={
>   ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-ECDSA-AES128-GC
>   M-SHA256:AES256-SHA:AES128-SHA:DES-CBC3-SHA:DHE-RSA-AES256-GCM-SHA384:DHE-RSA-A
>   ES256-SHA:DHE-RSA-AES128-SHA:DHE-RSA-DES-CBC3-SHA
>   }
> [ ... ]
> TlsDoHandshake SSL_HANDSHAKE_ERROR State=0x10 SSL_ERROR_SSL
> TlsDoHandshake ERROR 0x308010C=L6:R8010C

Library 6 is "EVP" ("envelope"):

# define ERR_LIB_EVP             6

Reason code is 0x8010C, in binary: 10000000000100001100

The least significant 18 bits (ERR_RFLAGS_OFFSET) are the actual reason
code (000000000100001100), bits above it are the reason flags (10). The
reason flag is therefore ERR_RFLAG_COMMON -- "the reason code is common
to all libraries". Reason code (0x10c, 268) is ERR_R_UNSUPPORTED:

# define ERR_R_UNSUPPORTED                       (268|ERR_RFLAG_COMMON)

> TlsDoHandshake ERROR 0xA0C0103=L14:RC0103

Library: 0x14

# define ERR_LIB_SSL             20

Reason flags: ERR_RFLAG_FATAL (1) + ERR_RFLAG_COMMON (2).

Reason code: 0x103 -- 259

# define ERR_R_FATAL                             (ERR_RFLAG_FATAL|ERR_RFLAG_COMMON)
# define ERR_R_INTERNAL_ERROR                    (259|ERR_R_FATAL)

For a successful handshake, we need the intersection of the following
sets not to be empty:

(1) the ciphers enabled in your system-wide crypto policy (likely
DEFAULT)

(2) TlsCipherMappingTable [CryptoPkg/Library/TlsLib/TlsConfig.c]

(3) the ciphers supported by the openssl library linked into the
firmware

(4) the ciphers supported by the HTTPS server

The OpenSSL3 update may have restricted set (3), causing the grand
intersection to be empty.

Can you perhaps relax your crypto policy -- i.e., widen set (1) -- to
LEGACY with "update-crypto-policies", to see if that makes a difference?

(Or else, on the QEMU command line, use a different priority from
@SYSTEM; but I'm not sure how that works.)

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109143): https://edk2.groups.io/g/devel/message/109143
Mute This Topic: https://groups.io/mt/101613778/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [edk2-devel] setting TLS ciphers is broken (openssl 3?)
  2023-09-28  9:11 ` Laszlo Ersek
@ 2023-09-28 14:25   ` Gerd Hoffmann
  2023-09-29  7:59     ` Laszlo Ersek
  2023-09-29 10:19     ` Gerd Hoffmann
  0 siblings, 2 replies; 9+ messages in thread
From: Gerd Hoffmann @ 2023-09-28 14:25 UTC (permalink / raw)
  To: Laszlo Ersek; +Cc: edk2-devel-groups-io, Li, Yi, Jiewen Yao

  Hi,

> > Test case: try http boot from https server, set ciphers on the qemu
> > command line using:
> >     -object tls-cipher-suites,id=tls-cipher0,priority=@SYSTEM
> >     -fw_cfg name=etc/edk2/https/ciphers,gen_id=tls-cipher0

> For a successful handshake, we need the intersection of the following
> sets not to be empty:
> 
> (1) the ciphers enabled in your system-wide crypto policy (likely
> DEFAULT)
> 
> (2) TlsCipherMappingTable [CryptoPkg/Library/TlsLib/TlsConfig.c]
> 
> (3) the ciphers supported by the openssl library linked into the
> firmware
> 
> (4) the ciphers supported by the HTTPS server

(1) + (4) should be the same (default system configuration).
The https server is standard python http module with ssl module
(see below).

> The OpenSSL3 update may have restricted set (3), causing the grand
> intersection to be empty.

That seems to be the case.  Maybe (2) needs an update to enable newer
ciphers, when logging the mappings I see there are quite a few IDs which
don't get mapped:

TlsDxe:TlsSetCipherList: skipping CipherId=0x1302
TlsDxe:TlsSetCipherList: skipping CipherId=0x1303
TlsDxe:TlsSetCipherList: skipping CipherId=0x1301
TlsDxe:TlsSetCipherList: skipping CipherId=0x1304
TlsDxe:TlsSetCipherList: CipherId=0xC030 -> ECDHE-RSA-AES256-GCM-SHA384
TlsDxe:TlsSetCipherList: skipping CipherId=0xCCA8
TlsDxe:TlsSetCipherList: skipping CipherId=0xC014
TlsDxe:TlsSetCipherList: skipping CipherId=0xC02F
TlsDxe:TlsSetCipherList: skipping CipherId=0xC013
TlsDxe:TlsSetCipherList: skipping CipherId=0xC012
TlsDxe:TlsSetCipherList: CipherId=0xC02C -> ECDHE-ECDSA-AES256-GCM-SHA384
TlsDxe:TlsSetCipherList: skipping CipherId=0xC0AD
TlsDxe:TlsSetCipherList: skipping CipherId=0xCCA9
TlsDxe:TlsSetCipherList: skipping CipherId=0xC00A
TlsDxe:TlsSetCipherList: CipherId=0xC02B -> ECDHE-ECDSA-AES128-GCM-SHA256
TlsDxe:TlsSetCipherList: skipping CipherId=0xC0AC
TlsDxe:TlsSetCipherList: skipping CipherId=0xC009
TlsDxe:TlsSetCipherList: skipping CipherId=0xC008
TlsDxe:TlsSetCipherList: skipping CipherId=0x009D
TlsDxe:TlsSetCipherList: skipping CipherId=0xC09D
TlsDxe:TlsSetCipherList: CipherId=0x0035 -> AES256-SHA
TlsDxe:TlsSetCipherList: skipping CipherId=0x009C
TlsDxe:TlsSetCipherList: skipping CipherId=0xC09C
TlsDxe:TlsSetCipherList: CipherId=0x002F -> AES128-SHA
TlsDxe:TlsSetCipherList: CipherId=0x000A -> DES-CBC3-SHA
TlsDxe:TlsSetCipherList: CipherId=0x009F -> DHE-RSA-AES256-GCM-SHA384
TlsDxe:TlsSetCipherList: skipping CipherId=0xC09F
TlsDxe:TlsSetCipherList: skipping CipherId=0xCCAA
TlsDxe:TlsSetCipherList: CipherId=0x0039 -> DHE-RSA-AES256-SHA
TlsDxe:TlsSetCipherList: skipping CipherId=0x009E
TlsDxe:TlsSetCipherList: skipping CipherId=0xC09E
TlsDxe:TlsSetCipherList: CipherId=0x0033 -> DHE-RSA-AES128-SHA
TlsDxe:TlsSetCipherList: CipherId=0x0016 -> DHE-RSA-DES-CBC3-SHA
TlsDxe:TlsSetCipherList: skipping CipherId=0x00A3
TlsDxe:TlsSetCipherList: skipping CipherId=0x0038
TlsDxe:TlsSetCipherList: skipping CipherId=0x00A2
TlsDxe:TlsSetCipherList: skipping CipherId=0x0032
TlsDxe:TlsSetCipherList: skipping CipherId=0x0013

> Can you perhaps relax your crypto policy -- i.e., widen set (1) -- to
> LEGACY with "update-crypto-policies", to see if that makes a difference?
> 
> (Or else, on the QEMU command line, use a different priority from
> @SYSTEM; but I'm not sure how that works.)

https://gnutls.org/manual/html_node/Priority-Strings.html

Tried @SYSTEM, DEFAULT and LEGACY, none of them work.

Not setting ciphers works.

take care,
  Gerd

----------------------------- cut here ---------------------------
#!/usr/bin/python
import os
import ssl
import socket
import optparse

from http.server import SimpleHTTPRequestHandler
try:
    from http.server import ThreadingHTTPServer as HTTPServer
except ImportError:
    from http.server import HTTPServer

class HTTPServerV6(HTTPServer):
    address_family = socket.AF_INET6

parser = optparse.OptionParser()
parser.add_option('-d', '--directory', dest = 'directory', type = 'string', default = '.',
                  help = 'server files from DIR', metavar = 'DIR')
parser.add_option('-p', '--port', dest = 'port', type = 'int', default = 8080,
                  help = 'bind to tcp port PORT', metavar = 'PORT')
parser.add_option('-b', '--bind', dest = 'bind', type = 'string', default ='',
                  help = 'bind to tcp addr ADDR', metavar = 'ADDR')
parser.add_option('--tls', dest = 'tls', action = 'store_true', default = False,
                  help = 'enable https mode')
parser.add_option('--cert', dest = 'certfile',
                  help = 'use tls certificate file CERT', metavar = 'CERT')
parser.add_option('--key', dest = 'keyfile',
                  help = 'use tls certificate key file KEY', metavar = 'KEY')
(options, args) = parser.parse_args()

print(f'config: dir="{options.directory}", addr="{options.bind}", port={options.port}, tls={options.tls}')
if options.tls:
    print(f'config: certfile="{options.certfile}", keyfile="{options.keyfile}"')

handler = SimpleHTTPRequestHandler
handler.protocol_version = "HTTP/1.1"
server = None
if ':' in options.bind:
    server = HTTPServerV6((options.bind, options.port), handler)
else:
    server = HTTPServer((options.bind, options.port), handler)
if options.tls:
    context = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER)
    context.load_cert_chain(options.certfile, options.keyfile)
    server.socket = context.wrap_socket(server.socket,
                                        server_side = True)

os.chdir(options.directory)
server.serve_forever()



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109147): https://edk2.groups.io/g/devel/message/109147
Mute This Topic: https://groups.io/mt/101613778/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [edk2-devel] setting TLS ciphers is broken (openssl 3?)
  2023-09-28 14:25   ` Gerd Hoffmann
@ 2023-09-29  7:59     ` Laszlo Ersek
  2023-09-29  8:42       ` Gerd Hoffmann
  2023-09-29 10:19     ` Gerd Hoffmann
  1 sibling, 1 reply; 9+ messages in thread
From: Laszlo Ersek @ 2023-09-29  7:59 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: edk2-devel-groups-io, Li, Yi, Jiewen Yao

On 9/28/23 16:25, Gerd Hoffmann wrote:

>> The OpenSSL3 update may have restricted set (3), causing the grand
>> intersection to be empty.
>
> That seems to be the case.  Maybe (2) needs an update to enable newer
> ciphers, when logging the mappings I see there are quite a few IDs which
> don't get mapped:
>
> TlsDxe:TlsSetCipherList: skipping CipherId=0x1302
> TlsDxe:TlsSetCipherList: skipping CipherId=0x1303
> TlsDxe:TlsSetCipherList: skipping CipherId=0x1301
> TlsDxe:TlsSetCipherList: skipping CipherId=0x1304
> TlsDxe:TlsSetCipherList: CipherId=0xC030 -> ECDHE-RSA-AES256-GCM-SHA384
> TlsDxe:TlsSetCipherList: skipping CipherId=0xCCA8
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC014
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC02F
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC013
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC012
> TlsDxe:TlsSetCipherList: CipherId=0xC02C -> ECDHE-ECDSA-AES256-GCM-SHA384
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC0AD
> TlsDxe:TlsSetCipherList: skipping CipherId=0xCCA9
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC00A
> TlsDxe:TlsSetCipherList: CipherId=0xC02B -> ECDHE-ECDSA-AES128-GCM-SHA256
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC0AC
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC009
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC008
> TlsDxe:TlsSetCipherList: skipping CipherId=0x009D
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC09D
> TlsDxe:TlsSetCipherList: CipherId=0x0035 -> AES256-SHA
> TlsDxe:TlsSetCipherList: skipping CipherId=0x009C
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC09C
> TlsDxe:TlsSetCipherList: CipherId=0x002F -> AES128-SHA
> TlsDxe:TlsSetCipherList: CipherId=0x000A -> DES-CBC3-SHA
> TlsDxe:TlsSetCipherList: CipherId=0x009F -> DHE-RSA-AES256-GCM-SHA384
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC09F
> TlsDxe:TlsSetCipherList: skipping CipherId=0xCCAA
> TlsDxe:TlsSetCipherList: CipherId=0x0039 -> DHE-RSA-AES256-SHA
> TlsDxe:TlsSetCipherList: skipping CipherId=0x009E
> TlsDxe:TlsSetCipherList: skipping CipherId=0xC09E
> TlsDxe:TlsSetCipherList: CipherId=0x0033 -> DHE-RSA-AES128-SHA
> TlsDxe:TlsSetCipherList: CipherId=0x0016 -> DHE-RSA-DES-CBC3-SHA
> TlsDxe:TlsSetCipherList: skipping CipherId=0x00A3
> TlsDxe:TlsSetCipherList: skipping CipherId=0x0038
> TlsDxe:TlsSetCipherList: skipping CipherId=0x00A2
> TlsDxe:TlsSetCipherList: skipping CipherId=0x0032
> TlsDxe:TlsSetCipherList: skipping CipherId=0x0013

> Not setting ciphers works.

Right, so the default cipher list built into OpenSSL3 (3) is not the
problem in itself, and (1) and (4) have not changed.

I have raised before that TlsCipherMappingTable should be kept in sync
with all the ciphers that we build in from OpenSSL:

* https://bugzilla.tianocore.org/show_bug.cgi?id=915#c0
  (dated 2018-03-30):

> (3) While it is correct that an unrecognized EFI_TLS_CIPHER is rejected with
> EFI_UNSUPPORTED, the table used by TlsGetCipherString(), namely
> "TlsCipherMappingTable", is very lacking -- at commit 9c7d0d499296, it lists
> 23 cipher suites, and even those include "NULL-MD5" and "NULL-SHA".
>
> On my laptop, "openssl ciphers -V ALL" lists 110 cipher suites.
>
> The "TlsCipherMappingTable" array should include all ciphers that are
> supported by the OpenSSL release identified in
>
>   CryptoPkg/Library/OpensslLib/OpenSSL-HOWTO.txt
>
> Whenever edk2 picks up a new OpenSSL release, the cipher table should be
> actualized.

* https://bugzilla.tianocore.org/show_bug.cgi?id=929
  (reported on 2018-04-10; closed as WONTFIX by myself, approx. 5 years
  later, because nobody cared):

> According to the mailing list discussion linked in
> <https://bugzilla.tianocore.org/show_bug.cgi?id=915#c8>,
> "TlsCipherMappingTable" should never offer *more* cipher suites than
> actually supported by OpensslLib (because then the TLS client might
> negotiate a cipher suite with the server that the client ultimately
> won't be able to support). However, for best coverage of the ciphers
> that *are* available, "TlsCipherMappingTable" should reasonably
> approach the set of ciphers that we enable in "OpensslLib.inf". Please
> review the latter set and merge the according cipher suite identifiers
> into "TlsCipherMappingTable".

Also reported by Microsoft separately:

* https://bugzilla.tianocore.org/show_bug.cgi?id=2541
  (reported on 2020-02-20; in CONFIRMED state currently)

... I wonder how TlsCipherMappingTable looks in Project Mu!

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109186): https://edk2.groups.io/g/devel/message/109186
Mute This Topic: https://groups.io/mt/101613778/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [edk2-devel] setting TLS ciphers is broken (openssl 3?)
  2023-09-29  7:59     ` Laszlo Ersek
@ 2023-09-29  8:42       ` Gerd Hoffmann
  2023-09-29  8:52         ` Gerd Hoffmann
  0 siblings, 1 reply; 9+ messages in thread
From: Gerd Hoffmann @ 2023-09-29  8:42 UTC (permalink / raw)
  To: Laszlo Ersek; +Cc: edk2-devel-groups-io, Li, Yi, Jiewen Yao

  Hi,

> > According to the mailing list discussion linked in
> > <https://bugzilla.tianocore.org/show_bug.cgi?id=915#c8>,
> > "TlsCipherMappingTable" should never offer *more* cipher suites than
> > actually supported by OpensslLib (because then the TLS client might
> > negotiate a cipher suite with the server that the client ultimately
> > won't be able to support).

Hmm, maybe *that* is the problem.  edk2 has its own crypto algo provider
(CryptoPkg/Library/OpensslLib/OpensslStub/uefiprov.c) offering a limited
set of ciphers to reduce the size of OpensslLib.  This was added with
the switch to openssl-3.

> ... I wonder how TlsCipherMappingTable looks in Project Mu!

mu_basecore (release/202302 which seems to be latest but doesn't include
the openssl-3 switch) is identical to upstream edk2.

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109187): https://edk2.groups.io/g/devel/message/109187
Mute This Topic: https://groups.io/mt/101613778/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [edk2-devel] setting TLS ciphers is broken (openssl 3?)
  2023-09-29  8:42       ` Gerd Hoffmann
@ 2023-09-29  8:52         ` Gerd Hoffmann
  0 siblings, 0 replies; 9+ messages in thread
From: Gerd Hoffmann @ 2023-09-29  8:52 UTC (permalink / raw)
  To: Laszlo Ersek; +Cc: edk2-devel-groups-io, Li, Yi, Jiewen Yao

On Fri, Sep 29, 2023 at 10:42:19AM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> > > According to the mailing list discussion linked in
> > > <https://bugzilla.tianocore.org/show_bug.cgi?id=915#c8>,
> > > "TlsCipherMappingTable" should never offer *more* cipher suites than
> > > actually supported by OpensslLib (because then the TLS client might
> > > negotiate a cipher suite with the server that the client ultimately
> > > won't be able to support).
> 
> Hmm, maybe *that* is the problem.  edk2 has its own crypto algo provider
> (CryptoPkg/Library/OpensslLib/OpensslStub/uefiprov.c) offering a limited
> set of ciphers to reduce the size of OpensslLib.  This was added with
> the switch to openssl-3.

Hmm, the man-page says otherwise, ciphers not compiled in are supposed
to get ignored:

<quote>
  The control string str for SSL_CTX_set_cipher_list(),
  SSL_set_cipher_list(), SSL_CTX_set_ciphersuites() and
  SSL_set_ciphersuites() should be universally usable and not depend on
  details of the library configuration (ciphers compiled in). Thus no
  syntax checking takes place. Items that are not recognized, because the
  corresponding ciphers are not compiled in or because they are mistyped,
  are simply ignored. Failure is only flagged if no ciphers could be
  collected at all.
</quote>

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109188): https://edk2.groups.io/g/devel/message/109188
Mute This Topic: https://groups.io/mt/101613778/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [edk2-devel] setting TLS ciphers is broken (openssl 3?)
  2023-09-28 14:25   ` Gerd Hoffmann
  2023-09-29  7:59     ` Laszlo Ersek
@ 2023-09-29 10:19     ` Gerd Hoffmann
  1 sibling, 0 replies; 9+ messages in thread
From: Gerd Hoffmann @ 2023-09-29 10:19 UTC (permalink / raw)
  To: Laszlo Ersek; +Cc: edk2-devel-groups-io, Li, Yi, Jiewen Yao

  Hi,

> > (2) TlsCipherMappingTable [CryptoPkg/Library/TlsLib/TlsConfig.c]

> That seems to be the case.  Maybe (2) needs an update to enable newer
> ciphers, when logging the mappings I see there are quite a few IDs which
> don't get mapped:

Turns out we don't need TlsCipherMappingTable at all,
we can just query the openssl library instead.

take care,
  Gerd

--------------------------- cut here ---------------------------
From 47f94a52d8e7a21ec3f0ff05ac7d146beff5be3b Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Fri, 29 Sep 2023 12:03:18 +0200
Subject: [PATCH 1/2] CryptoPkg: fix tls config

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 CryptoPkg/Library/TlsLib/TlsConfig.c | 157 ++++++---------------------
 1 file changed, 32 insertions(+), 125 deletions(-)

diff --git a/CryptoPkg/Library/TlsLib/TlsConfig.c b/CryptoPkg/Library/TlsLib/TlsConfig.c
index f9333165a913..2e5210193c6b 100644
--- a/CryptoPkg/Library/TlsLib/TlsConfig.c
+++ b/CryptoPkg/Library/TlsLib/TlsConfig.c
@@ -9,65 +9,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 #include "InternalTlsLib.h"
 
-typedef struct {
-  //
-  // IANA/IETF defined Cipher Suite ID
-  //
-  UINT16         IanaCipher;
-  //
-  // OpenSSL-used Cipher Suite String
-  //
-  CONST CHAR8    *OpensslCipher;
-  //
-  // Length of OpensslCipher
-  //
-  UINTN          OpensslCipherLength;
-} TLS_CIPHER_MAPPING;
-
-//
-// Create a TLS_CIPHER_MAPPING initializer from IanaCipher and OpensslCipher so
-// that OpensslCipherLength is filled in automatically. IanaCipher must be an
-// integer constant expression, and OpensslCipher must be a string literal.
-//
-#define MAP(IanaCipher, OpensslCipher) \
-  { (IanaCipher), (OpensslCipher), sizeof (OpensslCipher) - 1 }
-
-//
-// The mapping table between IANA/IETF Cipher Suite definitions and
-// OpenSSL-used Cipher Suite name.
-//
-// Keep the table uniquely sorted by the IanaCipher field, in increasing order.
-//
-STATIC CONST TLS_CIPHER_MAPPING  TlsCipherMappingTable[] = {
-  MAP (0x0001, "NULL-MD5"),                         /// TLS_RSA_WITH_NULL_MD5
-  MAP (0x0002, "NULL-SHA"),                         /// TLS_RSA_WITH_NULL_SHA
-  MAP (0x0004, "RC4-MD5"),                          /// TLS_RSA_WITH_RC4_128_MD5
-  MAP (0x0005, "RC4-SHA"),                          /// TLS_RSA_WITH_RC4_128_SHA
-  MAP (0x000A, "DES-CBC3-SHA"),                     /// TLS_RSA_WITH_3DES_EDE_CBC_SHA, mandatory TLS 1.1
-  MAP (0x0016, "DHE-RSA-DES-CBC3-SHA"),             /// TLS_DHE_RSA_WITH_3DES_EDE_CBC_SHA
-  MAP (0x002F, "AES128-SHA"),                       /// TLS_RSA_WITH_AES_128_CBC_SHA, mandatory TLS 1.2
-  MAP (0x0030, "DH-DSS-AES128-SHA"),                /// TLS_DH_DSS_WITH_AES_128_CBC_SHA
-  MAP (0x0031, "DH-RSA-AES128-SHA"),                /// TLS_DH_RSA_WITH_AES_128_CBC_SHA
-  MAP (0x0033, "DHE-RSA-AES128-SHA"),               /// TLS_DHE_RSA_WITH_AES_128_CBC_SHA
-  MAP (0x0035, "AES256-SHA"),                       /// TLS_RSA_WITH_AES_256_CBC_SHA
-  MAP (0x0036, "DH-DSS-AES256-SHA"),                /// TLS_DH_DSS_WITH_AES_256_CBC_SHA
-  MAP (0x0037, "DH-RSA-AES256-SHA"),                /// TLS_DH_RSA_WITH_AES_256_CBC_SHA
-  MAP (0x0039, "DHE-RSA-AES256-SHA"),               /// TLS_DHE_RSA_WITH_AES_256_CBC_SHA
-  MAP (0x003B, "NULL-SHA256"),                      /// TLS_RSA_WITH_NULL_SHA256
-  MAP (0x003C, "AES128-SHA256"),                    /// TLS_RSA_WITH_AES_128_CBC_SHA256
-  MAP (0x003D, "AES256-SHA256"),                    /// TLS_RSA_WITH_AES_256_CBC_SHA256
-  MAP (0x003E, "DH-DSS-AES128-SHA256"),             /// TLS_DH_DSS_WITH_AES_128_CBC_SHA256
-  MAP (0x003F, "DH-RSA-AES128-SHA256"),             /// TLS_DH_RSA_WITH_AES_128_CBC_SHA256
-  MAP (0x0067, "DHE-RSA-AES128-SHA256"),            /// TLS_DHE_RSA_WITH_AES_128_CBC_SHA256
-  MAP (0x0068, "DH-DSS-AES256-SHA256"),             /// TLS_DH_DSS_WITH_AES_256_CBC_SHA256
-  MAP (0x0069, "DH-RSA-AES256-SHA256"),             /// TLS_DH_RSA_WITH_AES_256_CBC_SHA256
-  MAP (0x006B, "DHE-RSA-AES256-SHA256"),            /// TLS_DHE_RSA_WITH_AES_256_CBC_SHA256
-  MAP (0x009F, "DHE-RSA-AES256-GCM-SHA384"),        /// TLS_DHE_RSA_WITH_AES_256_GCM_SHA384
-  MAP (0xC02B, "ECDHE-ECDSA-AES128-GCM-SHA256"),    /// TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256
-  MAP (0xC02C, "ECDHE-ECDSA-AES256-GCM-SHA384"),    /// TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384
-  MAP (0xC030, "ECDHE-RSA-AES256-GCM-SHA384"),      /// TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384
-};
-
 typedef struct {
   //
   // TLS Algorithm
@@ -96,54 +37,6 @@ STATIC CONST TLS_ALGO_TO_NAME  TlsSignatureAlgoToName[] = {
   { TlsSignatureAlgoEcdsa,     "ECDSA" },
 };
 
-/**
-  Gets the OpenSSL cipher suite mapping for the supplied IANA TLS cipher suite.
-
-  @param[in]  CipherId    The supplied IANA TLS cipher suite ID.
-
-  @return  The corresponding OpenSSL cipher suite mapping if found,
-           NULL otherwise.
-
-**/
-STATIC
-CONST TLS_CIPHER_MAPPING *
-TlsGetCipherMapping (
-  IN     UINT16  CipherId
-  )
-{
-  INTN  Left;
-  INTN  Right;
-  INTN  Middle;
-
-  //
-  // Binary Search Cipher Mapping Table for IANA-OpenSSL Cipher Translation
-  //
-  Left  = 0;
-  Right = ARRAY_SIZE (TlsCipherMappingTable) - 1;
-
-  while (Right >= Left) {
-    Middle = (Left + Right) / 2;
-
-    if (CipherId == TlsCipherMappingTable[Middle].IanaCipher) {
-      //
-      // Translate IANA cipher suite ID to OpenSSL name.
-      //
-      return &TlsCipherMappingTable[Middle];
-    }
-
-    if (CipherId < TlsCipherMappingTable[Middle].IanaCipher) {
-      Right = Middle - 1;
-    } else {
-      Left = Middle + 1;
-    }
-  }
-
-  //
-  // No Cipher Mapping found, return NULL.
-  //
-  return NULL;
-}
-
 /**
   Set a new TLS/SSL method for a particular TLS object.
 
@@ -281,16 +174,20 @@ TlsSetCipherList (
   IN     UINTN   CipherNum
   )
 {
-  TLS_CONNECTION            *TlsConn;
-  EFI_STATUS                Status;
-  CONST TLS_CIPHER_MAPPING  **MappedCipher;
-  UINTN                     MappedCipherBytes;
-  UINTN                     MappedCipherCount;
-  UINTN                     CipherStringSize;
-  UINTN                     Index;
-  CONST TLS_CIPHER_MAPPING  *Mapping;
-  CHAR8                     *CipherString;
-  CHAR8                     *CipherStringPosition;
+  TLS_CONNECTION    *TlsConn;
+  EFI_STATUS        Status;
+  CONST SSL_CIPHER  **MappedCipher;
+  UINTN             MappedCipherBytes;
+  UINTN             MappedCipherCount;
+  UINTN             CipherStringSize;
+  UINTN             Index, StackIdx;
+  CHAR8             *CipherString;
+  CHAR8             *CipherStringPosition;
+
+  STACK_OF (SSL_CIPHER)      *OpensslCipherStack;
+  CONST SSL_CIPHER  *OpensslCipher;
+  CONST CHAR8       *OpensslCipherName;
+  UINTN             OpensslCipherNameSize;
 
   TlsConn = (TLS_CONNECTION *)Tls;
   if ((TlsConn == NULL) || (TlsConn->Ssl == NULL) || (CipherId == NULL)) {
@@ -315,6 +212,8 @@ TlsSetCipherList (
     return EFI_OUT_OF_RESOURCES;
   }
 
+  OpensslCipherStack = SSL_get_ciphers (TlsConn->Ssl);
+
   //
   // Map the cipher IDs, and count the number of bytes for the full
   // CipherString.
@@ -325,8 +224,14 @@ TlsSetCipherList (
     //
     // Look up the IANA-to-OpenSSL mapping.
     //
-    Mapping = TlsGetCipherMapping (CipherId[Index]);
-    if (Mapping == NULL) {
+    for (StackIdx = 0; StackIdx < sk_SSL_CIPHER_num (OpensslCipherStack); StackIdx++) {
+      OpensslCipher = sk_SSL_CIPHER_value (OpensslCipherStack, StackIdx);
+      if (CipherId[Index] == SSL_CIPHER_get_protocol_id (OpensslCipher)) {
+        break;
+      }
+    }
+
+    if (StackIdx == sk_SSL_CIPHER_num (OpensslCipherStack)) {
       DEBUG ((
         DEBUG_VERBOSE,
         "%a:%a: skipping CipherId=0x%04x\n",
@@ -357,7 +262,7 @@ TlsSetCipherList (
 
     Status = SafeUintnAdd (
                CipherStringSize,
-               Mapping->OpensslCipherLength,
+               AsciiStrnLenS (SSL_CIPHER_get_name (OpensslCipher), MAX_STRING_SIZE),
                &CipherStringSize
                );
     if (EFI_ERROR (Status)) {
@@ -368,7 +273,7 @@ TlsSetCipherList (
     //
     // Record the mapping.
     //
-    MappedCipher[MappedCipherCount++] = Mapping;
+    MappedCipher[MappedCipherCount++] = OpensslCipher;
   }
 
   //
@@ -403,7 +308,9 @@ TlsSetCipherList (
   //
   CipherStringPosition = CipherString;
   for (Index = 0; Index < MappedCipherCount; Index++) {
-    Mapping = MappedCipher[Index];
+    OpensslCipher         = MappedCipher[Index];
+    OpensslCipherName     = SSL_CIPHER_get_name (OpensslCipher);
+    OpensslCipherNameSize = AsciiStrnLenS (OpensslCipherName, MAX_STRING_SIZE);
     //
     // Append the colon (":") prefix except for the first mapping, then append
     // Mapping->OpensslCipher.
@@ -414,10 +321,10 @@ TlsSetCipherList (
 
     CopyMem (
       CipherStringPosition,
-      Mapping->OpensslCipher,
-      Mapping->OpensslCipherLength
+      OpensslCipherName,
+      OpensslCipherNameSize
       );
-    CipherStringPosition += Mapping->OpensslCipherLength;
+    CipherStringPosition += OpensslCipherNameSize;
   }
 
   //
-- 
2.41.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109191): https://edk2.groups.io/g/devel/message/109191
Mute This Topic: https://groups.io/mt/101613778/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-09-29 10:19 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-27  8:38 [edk2-devel] setting TLS ciphers is broken (openssl 3?) Gerd Hoffmann
2023-09-27 17:30 ` Yao, Jiewen
2023-09-28  1:32   ` Li, Yi
2023-09-28  9:11 ` Laszlo Ersek
2023-09-28 14:25   ` Gerd Hoffmann
2023-09-29  7:59     ` Laszlo Ersek
2023-09-29  8:42       ` Gerd Hoffmann
2023-09-29  8:52         ` Gerd Hoffmann
2023-09-29 10:19     ` Gerd Hoffmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox