Do not allow pkcs1 sig algs with TLS 1.3 protocol

[simo5]

When tls1.3 is used pkcs1 is not supported so prevent selecting
rsa_pkcs1_shaXXX singature algorithms if TLS1.3 is in use.

Move signature algorithm selection to a helper function to make it
easier to read.

• I have read the CONTRIBUTING.md document and my PR follows change requirements therein
• the changes are also reflected in documentation and code comments
• all new and existing tests pass (see Travis CI results)
• test script checklist was followed for new scripts
• new test script added to tlslite-ng.json and tlslite-ng-random-subset.json
• new and modified scripts were ran against popular TLS implementations:
  • OpenSSL
  • NSS
  • GnuTLS
• required version of tlslite-ng updated in requirements.txt and README.md

---

This change is 

[tomato42]

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @simo5)

---

tlsfuzzer/messages.py, line 873 at r1 (raw file):

        for sig in cert_req.supported_signature_algs:
            if self.private_key.key_type == "rsa-pss":
                if sig[0] == 8 and sig[1] in (9, 10, 11):

that old code was using this compare but it was a combination of trying to make the code as small as possible, to make it ft into the next() and the fact they were not defined in SignatureSheme

I wonder if this should not be combined (and reuse the same lookup lists) as the code in ExpectCertificateVerify:
https://github.com/tomato42/tlsfuzzer/blob/a723fb424311d15e13dc5aa7adde3ebf4849ec9a/tlsfuzzer/expect.py#L752-L763

[simo5]

Yeah make sense, it will make it even more readable I think, let me try to change it

changed along the lines drafted in the review

[tomato42]

Reviewed 1 of 1 files at r2.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @simo5)

a discussion (no related file):
negative test coverage? (to see that pkcs1 signatures in TLS 1.3 are really rejected)

---

---

tlsfuzzer/messages.py, line 873 at r2 (raw file):

        for sig in cert_req.supported_signature_algs:
            if self.private_key.key_type == "rsa-pss":
                if sig in (SignatureScheme.rsa_pss_pss_sha256,

nested if that could be replaced with and

---

tlsfuzzer/messages.py, line 877 at r2 (raw file):

                           SignatureScheme.rsa_pss_pss_sha512):
                    return sig
            else:

else is not needed with return in primary case

---

tlsfuzzer/messages.py, line 879 at r2 (raw file):

            else:
                assert self.private_key.key_type == "rsa"
                if sig in (SignatureScheme.rsa_pkcs1_sha1,

md5 missing

---

tlsfuzzer/messages.py, line 884 at r2 (raw file):

                           SignatureScheme.rsa_pkcs1_sha384,
                           SignatureScheme.rsa_pkcs1_sha512):
                    if self.sig_version < (3, 4):

two ifs that can be replaced by and

---

tlsfuzzer/messages.py, line 886 at r2 (raw file):

                    if self.sig_version < (3, 4):
                        return sig
                elif sig in (SignatureScheme.rsa_pss_sha256,

same here, else with return in previous case

test added and nested if/else that could be removed have been removed

[tomato42]

Reviewed 1 of 1 files at r3, 1 of 1 files at r4.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @simo5)

---

tlsfuzzer/messages.py, line 883 at r3 (raw file):

                           SignatureScheme.rsa_pss_sha512):
                    return sig
                # as a flabback check for pkcs1 only if TLS < 1.3

typo: falback

fixed spelling

[tomato42]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @simo5)

---

tlsfuzzer/messages.py, line 883 at r3 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

typo: falback

not fixed?

[simo5]

doh, somehow the git push had failed

now fixed for real

[tomato42]

Reviewed 2 of 2 files at r5, 1 of 1 files at r6.
Reviewable status: complete! all files reviewed, all discussions resolved

[tomato42]

Thanks!