Skip to content

aws: add tinyxml2 dependency#30732

Closed
suniltheta wants to merge 4 commits intoenvoyproxy:mainfrom
suniltheta:tinyxml2
Closed

aws: add tinyxml2 dependency#30732
suniltheta wants to merge 4 commits intoenvoyproxy:mainfrom
suniltheta:tinyxml2

Conversation

@suniltheta
Copy link
Copy Markdown
Contributor

@suniltheta suniltheta commented Nov 5, 2023

Commit Message: aws: add tinyxml2 dependency
Additional Description: We would like to introduce tinyxml2 dependency into Envoy to parse the xml content. The example usecase is shown in one of the unmerged PR 23408. This external library let's us parse the AssumeRoleWithWebIdentityResponse provided by AWS STS service https://docs.aws.amazon.com/STS/latest/APIReference/API_AssumeRoleWithWebIdentity.html#API_AssumeRoleWithWebIdentity_Examples to fetch the AWS credentials for some of the AWS extensions such as AWS Lambda filter, AWS Request Signer filter etc.
Risk Level: -
Testing: NA
Docs Changes: NA
Release Notes: NA
Platform Specific Features: NA


The PR adds a new dependency on https://github.com/leethomason/tinyxml2. Here's the current criteria answers:

Criteria Answer
Cloud Native Computing Foundation (CNCF) approved license Zlib
Dependencies must not substantially increase the binary size unless they are optional (i.e. confined to specific extensions) The dependency is only used in AWS extension and compiled libtinyxml2.a file is 185KB in size.
No duplication of existing dependencies I failed to find any existing xml parser in use within Envoy. If there is I would be open to reuse that.
Hosted on a git repository and the archive fetch must directly reference this repository. We will NOT support intermediate artifacts built by-hand located on GCS, S3, etc. https://github.com/leethomason/tinyxml2
CVE history appears reasonable, no pathological CVE arcs so far 1 CVE related to tinyxml2 have been registered: https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=tinyxml2 which seem to be due to improper use of the library
Code review (ideally PRs) before merge PRs are reviewed before merge
Security vulnerability process exists, with contact details and reporting/disclosure process No documented process can be found, I think raising a github issue to get the maintainer's attention seems to be the best possible way
> 1 contributor responsible for a non-trivial number of commits 107 Contributors
Tests run in CI Tests are run with github actions
High test coverage (also static/dynamic analysis, fuzzing) Good code coverage but no fuzzing
Envoy can obtain advanced notification of vulnerabilities or of security releases Unsure if this is possible. There was only 1 CVE that was reported but even that was disputed for bad API usage reasons
Do other significant projects have shared fate by using this dependency? https://github.com/aws/aws-sdk-cpp
Releases (with release notes) release cut as tags
Commits/releases in last 90 days No. Last commit was in Jan 14, 2023

Signed-off-by: Sunil Narasimhamurthy <sunnrs@amazon.com>
@repokitteh-read-only repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label Nov 5, 2023
@repokitteh-read-only
Copy link
Copy Markdown

CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).
envoyproxy/dependency-shepherds assignee is @moderation

🐱

Caused by: #30732 was opened by suniltheta.

see: more, trace.

Signed-off-by: Sunil Narasimhamurthy <sunnrs@amazon.com>
Sunil Narasimhamurthy added 2 commits November 5, 2023 07:44
Signed-off-by: Sunil Narasimhamurthy <sunnrs@amazon.com>
Signed-off-by: Sunil Narasimhamurthy <sunnrs@amazon.com>
@moderation
Copy link
Copy Markdown
Contributor

moderation commented Nov 6, 2023

OSSF Scorecard not great. 25 commits since last release - leethomason/tinyxml2@9.0.0...master. No security policy. Is the project still active?

scorecard --repo=https://github.com/leethomason/tinyxml2
RESULTS
-------
Aggregate score: 4.3 / 10

Check scores:
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|
|  SCORE  |          NAME          |             REASON             |                                               DOCUMENTATION/REMEDIATION                                               |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|
| 10 / 10 | Binary-Artifacts       | no binaries found in the repo  | https://github.com/ossf/scorecard/blob/70c8e05d6dca69defac0fee1edbbf3fa8732af00/docs/checks.md#binary-artifacts       |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|
| 0 / 10  | Branch-Protection      | branch protection not enabled  | https://github.com/ossf/scorecard/blob/70c8e05d6dca69defac0fee1edbbf3fa8732af00/docs/checks.md#branch-protection      |
|         |                        | on development/release         |                                                                                                                       |
|         |                        | branches                       |                                                                                                                       |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|
| 0 / 10  | CI-Tests               | 0 out of 11 merged PRs         | https://github.com/ossf/scorecard/blob/70c8e05d6dca69defac0fee1edbbf3fa8732af00/docs/checks.md#ci-tests               |
|         |                        | checked by a CI test -- score  |                                                                                                                       |
|         |                        | normalized to 0                |                                                                                                                       |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|
| 0 / 10  | CII-Best-Practices     | no effort to earn an OpenSSF   | https://github.com/ossf/scorecard/blob/70c8e05d6dca69defac0fee1edbbf3fa8732af00/docs/checks.md#cii-best-practices     |
|         |                        | best practices badge detected  |                                                                                                                       |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|
| 6 / 10  | Code-Review            | found 6 unreviewed changesets  | https://github.com/ossf/scorecard/blob/70c8e05d6dca69defac0fee1edbbf3fa8732af00/docs/checks.md#code-review            |
|         |                        | out of 16 -- score normalized  |                                                                                                                       |
|         |                        | to 6                           |                                                                                                                       |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|
| 10 / 10 | Contributors           | project has 7 contributing     | https://github.com/ossf/scorecard/blob/70c8e05d6dca69defac0fee1edbbf3fa8732af00/docs/checks.md#contributors           |
|         |                        | companies or organizations     |                                                                                                                       |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|
| 10 / 10 | Dangerous-Workflow     | no dangerous workflow patterns | https://github.com/ossf/scorecard/blob/70c8e05d6dca69defac0fee1edbbf3fa8732af00/docs/checks.md#dangerous-workflow     |
|         |                        | detected                       |                                                                                                                       |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|
| 0 / 10  | Dependency-Update-Tool | no update tool detected        | https://github.com/ossf/scorecard/blob/70c8e05d6dca69defac0fee1edbbf3fa8732af00/docs/checks.md#dependency-update-tool |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|
| 10 / 10 | Fuzzing                | project is fuzzed              | https://github.com/ossf/scorecard/blob/70c8e05d6dca69defac0fee1edbbf3fa8732af00/docs/checks.md#fuzzing                |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|
| 10 / 10 | License                | license file detected          | https://github.com/ossf/scorecard/blob/70c8e05d6dca69defac0fee1edbbf3fa8732af00/docs/checks.md#license                |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|
| 0 / 10  | Maintained             | 0 commit(s) out of 30 and 0    | https://github.com/ossf/scorecard/blob/70c8e05d6dca69defac0fee1edbbf3fa8732af00/docs/checks.md#maintained             |
|         |                        | issue activity out of 30 found |                                                                                                                       |
|         |                        | in the last 90 days -- score   |                                                                                                                       |
|         |                        | normalized to 0                |                                                                                                                       |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|
| ?       | Packaging              | packaging workflow not         | https://github.com/ossf/scorecard/blob/70c8e05d6dca69defac0fee1edbbf3fa8732af00/docs/checks.md#packaging              |
|         |                        | detected                       |                                                                                                                       |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|
| 0 / 10  | Pinned-Dependencies    | dependency not pinned by hash  | https://github.com/ossf/scorecard/blob/70c8e05d6dca69defac0fee1edbbf3fa8732af00/docs/checks.md#pinned-dependencies    |
|         |                        | detected -- score normalized   |                                                                                                                       |
|         |                        | to 0                           |                                                                                                                       |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|
| 0 / 10  | SAST                   | SAST tool is not run on all    | https://github.com/ossf/scorecard/blob/70c8e05d6dca69defac0fee1edbbf3fa8732af00/docs/checks.md#sast                   |
|         |                        | commits -- score normalized to |                                                                                                                       |
|         |                        | 0                              |                                                                                                                       |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|
| 0 / 10  | Security-Policy        | security policy file not       | https://github.com/ossf/scorecard/blob/70c8e05d6dca69defac0fee1edbbf3fa8732af00/docs/checks.md#security-policy        |
|         |                        | detected                       |                                                                                                                       |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|
| ?       | Signed-Releases        | no releases found              | https://github.com/ossf/scorecard/blob/70c8e05d6dca69defac0fee1edbbf3fa8732af00/docs/checks.md#signed-releases        |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|
| 0 / 10  | Token-Permissions      | detected GitHub workflow       | https://github.com/ossf/scorecard/blob/70c8e05d6dca69defac0fee1edbbf3fa8732af00/docs/checks.md#token-permissions      |
|         |                        | tokens with excessive          |                                                                                                                       |
|         |                        | permissions                    |                                                                                                                       |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|
| 10 / 10 | Vulnerabilities        | 0 existing vulnerabilities     | https://github.com/ossf/scorecard/blob/70c8e05d6dca69defac0fee1edbbf3fa8732af00/docs/checks.md#vulnerabilities        |
|         |                        | detected                       |                                                                                                                       |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|

@suniltheta
Copy link
Copy Markdown
Contributor Author

Seems not a lot of update given it is just basic xml parser.

Do you suggest I try updating the code using https://github.com/zeux/pugixml that seem to be more active and better OSSF Aggregate score: 5.5 / 10

@suniltheta
Copy link
Copy Markdown
Contributor Author

Closing this in favor of #30973

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deps Approval required for changes to Envoy's external dependencies

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants