RE: [RFC crypto v3 8/9] chtls: Register the ULP
I thought about this and approach below can avoid new ulp type:
1. Register Inline TLS driver to net TLS
2. enable ethtool -K tls-hw-record-offload on
3. Issue " setsockopt(fd, SOL_TCP, TCP_ULP, "tls", sizeof("tls")) " after Bind,
this will enable user fetch net_device corresponding to ipaadr bound to
interface, if dev found is the one registered and record-offload enabled,
program the sk->sk_prot as required.
4. fallback to SW TLS for any other case, bind to inaddr_any falls in this
category and need proper handling?
tls-hw-record-offload is TLS record offload to HW, which does tx/rx and record
creation Inline.
enum {
TLS_BASE_TX,
TLS_SW_TX,
TLS_RECORD_HW, /* TLS record processed Inline */
TLS_NUM_CONFIG,
};
-Original Message-
From: Dave Watson [mailto:davejwat...@fb.com]
Sent: Wednesday, January 31, 2018 10:14 PM
To: Atul Gupta
Cc: s...@queasysnail.net; herb...@gondor.apana.org.au;
linux-crypto@vger.kernel.org; ganes...@chelsio.co; net...@vger.kernel.org;
da...@davemloft.net; Boris Pismenny ; Ilya Lesokhin
Subject: Re: [RFC crypto v3 8/9] chtls: Register the ULP
On 01/31/18 04:14 PM, Atul Gupta wrote:
>
>
> On Tuesday 30 January 2018 10:41 PM, Dave Watson wrote:
> > On 01/30/18 06:51 AM, Atul Gupta wrote:
> >
> > > What I was referring is that passing "tls" ulp type in setsockopt
> > > may be insufficient to make the decision when multi HW assist
> > > Inline TLS solution exists.
> > Setting the ULP doesn't choose HW or SW implementation, I think that
> > should be done later when setting up crypto with
> >
> > setsockopt(SOL_TLS, TLS_TX, struct crypto_info).
> setsockpot [mentioned above] is quite late for driver to enable HW
> implementation, we require something as early as tls_init
> [setsockopt(sock, SOL_TCP, TCP_ULP, "tls", sizeof("tls"))], for driver
> to set HW prot and offload connection beside Inline Tx/Rx.
> >
> > Any reason we can't use ethtool to choose HW vs SW implementation,
> > if available on the device?
> Thought about it, the interface index is not available to fetch
> netdev and caps check to set HW prot eg. bind [prot.hash] --> tls_hash to
> program HW.
Perhaps this is the part I don't follow - why do you need to override hash and
check for LISTEN? I briefly looked through the patch named "CPL handler
definition", this looks like it is a full TCP offload?
Yes, this is connection and record layer offload, and the reason I used
different ulp type, need to see what additional info or check can help setup
the required sk prot.
RE: [RFC crypto v3 8/9] chtls: Register the ULP
> -Original Message-
> From: linux-crypto-ow...@vger.kernel.org [mailto:linux-crypto-
> ow...@vger.kernel.org] On Behalf Of Atul Gupta
> Sent: Thursday, February 8, 2018 3:56 PM
> To: Dave Watson
> Cc: s...@queasysnail.net; herb...@gondor.apana.org.au; linux-
> cry...@vger.kernel.org; ganes...@chelsio.co; net...@vger.kernel.org;
> da...@davemloft.net; Boris Pismenny ; Ilya
> Lesokhin
> Subject: RE: [RFC crypto v3 8/9] chtls: Register the ULP
>
> I thought about this and approach below can avoid new ulp type:
>
> 1. Register Inline TLS driver to net TLS
> 2. enable ethtool -K tls-hw-record-offload on
> 3. Issue " setsockopt(fd, SOL_TCP, TCP_ULP, "tls", sizeof("tls")) " after
> Bind,
> this will enable user fetch net_device corresponding to ipaadr bound to
> interface, if dev found is the one registered and record-offload enabled,
> program the sk->sk_prot as required.
What happens in case of TLS clients which do not explicitly call bind() and
rely on kernel to choose an ephemeral port for socket?
Does calling setsockopt after the connection is established fix the problem?
> 4. fallback to SW TLS for any other case, bind to inaddr_any falls in this
> category and need proper handling?
>
> tls-hw-record-offload is TLS record offload to HW, which does tx/rx and
> record creation Inline.
>
> enum {
> TLS_BASE_TX,
> TLS_SW_TX,
> TLS_RECORD_HW, /* TLS record processed Inline */
> TLS_NUM_CONFIG,
> };
>
> -Original Message-
> From: Dave Watson [mailto:davejwat...@fb.com]
> Sent: Wednesday, January 31, 2018 10:14 PM
> To: Atul Gupta
> Cc: s...@queasysnail.net; herb...@gondor.apana.org.au; linux-
> cry...@vger.kernel.org; ganes...@chelsio.co; net...@vger.kernel.org;
> da...@davemloft.net; Boris Pismenny ; Ilya
> Lesokhin
> Subject: Re: [RFC crypto v3 8/9] chtls: Register the ULP
>
> On 01/31/18 04:14 PM, Atul Gupta wrote:
> >
> >
> > On Tuesday 30 January 2018 10:41 PM, Dave Watson wrote:
> > > On 01/30/18 06:51 AM, Atul Gupta wrote:
> > >
> > > > What I was referring is that passing "tls" ulp type in setsockopt
> > > > may be insufficient to make the decision when multi HW assist
> > > > Inline TLS solution exists.
> > > Setting the ULP doesn't choose HW or SW implementation, I think that
> > > should be done later when setting up crypto with
> > >
> > > setsockopt(SOL_TLS, TLS_TX, struct crypto_info).
> > setsockpot [mentioned above] is quite late for driver to enable HW
> > implementation, we require something as early as tls_init
> > [setsockopt(sock, SOL_TCP, TCP_ULP, "tls", sizeof("tls"))], for driver
> > to set HW prot and offload connection beside Inline Tx/Rx.
> > >
> > > Any reason we can't use ethtool to choose HW vs SW implementation,
> > > if available on the device?
> > Thought about it, the interface index is not available to fetch
> > netdev and caps check to set HW prot eg. bind [prot.hash] --> tls_hash to
> program HW.
>
> Perhaps this is the part I don't follow - why do you need to override hash and
> check for LISTEN? I briefly looked through the patch named "CPL handler
> definition", this looks like it is a full TCP offload?
>
> Yes, this is connection and record layer offload, and the reason I used
> different ulp type, need to see what additional info or check can help setup
> the required sk prot.
Re: [PATCH 0/9] PKCS#7 / X.509 fixes and cleanups
I presume you don't have this in a git tree somewhere that I can pull? David
Re: [PATCH 4/9] X.509: fix BUG_ON() when hash algorithm is unsupported
Eric Biggers wrote: > The X.509 parser mishandles the case where the certificate's signature's > hash algorithm is not available in the crypto API. In this case, > x509_get_sig_params() doesn't allocate the cert->sig->digest buffer; this > part seems to be intentional. Well, yes, that would be intentional: we can't digest the digestibles without access to a hash algorithm to do so and we can't allocate a digest buffer without knowing how big it should be. > Fix this by making public_key_verify_signature() return -ENOPKG if the > hash buffer has not been allocated. Hmmm... I'm not sure that this is the right place to do this, since it obscures a potential invalid argument within the kernel. I'm more inclined that the users of X.509 certs should check x509->unsupported_sig (pkcs7_verify_sig_chain() does this already partially). David
Re: [PATCH 6/9] PKCS#7: remove unnecessary check for NULL sinfo->sig->hash_algo
Eric Biggers wrote: > The PKCS#7 parser is guaranteed to set ->sig->hash_algo for every > SignerInfo, since pkcs7_sig_note_digest_algo() is a mandatory action in > the PKCS#7 ASN.1 grammar, and it returns an error code if an > unrecognized DigestAlgorithmIdentifier is given rather than leaving the > algorithm as NULL. Therefore, remove the unnecessary NULL check. Actually, we might be better off deferring ENOPKG generation as we might have multiple signatures, at least one of which does have a digest algorithm that we can handle. David
Re: [PATCH 8/9] X.509: remove dead code that set ->unsupported_sig
Eric Biggers wrote: > The X.509 parser is guaranteed to set cert->sig->pkey_algo and > cert->sig->hash_algo, since x509_note_pkey_algo() is a mandatory action > in the X.509 ASN.1 grammar, and it returns an error code if an > unrecognized AlgorithmIdentifier is given rather than leaving the > algorithms as NULL. I'm leaning towards ENOPKG production here being deferred so that X.509 certs that we can't verify can still be built into the kernel or loaded from 'trusted' sources. Let me think about this a bit more. David
Re: [PATCH 0/5] crypto: Speck support
On Wed, Feb 07, 2018 at 08:47:05PM -0500, Jeffrey Walton wrote: > On Wed, Feb 7, 2018 at 7:09 PM, Eric Biggers wrote: > > Hello, > > > > This series adds Speck support to the crypto API, including the Speck128 > > and Speck64 variants. Speck is a lightweight block cipher that can be > > much faster than AES on processors that don't have AES instructions. > > > > We are planning to offer Speck-XTS (probably Speck128/256-XTS) as an > > option for dm-crypt and fscrypt on Android, for low-end mobile devices > > with older CPUs such as ARMv7 which don't have the Cryptography > > Extensions. Currently, such devices are unencrypted because AES is not > > fast enough, even when the NEON bit-sliced implementation of AES is > > used. Other AES alternatives such as Blowfish, Twofish, Camellia, > > Cast6, and Serpent aren't fast enough either; it seems that only a > > modern ARX cipher can provide sufficient performance on these devices. > > > > This is a replacement for our original proposal > > (https://patchwork.kernel.org/patch/10101451/) which was to offer > > ChaCha20 for these devices. However, the use of a stream cipher for > > disk/file encryption with no space to store nonces would have been much > > more insecure than we thought initially, given that it would be used on > > top of flash storage as well as potentially on top of F2FS, neither of > > which is guaranteed to overwrite data in-place. > > > > Speck has been somewhat controversial due to its origin. Nevertheless, > > it has a straightforward design (it's an ARX cipher), and it appears to > > be the leading software-optimized lightweight block cipher currently, > > with the most cryptanalysis. It's also easy to implement without side > > channels, unlike AES. Moreover, we only intend Speck to be used when > > the status quo is no encryption, due to AES not being fast enough. > > > > We've also considered a novel length-preserving encryption mode based on > > ChaCha20 and Poly1305. While theoretically attractive, such a mode > > would be a brand new crypto construction and would be more complicated > > and difficult to implement efficiently in comparison to Speck-XTS. > > > > Thus, patch 1 adds a generic implementation of Speck, and the following > > patches add a 32-bit ARM NEON implementation of Speck-XTS. The > > NEON-accelerated implementation is much faster than the generic > > implementation and therefore is the implementation that would primarily > > be used in practice on the devices we are targeting. > > > > There is no AArch64 implementation added, since such CPUs are likely to > > have the Cryptography Extensions, allowing the use of AES. > > +1 on SPECK. > > Its a nice cipher that runs fast. It is nice because the security > engineering and parameter selection is well specified, and you can > push the margins as low as you like. It does not guess at security > parameters like some of the other ciphers used in dm-crypt. > > On a modern Core-i5 6th gen I've seen numbers as low as ... > SPECK-64/128 runs around 2.1 cpb, and SPECK-128/256 runs around 2.4 > cpb. > > I've already done some work for a US contractor who wanted/needed > SPECK for a possible NASA contract. NASA is looking at SPECK for some > satellite comms. > Hi Jeffrey, I see you wrote the SPECK implementation in Crypto++, and you are treating the words as big endian. Do you have a reference for this being the "correct" order? Unfortunately the authors of the cipher failed to mention the byte order in their paper. And they gave the test vectors as words, so the test vectors don't clarify it either. I had assumed little endian words, but now I am having second thoughts... And to confuse things further, it seems that some implementations (including the authors own implementation for the SUPERCOP benchmark toolkit [1]) even consider the words themselves in the order (y, x) rather than the more intuitive (x, y). [1] https://github.com/iadgov/simon-speck-supercop/blob/master/crypto_stream/speck128128ctr/ref/stream.c In fact, even the reference code from the paper treats pt[0] as y and pt[1] as x, where 'pt' is a u64 array -- although that being said, it's not shown how the actual bytes should be translated to/from those u64 arrays. I'd really like to avoid people having to add additional versions of SPECK later for the different byte and word orders... - Eric
