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.

Sign up for GitHub

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

Jump to bottom

add support for X509_get_signature_info #1504

Merged
merged 5 commits into from
Apr 10, 2024
Merged

add support for X509_get_signature_info #1504

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bac806d
add support for X509_get_signature_info
samuel40791765 Mar 21, 2024
d56c5bd
add checks and switch to x509v3_cache_extensions
samuel40791765 Mar 28, 2024
a093751
drop failures when invalid extension exist
samuel40791765 Mar 28, 2024
3b7e3fc
PR comments; documentation and checks
samuel40791765 Apr 9, 2024
2cb2672
fix postgres CI; patch RSA-PSS ssl usage
samuel40791765 Apr 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions crypto/err/x509.errordata
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ X509,137,SIGNATURE_ALGORITHM_MISMATCH
X509,128,UNKNOWN_KEY_TYPE
X509,129,UNKNOWN_NID
X509,130,UNKNOWN_PURPOSE_ID
X509,145,UNKNOWN_SIGID_ALGS
X509,131,UNKNOWN_TRUST_ID
X509,132,UNSUPPORTED_ALGORITHM
X509,133,WRONG_LOOKUP_TYPE
Expand Down
20 changes: 17 additions & 3 deletions crypto/x509/internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,10 +139,21 @@ typedef struct {
// an |X509_NAME|.
DECLARE_ASN1_FUNCTIONS(X509_CINF)

struct x509_sig_info_st {
// NID of message digest.
int digest_nid;
// NID of public key algorithm.
int pubkey_nid;
// Security bits.
int sec_bits;
WillChilds-Klein marked this conversation as resolved.
Show resolved Hide resolved
uint32_t flags;
} /* X509_SIG_INFO */;

struct x509_st {
X509_CINF *cert_info;
X509_ALGOR *sig_alg;
ASN1_BIT_STRING *signature;
X509_SIG_INFO sig_info;
CRYPTO_refcount_t references;
CRYPTO_EX_DATA ex_data;
// These contain copies of various extension values
Expand Down Expand Up @@ -341,9 +352,9 @@ struct x509_store_ctx_st {
X509_STORE_CTX_cleanup_fn cleanup;

// The following is built up
int valid; // if 0, rebuild chain
int last_untrusted; // index of last untrusted cert
STACK_OF(X509) *chain; // chain of X509s - built up and trusted
int valid; // if 0, rebuild chain
int last_untrusted; // index of last untrusted cert
STACK_OF(X509) *chain; // chain of X509s - built up and trusted

// When something goes wrong, this is why
int error_depth;
Expand Down Expand Up @@ -409,6 +420,9 @@ int x509_digest_sign_algorithm(EVP_MD_CTX *ctx, X509_ALGOR *algor);
int x509_digest_verify_init(EVP_MD_CTX *ctx, const X509_ALGOR *sigalg,
EVP_PKEY *pkey);

// x509_init_signature_info initializes the signature info for |x509|.
int x509_init_signature_info(X509 *x509);


// Path-building functions.

Expand Down
71 changes: 71 additions & 0 deletions crypto/x509/x509_set.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,9 @@
#include <openssl/obj.h>
#include <openssl/x509.h>

#include "../x509v3/internal.h"
#include "internal.h"
#include "openssl/x509v3.h"


long X509_get_version(const X509 *x509) {
Expand Down Expand Up @@ -238,3 +240,72 @@ const X509_ALGOR *X509_get0_tbs_sigalg(const X509 *x) {
X509_PUBKEY *X509_get_X509_PUBKEY(const X509 *x509) {
return x509->cert_info->key;
}

static int X509_SIG_INFO_get(const X509_SIG_INFO *sig_info, int *digest_nid,
int *pubkey_nid, int *sec_bits, uint32_t *flags) {
if (digest_nid != NULL) {
samuel40791765 marked this conversation as resolved.
Show resolved Hide resolved
*digest_nid = sig_info->digest_nid;
}
if (pubkey_nid != NULL) {
*pubkey_nid = sig_info->pubkey_nid;
}
if (sec_bits != NULL) {
*sec_bits = sig_info->sec_bits;
}
if (flags != NULL) {
*flags = sig_info->flags;
}
return (sig_info->flags & X509_SIG_INFO_VALID) != 0;
}

int X509_get_signature_info(X509 *x509, int *digest_nid, int *pubkey_nid,
samuel40791765 marked this conversation as resolved.
Show resolved Hide resolved
int *sec_bits, uint32_t *flags) {
if (x509 == NULL) {
OPENSSL_PUT_ERROR(X509, ERR_R_PASSED_NULL_PARAMETER);
return 0;
}

// The return value of |x509v3_cache_extensions| is not checked because
// |X509_get_signature_info|'s function contract does not encapsulate failures
// if any invalid extensions do exist.
x509v3_cache_extensions(x509);
return X509_SIG_INFO_get(&x509->sig_info, digest_nid, pubkey_nid, sec_bits,
flags);
}

int x509_init_signature_info(X509 *x509) {
Copy link
Member

Choose a reason for hiding this comment

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

Don't recall if we were only adding the check on external functions first, but if we are also adding to same checks to internal functions then should this validated x509 != NULL?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This did come up a long while back, I think the final consensus was to cover external functions. The justification being that there would be NULL checks in the external functions calling these internal functions, so the extra check would be overkill and add additional bloat.

int pubkey_nid, digest_nid;
const EVP_MD *md;

x509->sig_info.digest_nid = NID_undef;
x509->sig_info.pubkey_nid = NID_undef;
x509->sig_info.sec_bits = -1;
x509->sig_info.flags = 0;
if (!OBJ_find_sigid_algs(OBJ_obj2nid(x509->sig_alg->algorithm), &digest_nid,
&pubkey_nid) ||
pubkey_nid == NID_undef) {
OPENSSL_PUT_ERROR(X509, X509_R_UNKNOWN_SIGID_ALGS);
return 0;
}
x509->sig_info.pubkey_nid = pubkey_nid;
x509->sig_info.digest_nid = digest_nid;
x509->sig_info.flags |= X509_SIG_INFO_VALID;

md = EVP_get_digestbynid(digest_nid);
if (md == NULL) {
// Some valid signature algorithms have an undefined digest. See
// crypto/obj/obj_xref.c.
return 1;
}
// Security bits: half number of bits in digest.
x509->sig_info.sec_bits = (int)EVP_MD_size(md) * 4;

switch (digest_nid) {
case NID_sha1:
case NID_sha256:
case NID_sha384:
case NID_sha512:
x509->sig_info.flags |= X509_SIG_INFO_TLS;
}
return 1;
}
31 changes: 31 additions & 0 deletions crypto/x509/x509_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7049,3 +7049,34 @@ TEST(X509Test, GetTextByOBJ) {
}
}
}

TEST(X509Test, GetSigInfo) {
bssl::UniquePtr<X509> cert(CertFromPEM(kLeafPEM));
ASSERT_TRUE(cert);

int digest_nid, pubkey_nid, sec_bits;
uint32_t flags;
EXPECT_TRUE(X509_get_signature_info(cert.get(), &digest_nid, &pubkey_nid,
&sec_bits, &flags));

EXPECT_EQ(digest_nid, NID_sha256);
EXPECT_EQ(pubkey_nid, NID_rsaEncryption);
EXPECT_EQ(sec_bits, (int)EVP_MD_size(EVP_sha256()) * 4);
EXPECT_TRUE(flags & (X509_SIG_INFO_VALID | X509_SIG_INFO_TLS));

cert = CertFromPEM(kEd25519Cert);
EXPECT_TRUE(X509_get_signature_info(cert.get(), &digest_nid, &pubkey_nid,
&sec_bits, &flags));
EXPECT_EQ(digest_nid, NID_undef);
EXPECT_EQ(pubkey_nid, NID_ED25519);
EXPECT_EQ(sec_bits, -1);
EXPECT_TRUE(flags & X509_SIG_INFO_VALID);

cert = CertFromPEM(kExampleRsassaPssCert);
EXPECT_TRUE(X509_get_signature_info(cert.get(), &digest_nid, &pubkey_nid,
&sec_bits, &flags));
EXPECT_EQ(digest_nid, NID_undef);
EXPECT_EQ(pubkey_nid, NID_rsaEncryption);
EXPECT_EQ(sec_bits, -1);
EXPECT_TRUE(flags & X509_SIG_INFO_VALID);
}
7 changes: 6 additions & 1 deletion crypto/x509v3/v3_purp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,9 @@ static int xp_cmp(const X509_PURPOSE *const *a, const X509_PURPOSE *const *b) {

// As much as I'd like to make X509_check_purpose use a "const" X509* I
// really can't because it does recalculate hashes and do other non-const
// things.
// things. If |id| is -1 it just calls |x509v3_cache_extensions| for its
// side-effect.
// Returns 1 on success, 0 if x does not allow purpose, -1 on (internal) error.
int X509_check_purpose(X509 *x, int id, int ca) {
int idx;
const X509_PURPOSE *pt;
Expand Down Expand Up @@ -565,6 +567,9 @@ int x509v3_cache_extensions(X509 *x) {
break;
}
}
if (!x509_init_signature_info(x)) {
x->ex_flags |= EXFLAG_INVALID;
}
x->ex_flags |= EXFLAG_SET;

CRYPTO_MUTEX_unlock_write(&x->lock);
Expand Down
130 changes: 66 additions & 64 deletions generated-src/err_data.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,9 +233,9 @@ const uint32_t kOpenSSLReasonValues[] = {
0x2c403998,
0x2c4093a9,
0x2c4139a9,
0x2c41b9bc,
0x2c41b9cf,
0x2c42136f,
0x2c42b9cd,
0x2c42b9e0,
0x2c43076d,
0x2c43b8b3,
0x2c4437fb,
Expand All @@ -247,6 +247,7 @@ const uint32_t kOpenSSLReasonValues[] = {
0x2c4738ea,
0x2c47b923,
0x2c48380d,
0x2c48b9bc,
0x30320000,
0x30328015,
0x3033001f,
Expand Down Expand Up @@ -705,71 +706,71 @@ const uint32_t kOpenSSLReasonValues[] = {
0x4c4197bc,
0x4c421925,
0x4c429704,
0x503239df,
0x5032b9ee,
0x503339f9,
0x5033ba09,
0x50343a22,
0x5034ba3c,
0x50353a4a,
0x5035ba60,
0x50363a72,
0x5036ba88,
0x50373aa1,
0x5037bab4,
0x50383acc,
0x5038badd,
0x50393af2,
0x5039bb06,
0x503a3b26,
0x503abb3c,
0x503b3b54,
0x503bbb66,
0x503c3b82,
0x503cbb99,
0x503d3bb2,
0x503dbbc8,
0x503e3bd5,
0x503ebbeb,
0x503f3bfd,
0x503239f2,
0x5032ba01,
0x50333a0c,
0x5033ba1c,
0x50343a35,
0x5034ba4f,
0x50353a5d,
0x5035ba73,
0x50363a85,
0x5036ba9b,
0x50373ab4,
0x5037bac7,
0x50383adf,
0x5038baf0,
0x50393b05,
0x5039bb19,
0x503a3b39,
0x503abb4f,
0x503b3b67,
0x503bbb79,
0x503c3b95,
0x503cbbac,
0x503d3bc5,
0x503dbbdb,
0x503e3be8,
0x503ebbfe,
0x503f3c10,
0x503f83b3,
0x50403c10,
0x5040bc20,
0x50413c3a,
0x5041bc49,
0x50423c63,
0x5042bc80,
0x50433c90,
0x5043bca0,
0x50443cbd,
0x50403c23,
0x5040bc33,
0x50413c4d,
0x5041bc5c,
0x50423c76,
0x5042bc93,
0x50433ca3,
0x5043bcb3,
0x50443cd0,
0x50448469,
0x50453cd1,
0x5045bcef,
0x50463d02,
0x5046bd18,
0x50473d2a,
0x5047bd3f,
0x50483d65,
0x5048bd73,
0x50493d86,
0x5049bd9b,
0x504a3db1,
0x504abdc1,
0x504b3de1,
0x504bbdf4,
0x504c3e17,
0x504cbe45,
0x504d3e72,
0x504dbe8f,
0x504e3eaa,
0x504ebec6,
0x504f3ed8,
0x504fbeef,
0x50503efe,
0x50453ce4,
0x5045bd02,
0x50463d15,
0x5046bd2b,
0x50473d3d,
0x5047bd52,
0x50483d78,
0x5048bd86,
0x50493d99,
0x5049bdae,
0x504a3dc4,
0x504abdd4,
0x504b3df4,
0x504bbe07,
0x504c3e2a,
0x504cbe58,
0x504d3e85,
0x504dbea2,
0x504e3ebd,
0x504ebed9,
0x504f3eeb,
0x504fbf02,
0x50503f11,
0x50508729,
0x50513f11,
0x5051bcaf,
0x50523e57,
0x50513f24,
0x5051bcc2,
0x50523e6a,
0x583210b7,
0x5c3293b5,
0x5c3313ce,
Expand Down Expand Up @@ -1534,6 +1535,7 @@ const char kOpenSSLReasonStringData[] =
"SIGNATURE_ALGORITHM_MISMATCH\0"
"UNKNOWN_KEY_TYPE\0"
"UNKNOWN_PURPOSE_ID\0"
"UNKNOWN_SIGID_ALGS\0"
"UNKNOWN_TRUST_ID\0"
"WRONG_LOOKUP_TYPE\0"
"BAD_IP_ADDRESS\0"
Expand Down
1 change: 1 addition & 0 deletions include/openssl/base.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,7 @@ typedef struct X509_name_entry_st X509_NAME_ENTRY;
typedef struct X509_name_st X509_NAME;
typedef struct X509_pubkey_st X509_PUBKEY;
typedef struct X509_req_st X509_REQ;
typedef struct x509_sig_info_st X509_SIG_INFO;
typedef struct X509_sig_st X509_SIG;
typedef struct bignum_ctx BN_CTX;
typedef struct bignum_st BIGNUM;
Expand Down
Loading
Loading