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

x509 attribute cert support #7926

Merged
merged 2 commits into from
Sep 14, 2024
Merged

Conversation

philljj
Copy link
Contributor

@philljj philljj commented Aug 30, 2024

Description

Initial support for rfc5755 x509 attribute certificates (acerts). Enabled with --enable-acert.

Supports reading, printing, verifying.

No support for signing or generating yet though.

The Attributes field is not parsed, but can be accessed through the API wolfSSL_X509_ACERT_get_attr_buf(). To facilitate user parsing of the attributes field, the ASN functions GetLength() and GetASNTag() were changed to WOLFSSL_ASN_API.

Testing

Tested with unit tests in this example repo:

Added new tests to tests/api.c:

  • test_wolfSSL_X509_ACERT_verify
  • test_wolfSSL_X509_ACERT_misc_api

Added new rsa and rsa_pss acerts to certs/acert/. These were signed with https://github.com/philljj/acert-test.

Tests require this build config:

./configure --enable-acert --enable-opensslextra --enable-rsapss

Examples

Added acert example to wolfssl-examples:

Copy link
Contributor

@douzzer douzzer left a comment

Choose a reason for hiding this comment

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

most of the defects discovered using
ENABLE_ALL_TEST_FLAGS='--enable-all --enable-testcert --enable-acert' ../testing/git-hooks/wolfssl-multi-test.sh --keep-going --max-check-try-count=2 --enable-bwrap --verbose-analyzers --enable-text-styles --report-cumulative-times --no-result-cache --no-git-refresh --test-uncommitted super-quick-check

(word32)pubKeyOID, sig, sigSz, sigOID,
sigParams, sigParamsSz, NULL);

if (ret == ASN_SIG_CONFIRM_E) {
Copy link
Contributor

Choose a reason for hiding this comment

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

ASN_SIG_CONFIRM_E needs a WC_NO_ERR_TRACE() wrapper here.

* @return MEMORY_E when dynamic memory allocation fails.
*/
static int DecodeAcertGeneralName(const byte* input, word32* inOutIdx, byte tag,
int len, DecodedAcert* acert, DNS_entry** entries)
Copy link
Contributor

Choose a reason for hiding this comment

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

line length


/* GeneralName choice: dnsName */
if (tag == (ASN_CONTEXT_SPECIFIC | ASN_DNS_TYPE)) {
ret = SetDNSEntry(acert->heap, (const char*)(input + idx), len, ASN_DNS_TYPE,
Copy link
Contributor

Choose a reason for hiding this comment

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

line length

}
#endif

ret = SetDNSEntry(acert->heap, (const char*)(input + idx), len, ASN_URI_TYPE, entries);
Copy link
Contributor

Choose a reason for hiding this comment

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

line length

defined(WOLFSSL_IP_ALT_NAME)
/* GeneralName choice: iPAddress */
else if (tag == (ASN_CONTEXT_SPECIFIC | ASN_IP_TYPE)) {
ret = SetDNSEntry(acert->heap, (const char*)(input + idx), len, ASN_IP_TYPE, entries);
Copy link
Contributor

Choose a reason for hiding this comment

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

line length

int badDate = 0;
byte version = 0;
(void) verify;
word32 serialSz = EXTERNAL_SERIAL_SIZE;
Copy link
Contributor

Choose a reason for hiding this comment

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

ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
40809 |     word32 serialSz = EXTERNAL_SERIAL_SIZE;
      |     ^~~~~~

if (dataASN[ACERT_IDX_ACINFO_ALGOID_OID].data.oid.sum
!= dataASN[ACERT_IDX_SIGALGO_OID].data.oid.sum) {
WOLFSSL_MSG("error: VerifyX509Acert: sig OID mismatch");
FREE_ASNGETDATA(dataASN, acert->heap);
Copy link
Contributor

Choose a reason for hiding this comment

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

error: request for member ‘heap’ in something not a structure or union
41123 |         FREE_ASNGETDATA(dataASN, acert->heap);
      |                                       ^~


if (acinfo == NULL || acinfoSz == 0) {
WOLFSSL_MSG("error: VerifyX509Acert: empty acinfo");
FREE_ASNGETDATA(dataASN, acert->heap);
Copy link
Contributor

Choose a reason for hiding this comment

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

error: request for member ‘heap’ in something not a structure or union
41123 |         FREE_ASNGETDATA(dataASN, acert->heap);
      |                                       ^~


if (ret != 0) {
WOLFSSL_MSG("error: VerifyX509Acert: GetASN_Items failed");
FREE_ASNGETDATA(dataASN, acert->heap);
Copy link
Contributor

Choose a reason for hiding this comment

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

error: request for member ‘heap’ in something not a structure or union
41123 |         FREE_ASNGETDATA(dataASN, acert->heap);
      |                                       ^~

heap);
}

FREE_ASNGETDATA(dataASN, acert->heap);
Copy link
Contributor

Choose a reason for hiding this comment

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

error: request for member ‘heap’ in something not a structure or union
41123 |         FREE_ASNGETDATA(dataASN, acert->heap);
      |                                       ^~

@douzzer douzzer assigned philljj and unassigned wolfSSL-Bot Sep 12, 2024
@philljj
Copy link
Contributor Author

philljj commented Sep 13, 2024

Retest this please

@philljj philljj requested a review from douzzer September 13, 2024 15:15
@philljj philljj removed their assignment Sep 13, 2024
@douzzer douzzer merged commit 80f3b0d into wolfSSL:master Sep 14, 2024
135 checks passed
@philljj philljj deleted the x509_acert_support branch September 15, 2024 18:09
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.

3 participants