Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid handshake failure with small RSA keys #394

Merged
merged 5 commits into from
Aug 29, 2019
Merged

Avoid handshake failure with small RSA keys #394

merged 5 commits into from
Aug 29, 2019

Conversation

ocheron
Copy link
Contributor

@ocheron ocheron commented Aug 22, 2019

1024-bit is a small RSA size but this is often the default value when generating with some tools.
As reported in #365, enabling RSA-PSS signatures has side effect and prevents to use keys which were accepted before.

This PR extends selection of hash algorithm to avoid the fatal failure with SHA-512, and pick SHA-384 (or lower) instead.

The data type DigitalSignatureAlg is removed, instead signatureCompatible and calling functions use PubKey instead. When deciding about signature schemes, the full content of the public key is available, so the size of the modulus can be checked.

PubKey allows more key types than DigitalSignatureAlg, so to prevent an invalid input to a function like signatureParams, the existing decoding to DigitalSignatureAlg that previously rejected bad inputs is not removed but adapted:

  • findDigitalSignatureAlg becomes isDigitalSignaturePair
  • fromPubKey becomes isDigitalSignatureKey/checkDigitalSignatureKey

Only exception to this rule is when transforming getLocalDigitalSignatureAlg to getLocalPublicKey. The code path ensures that the key pair stored in hksLocalPublicPrivateKeys is a combination already validated by isDigitalSignaturePair, so there is no need to verify again.

The consistency check between remote public-key type and selected signature scheme that was missing for TLS13 is added to checkCertVerify.

When using the local key pair there is no need to verify if it has a
supported key type.  This has already been verified when storing with
storePrivInfo.
Similar functions are now placed in the same module.
Now that RSASSA-PSS is enabled, and used by default with SHA-512, we
require an RSA key with 1034 bits minimum.  This may create failures
for people still using a 1024-bit key.

This commit adds the key-length criteria when selecting the hash
algorithm so that the selection is compatible with the actual key
length available.  It deals with both RSASSA-PKCS1 and RSASSA-PSS.
The (EC)DSA signature algorithms internally truncate the digest and
need no specific condition.
@kazu-yamamoto kazu-yamamoto self-requested a review August 29, 2019 03:12
Copy link
Collaborator

@kazu-yamamoto kazu-yamamoto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

kazu-yamamoto added a commit to kazu-yamamoto/hs-tls that referenced this pull request Aug 29, 2019
@kazu-yamamoto kazu-yamamoto merged commit 56485a1 into haskell-tls:master Aug 29, 2019
@kazu-yamamoto
Copy link
Collaborator

Merged.

@ocheron ocheron deleted the small-rsa-keys branch September 3, 2019 18:45
@ocheron
Copy link
Contributor Author

ocheron commented Sep 3, 2019

Thank you for your review. This required a bit more changes that I'd have hoped. But having the full PubKey is useful for ECDSA and pss_pss too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants