Added support for RSASSA-PSS padding algorithms

[MasonCitywide]

Added algorithms to sign_algo.rsPKCS_RSA_PSS_SHA256, PKCS_RSA_PSS_SHA384, and PKCS_RSA_PSS_SHA512.

A half implemented version of PKCS_RSA_PSS_SHA256 already existed with a comment saying this doesn't work because ring hasn't implemented PSS padding (here). It seems that since then it has (here), and that comment was made before the release of aws-lc-rs.

There was also an issue in the pre-existing PKCS_RSA_PSS_SHA256 function in which the salt length was set to the default 20 instead of the recommended value of the number of octets of the hash algorithm (RFC 4055, pg. 9).

This is an important change as, if I am reading it correctly, non-PSS padding has been deprecated since RFC 8446 (pg. 70), with security concerns like ROBOT.

I was able to successfully create CSRs using all three of these algorithms using the aws-lc-rs backend. However, I'm not familiar with the unit testing of this library and I am new to contributions, so I would appreciate an independant review of these additions before they are merged.

Thank you for your time,
MC

[djc]

The changes here look okay so far, but CI doesn't pass and we'll want to make sure there some tests exercising this against a different backend (maybe OpenSSL)?

[MasonCitywide]

The first build was failing because the algorithms I implemented had the same OIDs. However, RSA-PSS algorithms are supposed to have the same OIDs and are instead differentiated by their hash algorithm OIDs (RFC 4055, pgs. 8, 9, 15). So, I changed the PartialEq trait for sign_algo::SignatureAlgorithm to, if the params field is SignatureAlgorithmParams::RsaPss, differentiate based on the hash OID.

I checked CSR generation with all three algorithms, and it worked, but only with aws_lc_rs and not ring. Since the *::signature::RSA_PSS_SHA256 path is identical for each, I assumed it was a problem with ring and just gated all RSA-PSS functionality behind #[cfg(feature = "aws_lc_rs")].

After trying for several hours, I can't get the project to build with OpenSSL. Unfortunately, I don't think I have more time to dedicate to this PR. If anyone would like to take it from here, I am quite sure the code works, I just can't write proper OpenSSL tests.

[djc]

Going to keep this open to make it easier for other interested parties to find. Thanks for your efforts!

[cpu]

There was also an issue in the pre-existing PKCS_RSA_PSS_SHA256 function in which the salt length was set to the default 20 instead of the recommended value of the number of octets of the hash algorithm (RFC 4055, pg. 9).

Just wanted to note I bumped into this in #445 where I noticed not only did we set the salt length to the wrong value, but we also misencoded it by writing the default that should have been elided for proper DER encoding. I removed the half implemented support in f8ef6ee and if anyone revives this work we should take care to not re-introduce that bug.

[MasonCitywide]

Added algorithms to sign_algo.rsPKCS_RSA_PSS_SHA256, PKCS_RSA_PSS_SHA384, and PKCS_RSA_PSS_SHA512.

A half implemented version of PKCS_RSA_PSS_SHA256 already existed with a comment saying this doesn't work because ring hasn't implemented PSS padding (here). It seems that since then it has (here), and that comment was made before the release of aws-lc-rs.

There was also an issue in the pre-existing PKCS_RSA_PSS_SHA256 function in which the salt length was set to the default 20 instead of the recommended value of the number of octets of the hash algorithm (RFC 4055, pg. 9).

This is an important change as, if I am reading it correctly, non-PSS padding has been deprecated since RFC 8446 (pg. 70), with security concerns like ROBOT.

I was able to successfully create CSRs using all three of these algorithms using the aws-lc-rs backend. However, I'm not familiar with the unit testing of this library and I am new to contributions, so I would appreciate an independant review of these additions before they are merged.

Thank you for your time,
MC

[djc]

The changes here look okay so far, but CI doesn't pass and we'll want to make sure there some tests exercising this against a different backend (maybe OpenSSL)?

[MasonCitywide]

The first build was failing because the algorithms I implemented had the same OIDs. However, RSA-PSS algorithms are supposed to have the same OIDs and are instead differentiated by their hash algorithm OIDs (RFC 4055, pgs. 8, 9, 15). So, I changed the PartialEq trait for sign_algo::SignatureAlgorithm to, if the params field is SignatureAlgorithmParams::RsaPss, differentiate based on the hash OID.

I checked CSR generation with all three algorithms, and it worked, but only with aws_lc_rs and not ring. Since the *::signature::RSA_PSS_SHA256 path is identical for each, I assumed it was a problem with ring and just gated all RSA-PSS functionality behind #[cfg(feature = "aws_lc_rs")].

After trying for several hours, I can't get the project to build with OpenSSL. Unfortunately, I don't think I have more time to dedicate to this PR. If anyone would like to take it from here, I am quite sure the code works, I just can't write proper OpenSSL tests.

[djc]

Going to keep this open to make it easier for other interested parties to find. Thanks for your efforts!

[cpu]

There was also an issue in the pre-existing PKCS_RSA_PSS_SHA256 function in which the salt length was set to the default 20 instead of the recommended value of the number of octets of the hash algorithm (RFC 4055, pg. 9).

Just wanted to note I bumped into this in #445 where I noticed not only did we set the salt length to the wrong value, but we also misencoded it by writing the default that should have been elided for proper DER encoding. I removed the half implemented support in f8ef6ee and if anyone revives this work we should take care to not re-introduce that bug.

[MasonCitywide]

Added algorithms to sign_algo.rsPKCS_RSA_PSS_SHA256, PKCS_RSA_PSS_SHA384, and PKCS_RSA_PSS_SHA512.

A half implemented version of PKCS_RSA_PSS_SHA256 already existed with a comment saying this doesn't work because ring hasn't implemented PSS padding (here). It seems that since then it has (here), and that comment was made before the release of aws-lc-rs.

There was also an issue in the pre-existing PKCS_RSA_PSS_SHA256 function in which the salt length was set to the default 20 instead of the recommended value of the number of octets of the hash algorithm (RFC 4055, pg. 9).

This is an important change as, if I am reading it correctly, non-PSS padding has been deprecated since RFC 8446 (pg. 70), with security concerns like ROBOT.

I was able to successfully create CSRs using all three of these algorithms using the aws-lc-rs backend. However, I'm not familiar with the unit testing of this library and I am new to contributions, so I would appreciate an independant review of these additions before they are merged.

Thank you for your time,
MC

[djc]

The changes here look okay so far, but CI doesn't pass and we'll want to make sure there some tests exercising this against a different backend (maybe OpenSSL)?

[MasonCitywide]

The first build was failing because the algorithms I implemented had the same OIDs. However, RSA-PSS algorithms are supposed to have the same OIDs and are instead differentiated by their hash algorithm OIDs (RFC 4055, pgs. 8, 9, 15). So, I changed the PartialEq trait for sign_algo::SignatureAlgorithm to, if the params field is SignatureAlgorithmParams::RsaPss, differentiate based on the hash OID.

I checked CSR generation with all three algorithms, and it worked, but only with aws_lc_rs and not ring. Since the *::signature::RSA_PSS_SHA256 path is identical for each, I assumed it was a problem with ring and just gated all RSA-PSS functionality behind #[cfg(feature = "aws_lc_rs")].

After trying for several hours, I can't get the project to build with OpenSSL. Unfortunately, I don't think I have more time to dedicate to this PR. If anyone would like to take it from here, I am quite sure the code works, I just can't write proper OpenSSL tests.

[djc]

Going to keep this open to make it easier for other interested parties to find. Thanks for your efforts!

[cpu]

There was also an issue in the pre-existing PKCS_RSA_PSS_SHA256 function in which the salt length was set to the default 20 instead of the recommended value of the number of octets of the hash algorithm (RFC 4055, pg. 9).

Just wanted to note I bumped into this in #445 where I noticed not only did we set the salt length to the wrong value, but we also misencoded it by writing the default that should have been elided for proper DER encoding. I removed the half implemented support in f8ef6ee and if anyone revives this work we should take care to not re-introduce that bug.

[MasonCitywide]

Added algorithms to sign_algo.rsPKCS_RSA_PSS_SHA256, PKCS_RSA_PSS_SHA384, and PKCS_RSA_PSS_SHA512.

A half implemented version of PKCS_RSA_PSS_SHA256 already existed with a comment saying this doesn't work because ring hasn't implemented PSS padding (here). It seems that since then it has (here), and that comment was made before the release of aws-lc-rs.

There was also an issue in the pre-existing PKCS_RSA_PSS_SHA256 function in which the salt length was set to the default 20 instead of the recommended value of the number of octets of the hash algorithm (RFC 4055, pg. 9).

This is an important change as, if I am reading it correctly, non-PSS padding has been deprecated since RFC 8446 (pg. 70), with security concerns like ROBOT.

I was able to successfully create CSRs using all three of these algorithms using the aws-lc-rs backend. However, I'm not familiar with the unit testing of this library and I am new to contributions, so I would appreciate an independant review of these additions before they are merged.

Thank you for your time,
MC

[djc]

The changes here look okay so far, but CI doesn't pass and we'll want to make sure there some tests exercising this against a different backend (maybe OpenSSL)?

[MasonCitywide]

The first build was failing because the algorithms I implemented had the same OIDs. However, RSA-PSS algorithms are supposed to have the same OIDs and are instead differentiated by their hash algorithm OIDs (RFC 4055, pgs. 8, 9, 15). So, I changed the PartialEq trait for sign_algo::SignatureAlgorithm to, if the params field is SignatureAlgorithmParams::RsaPss, differentiate based on the hash OID.

I checked CSR generation with all three algorithms, and it worked, but only with aws_lc_rs and not ring. Since the *::signature::RSA_PSS_SHA256 path is identical for each, I assumed it was a problem with ring and just gated all RSA-PSS functionality behind #[cfg(feature = "aws_lc_rs")].

After trying for several hours, I can't get the project to build with OpenSSL. Unfortunately, I don't think I have more time to dedicate to this PR. If anyone would like to take it from here, I am quite sure the code works, I just can't write proper OpenSSL tests.

[djc]

Going to keep this open to make it easier for other interested parties to find. Thanks for your efforts!

[cpu]

There was also an issue in the pre-existing PKCS_RSA_PSS_SHA256 function in which the salt length was set to the default 20 instead of the recommended value of the number of octets of the hash algorithm (RFC 4055, pg. 9).

Just wanted to note I bumped into this in #445 where I noticed not only did we set the salt length to the wrong value, but we also misencoded it by writing the default that should have been elided for proper DER encoding. I removed the half implemented support in f8ef6ee and if anyone revives this work we should take care to not re-introduce that bug.

[MasonCitywide]

Added algorithms to sign_algo.rsPKCS_RSA_PSS_SHA256, PKCS_RSA_PSS_SHA384, and PKCS_RSA_PSS_SHA512.

A half implemented version of PKCS_RSA_PSS_SHA256 already existed with a comment saying this doesn't work because ring hasn't implemented PSS padding (here). It seems that since then it has (here), and that comment was made before the release of aws-lc-rs.

There was also an issue in the pre-existing PKCS_RSA_PSS_SHA256 function in which the salt length was set to the default 20 instead of the recommended value of the number of octets of the hash algorithm (RFC 4055, pg. 9).

This is an important change as, if I am reading it correctly, non-PSS padding has been deprecated since RFC 8446 (pg. 70), with security concerns like ROBOT.

I was able to successfully create CSRs using all three of these algorithms using the aws-lc-rs backend. However, I'm not familiar with the unit testing of this library and I am new to contributions, so I would appreciate an independant review of these additions before they are merged.

Thank you for your time,
MC

[djc]

The changes here look okay so far, but CI doesn't pass and we'll want to make sure there some tests exercising this against a different backend (maybe OpenSSL)?

[MasonCitywide]

The first build was failing because the algorithms I implemented had the same OIDs. However, RSA-PSS algorithms are supposed to have the same OIDs and are instead differentiated by their hash algorithm OIDs (RFC 4055, pgs. 8, 9, 15). So, I changed the PartialEq trait for sign_algo::SignatureAlgorithm to, if the params field is SignatureAlgorithmParams::RsaPss, differentiate based on the hash OID.

I checked CSR generation with all three algorithms, and it worked, but only with aws_lc_rs and not ring. Since the *::signature::RSA_PSS_SHA256 path is identical for each, I assumed it was a problem with ring and just gated all RSA-PSS functionality behind #[cfg(feature = "aws_lc_rs")].

After trying for several hours, I can't get the project to build with OpenSSL. Unfortunately, I don't think I have more time to dedicate to this PR. If anyone would like to take it from here, I am quite sure the code works, I just can't write proper OpenSSL tests.

[djc]

Going to keep this open to make it easier for other interested parties to find. Thanks for your efforts!

[cpu]

There was also an issue in the pre-existing PKCS_RSA_PSS_SHA256 function in which the salt length was set to the default 20 instead of the recommended value of the number of octets of the hash algorithm (RFC 4055, pg. 9).

Just wanted to note I bumped into this in #445 where I noticed not only did we set the salt length to the wrong value, but we also misencoded it by writing the default that should have been elided for proper DER encoding. I removed the half implemented support in f8ef6ee and if anyone revives this work we should take care to not re-introduce that bug.

[MasonCitywide]

Added algorithms to sign_algo.rsPKCS_RSA_PSS_SHA256, PKCS_RSA_PSS_SHA384, and PKCS_RSA_PSS_SHA512.

A half implemented version of PKCS_RSA_PSS_SHA256 already existed with a comment saying this doesn't work because ring hasn't implemented PSS padding (here). It seems that since then it has (here), and that comment was made before the release of aws-lc-rs.

There was also an issue in the pre-existing PKCS_RSA_PSS_SHA256 function in which the salt length was set to the default 20 instead of the recommended value of the number of octets of the hash algorithm (RFC 4055, pg. 9).

This is an important change as, if I am reading it correctly, non-PSS padding has been deprecated since RFC 8446 (pg. 70), with security concerns like ROBOT.

I was able to successfully create CSRs using all three of these algorithms using the aws-lc-rs backend. However, I'm not familiar with the unit testing of this library and I am new to contributions, so I would appreciate an independant review of these additions before they are merged.

Thank you for your time,
MC

[djc]

The changes here look okay so far, but CI doesn't pass and we'll want to make sure there some tests exercising this against a different backend (maybe OpenSSL)?

[MasonCitywide]

The first build was failing because the algorithms I implemented had the same OIDs. However, RSA-PSS algorithms are supposed to have the same OIDs and are instead differentiated by their hash algorithm OIDs (RFC 4055, pgs. 8, 9, 15). So, I changed the PartialEq trait for sign_algo::SignatureAlgorithm to, if the params field is SignatureAlgorithmParams::RsaPss, differentiate based on the hash OID.

I checked CSR generation with all three algorithms, and it worked, but only with aws_lc_rs and not ring. Since the *::signature::RSA_PSS_SHA256 path is identical for each, I assumed it was a problem with ring and just gated all RSA-PSS functionality behind #[cfg(feature = "aws_lc_rs")].

After trying for several hours, I can't get the project to build with OpenSSL. Unfortunately, I don't think I have more time to dedicate to this PR. If anyone would like to take it from here, I am quite sure the code works, I just can't write proper OpenSSL tests.

[djc]

Going to keep this open to make it easier for other interested parties to find. Thanks for your efforts!

[cpu]

There was also an issue in the pre-existing PKCS_RSA_PSS_SHA256 function in which the salt length was set to the default 20 instead of the recommended value of the number of octets of the hash algorithm (RFC 4055, pg. 9).

Just wanted to note I bumped into this in #445 where I noticed not only did we set the salt length to the wrong value, but we also misencoded it by writing the default that should have been elided for proper DER encoding. I removed the half implemented support in f8ef6ee and if anyone revives this work we should take care to not re-introduce that bug.

[MasonCitywide]

Added algorithms to sign_algo.rsPKCS_RSA_PSS_SHA256, PKCS_RSA_PSS_SHA384, and PKCS_RSA_PSS_SHA512.

A half implemented version of PKCS_RSA_PSS_SHA256 already existed with a comment saying this doesn't work because ring hasn't implemented PSS padding (here). It seems that since then it has (here), and that comment was made before the release of aws-lc-rs.

There was also an issue in the pre-existing PKCS_RSA_PSS_SHA256 function in which the salt length was set to the default 20 instead of the recommended value of the number of octets of the hash algorithm (RFC 4055, pg. 9).

This is an important change as, if I am reading it correctly, non-PSS padding has been deprecated since RFC 8446 (pg. 70), with security concerns like ROBOT.

I was able to successfully create CSRs using all three of these algorithms using the aws-lc-rs backend. However, I'm not familiar with the unit testing of this library and I am new to contributions, so I would appreciate an independant review of these additions before they are merged.

Thank you for your time,
MC

[djc]

The changes here look okay so far, but CI doesn't pass and we'll want to make sure there some tests exercising this against a different backend (maybe OpenSSL)?

[MasonCitywide]

The first build was failing because the algorithms I implemented had the same OIDs. However, RSA-PSS algorithms are supposed to have the same OIDs and are instead differentiated by their hash algorithm OIDs (RFC 4055, pgs. 8, 9, 15). So, I changed the PartialEq trait for sign_algo::SignatureAlgorithm to, if the params field is SignatureAlgorithmParams::RsaPss, differentiate based on the hash OID.

I checked CSR generation with all three algorithms, and it worked, but only with aws_lc_rs and not ring. Since the *::signature::RSA_PSS_SHA256 path is identical for each, I assumed it was a problem with ring and just gated all RSA-PSS functionality behind #[cfg(feature = "aws_lc_rs")].

After trying for several hours, I can't get the project to build with OpenSSL. Unfortunately, I don't think I have more time to dedicate to this PR. If anyone would like to take it from here, I am quite sure the code works, I just can't write proper OpenSSL tests.

[djc]

Going to keep this open to make it easier for other interested parties to find. Thanks for your efforts!

[cpu]

There was also an issue in the pre-existing PKCS_RSA_PSS_SHA256 function in which the salt length was set to the default 20 instead of the recommended value of the number of octets of the hash algorithm (RFC 4055, pg. 9).

Just wanted to note I bumped into this in #445 where I noticed not only did we set the salt length to the wrong value, but we also misencoded it by writing the default that should have been elided for proper DER encoding. I removed the half implemented support in f8ef6ee and if anyone revives this work we should take care to not re-introduce that bug.

[MasonCitywide]

Added algorithms to sign_algo.rsPKCS_RSA_PSS_SHA256, PKCS_RSA_PSS_SHA384, and PKCS_RSA_PSS_SHA512.

A half implemented version of PKCS_RSA_PSS_SHA256 already existed with a comment saying this doesn't work because ring hasn't implemented PSS padding (here). It seems that since then it has (here), and that comment was made before the release of aws-lc-rs.

There was also an issue in the pre-existing PKCS_RSA_PSS_SHA256 function in which the salt length was set to the default 20 instead of the recommended value of the number of octets of the hash algorithm (RFC 4055, pg. 9).

This is an important change as, if I am reading it correctly, non-PSS padding has been deprecated since RFC 8446 (pg. 70), with security concerns like ROBOT.

I was able to successfully create CSRs using all three of these algorithms using the aws-lc-rs backend. However, I'm not familiar with the unit testing of this library and I am new to contributions, so I would appreciate an independant review of these additions before they are merged.

Thank you for your time,
MC

[djc]

The changes here look okay so far, but CI doesn't pass and we'll want to make sure there some tests exercising this against a different backend (maybe OpenSSL)?

[MasonCitywide]

The first build was failing because the algorithms I implemented had the same OIDs. However, RSA-PSS algorithms are supposed to have the same OIDs and are instead differentiated by their hash algorithm OIDs (RFC 4055, pgs. 8, 9, 15). So, I changed the PartialEq trait for sign_algo::SignatureAlgorithm to, if the params field is SignatureAlgorithmParams::RsaPss, differentiate based on the hash OID.

I checked CSR generation with all three algorithms, and it worked, but only with aws_lc_rs and not ring. Since the *::signature::RSA_PSS_SHA256 path is identical for each, I assumed it was a problem with ring and just gated all RSA-PSS functionality behind #[cfg(feature = "aws_lc_rs")].

After trying for several hours, I can't get the project to build with OpenSSL. Unfortunately, I don't think I have more time to dedicate to this PR. If anyone would like to take it from here, I am quite sure the code works, I just can't write proper OpenSSL tests.

[djc]

Going to keep this open to make it easier for other interested parties to find. Thanks for your efforts!

[cpu]

There was also an issue in the pre-existing PKCS_RSA_PSS_SHA256 function in which the salt length was set to the default 20 instead of the recommended value of the number of octets of the hash algorithm (RFC 4055, pg. 9).

Just wanted to note I bumped into this in #445 where I noticed not only did we set the salt length to the wrong value, but we also misencoded it by writing the default that should have been elided for proper DER encoding. I removed the half implemented support in f8ef6ee and if anyone revives this work we should take care to not re-introduce that bug.