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

[Core] CredScan: handle false positives #23505

Closed
kinelski opened this issue Aug 24, 2021 · 4 comments · Fixed by #23653
Closed

[Core] CredScan: handle false positives #23505

kinelski opened this issue Aug 24, 2021 · 4 comments · Fixed by #23653
Assignees
Labels
Azure.Core Client This issue points to a problem in the data-plane of the library. EngSys This issue is impacting the engineering system.

Comments

@kinelski
Copy link
Member

Issue

We currently have 2 test secrets in Core Track 2 library being detected by CredScan (report).

These are the secrets being detected:
1.

-----BEGIN PRIVATE KEY-----
MIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQDbRUT3kwyXKfvK
7vpeYQ8FXkaPtKfQHNq8ZNhzj3sddGnLgRzx51fkLq7tCk755A5WUUBt7IiDX8gw
Z2h3xC5qvHXWaoLIGFdWVK2uOqg1MijTQ4D0QWtaORrZlisvGWdLqT/DaTXydAT4
U/rcMhfU6RdTdPEd9WFlbAeA5urmNbPHgSMbfuGdhImScAH/vU31/aDrdxQzbwdw
YN8zM2sSFuT5OkV8aDetmKiwfJsPIMfHzyNPFHEVgGJpZCk07NkD4z1dR5Qret3E
5+PkkvwTFYa48yP1A0oZwqRxysugy0zmEU/1Hx/2cF55iuYBSxnBTsdZR9pPNYfi
ChZ9lEVBAgMBAAECggEBALOLCcjrCLEyeHGXszzanrEXnBGJrKt1JQqETNR7FSVu
MD6pjyjo9IfsTeHcwgROYAr/5xDbUIC6SjKQSfNfmV5eyOJ0PnhXrhQLrFdwwlEk
rFco/AuFEcFD5x9pnhyn4XQLtyFsIfdQVs17/nqVLfxxjF8S72hHh6PDjHVZH9i1
daF4woC7obyKqv8+uoc/MEqTgE/bbaQoL8tDUhCcuoKjcBEXpRryWFaeMoolnuTU
D7656CHKFAnloGpNaeNPy8XmpicRxHEODXSoEZDxYoj3SSbPqn/7FDzHqh/Fk3kD
tfjhYdhwZFrlTkvC766DxEBgIQHvJbdjujsX4uLJVQECgYEA/HoOKmA2KgK+f126
oKmF38o4S/8hBd4vT7Qq2Q88zFom7MWrdlHdg5WO0ZLXS+l6LTOVkS2NM+WtbeeF
5el4ICEl0x/Z1/e3vZl0QTDDOKqzfWsVxNwM9FD2bmVjVZHU0o9DPy45ugZdV0ti
9YTm9TLIachAjU5vIIwz6CJ5g9ECgYEA3lSWtSeo76q0hlXE/FN5jn9BeuZHYmCJ
aHYhtG50tXUxghwPRzfTVoQFbUQa1w/Ex4Ny2xZ+OnQTzdBjC13Xz2PAaldrNYlH
5C/LjhbFdOvuQWU3nli9wXq25GTvzla3F5dn0NCUP3xwUIMld1Yq7mCaIy9HrP4i
gn6jZimzNnECgYEAsXdR0bODHxCCuqA1eIzwTxejbrfMjIVamBm6LIyrXbDYv4FK
PobYv482rlUbBH7+pBsoPL3aDOqadzBQTAVJrLvgDIDM7SNwgdMFhnUyI/jI5ZWJ
3bAXYvwt2/dkVVeGUuLkj9p8NSgYIC4bRxy+AwhJGyHpTeod7rDeI4NoCqECgYB1
DKWXU/ztyLpn6undSfk6GycXE/tLALX2yBKwkmJhUgSxkiI9BVf/OVw+DVfwF34q
57plO69TCN+QQICUcGB47/RSSBnKQq8VpFAPS0/DYZ660RX6CJBGN1voXHef8ylL
g0uFtPoHfnUG/jSQYk4R18vucCrVGaqDdzaBR7zxEQKBgCEqovhMGJ1xOrkzetBB
+zgh5zJbAWx5DQk5ZdmAcAnEeqconM2yhFB636wC07UbeAZaQmhB5kQYMOuiCstt
30sdQlNG9EGdqNsoVn/363Cg1iKJy4JU5uW/5kjh4UfBZG6DDwjLK88ZWh0OHPRV
h8q0or9YnvqnVrELMR8cjUkZ
-----END PRIVATE KEY-----

2.
-----BEGIN PRIVATE KEY-----
MIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQDbRUT3kwyXKfvK
7vpeYQ8FXkaPtKfQHNq8ZNhzj3sddGnLgRzx51fkLq7tCk755A5WUUBt7IiDX8gw
Z2h3xC5qvHXWaoLIGFdWVK2uOqg1MijTQ4D0QWtaORrZlisvGWdLqT/DaTXydAT4
U/rcMhfU6RdTdPEd9WFlbAeA5urmNbPHgSMbfuGdhImScAH/vU31/aDrdxQzbwdw
YN8zM2sSFuT5OkV8aDetmKiwfJsPIMfHzyNPFHEVgGJpZCk07NkD4z1dR5Qret3E
5+PkkvwTFYa48yP1A0oZwqRxysugy0zmEU/1Hx/2cF55iuYBSxnBTsdZR9pPNYfi
ChZ9lEVBAgMBAAECggEBALOLCcjrCLEyeHGXszzanrEXnBGJrKt1JQqETNR7FSVu
MD6pjyjo9IfsTeHcwgROYAr/5xDbUIC6SjKQSfNfmV5eyOJ0PnhXrhQLrFdwwlEk
rFco/AuFEcFD5x9pnhyn4XQLtyFsIfdQVs17/nqVLfxxjF8S72hHh6PDjHVZH9i1
daF4woC7obyKqv8+uoc/MEqTgE/bbaQoL8tDUhCcuoKjcBEXpRryWFaeMoolnuTU
D7656CHKFAnloGpNaeNPy8XmpicRxHEODXSoEZDxYoj3SSbPqn/7FDzHqh/Fk3kD
tfjhYdhwZFrlTkvC766DxEBgIQHvJbdjujsX4uLJVQECgYEA/HoOKmA2KgK+f126
oKmF38o4S/8hBd4vT7Qq2Q88zFom7MWrdlHdg5WO0ZLXS+l6LTOVkS2NM+WtbeeF
5el4ICEl0x/Z1/e3vZl0QTDDOKqzfWsVxNwM9FD2bmVjVZHU0o9DPy45ugZdV0ti
9YTm9TLIachAjU5vIIwz6CJ5g9ECgYEA3lSWtSeo76q0hlXE/FN5jn9BeuZHYmCJ
aHYhtG50tXUxghwPRzfTVoQFbUQa1w/Ex4Ny2xZ+OnQTzdBjC13Xz2PAaldrNYlH
5C/LjhbFdOvuQWU3nli9wXq25GTvzla3F5dn0NCUP3xwUIMld1Yq7mCaIy9HrP4i
gn6jZimzNnECgYEAsXdR0bODHxCCuqA1eIzwTxejbrfMjIVamBm6LIyrXbDYv4FK
PobYv482rlUbBH7+pBsoPL3aDOqadzBQTAVJrLvgDIDM7SNwgdMFhnUyI/jI5ZWJ
3bAXYvwt2/dkVVeGUuLkj9p8NSgYIC4bRxy+AwhJGyHpTeod7rDeI4NoCqECgYB1
DKWXU/ztyLpn6undSfk6GycXE/tLALX2yBKwkmJhUgSxkiI9BVf/OVw+DVfwF34q
57plO69TCN+QQICUcGB47/RSSBnKQq8VpFAPS0/DYZ660RX6CJBGN1voXHef8ylL
g0uFtPoHfnUG/jSQYk4R18vucCrVGaqDdzaBR7zxEQKBgCEqovhMGJ1xOrkzetBB
+zgh5zJbAWx5DQk5ZdmAcAnEeqconM2yhFB636wC07UbeAZaQmhB5kQYMOuiCstt
30sdQlNG9EGdqNsoVn/363Cg1iKJy4JU5uW/5kjh4UfBZG6DDwjLK88ZWh0OHPRV
h8q0or9YnvqnVrELMR8cjUkZ
-----END PRIVATE KEY-----";

Goal

We need to move the certificate keys to a separate file and make tests read them from there. After that's done, the file path must be added to the suppression file as well:

"sdk/identity/Azure.Identity/tests/Data/cert.pfx",
"sdk/identity/Azure.Identity/tests/Data/cert-invalid-data.pem",
"sdk/identity/Azure.Identity/tests/Data/cert-password-protected.pfx",
"sdk/identity/Azure.Identity/tests/Data/cert.pem",
"sdk/identity/Azure.Identity/tests/Data/mock-arc-mi-key.key",
"sdk/storage/Microsoft.Azure.WebJobs.Extensions.Storage.Common/tests/cert.pem"

Here's an example.

@kinelski kinelski added Client This issue points to a problem in the data-plane of the library. EngSys This issue is impacting the engineering system. Azure.Core labels Aug 24, 2021
@pakrym pakrym assigned heaths and unassigned pakrym Aug 24, 2021
@pakrym
Copy link
Contributor

pakrym commented Aug 24, 2021

@heaths, can you look at this, please?

@heaths
Copy link
Member

heaths commented Aug 24, 2021

@kinelski can't we just add azure-sdk-for-net/sdk/core/Azure.Core/tests/PemReaderTests.cs to the suppression list?

@kinelski
Copy link
Member Author

@kinelski can't we just add azure-sdk-for-net/sdk/core/Azure.Core/tests/PemReaderTests.cs to the suppression list?

I would advise against doing so because this would prevent us from identifying possible other leaks in this file in the future. I think actual leaks are only likely to happen in recording files, though. @weshaggard What do you think?

@weshaggard
Copy link
Member

We have been trying to avoid adding entire source files to the suppression list unless the file is known to only contain test/fake secrets like a cert file or a fakes file but for any standard cs files we don't add them to the suppression list to avoid potential leaks.

heaths added a commit to heaths/azure-sdk-for-net that referenced this issue Aug 30, 2021
heaths added a commit that referenced this issue Sep 3, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Azure.Core Client This issue points to a problem in the data-plane of the library. EngSys This issue is impacting the engineering system.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants