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

Get-ScubaSpfRecord returns in rdata all TXT records not only SPF #1510

Open
slavag opened this issue Jan 10, 2025 · 5 comments
Open

Get-ScubaSpfRecord returns in rdata all TXT records not only SPF #1510

slavag opened this issue Jan 10, 2025 · 5 comments
Labels
public-reported This issue is reported by the public users of the tool.

Comments

@slavag
Copy link

slavag commented Jan 10, 2025

🐛 Summary

Get-ScubaSpfRecord returns in rdata all TXT records not only SPF

To reproduce

Check on domains with TXT records, but not SPF.

Expected behavior

Only SPF records to be present in rdata

@buidav buidav added the public-reported This issue is reported by the public users of the tool. label Jan 13, 2025
@adhilto
Copy link
Collaborator

adhilto commented Jan 13, 2025

Hi, thanks for reaching out. There are couple of moving pieces here. The "provider" module, which pulls all the data for ScubaGear, doesn't filter the TXT records returned, as you say. But the Rego code, the code that actually evaluates the policies, does filter the TXT records returned to just the SPF records, so the report produced will still be accurate, though there may be some superfluous records in the raw data stored in the ScubaResults.json file. Is there a reason this might cause a problem for your use case? Though on further reflection, I can see that there is a slight discrepancy between the function name and what it actually does, which certainly isn't good practice. Either way, curious to hear what additional thoughts you might have on this issue.

@slavag
Copy link
Author

slavag commented Jan 13, 2025

Hi,
Thanks for your reply.
Well, I don't have any specific issue, but just tried to understand the code and found that it's contains not only SPF data, and also number of records in the log entries is not exactly correct.
So, I was thinking that it worth check with the dev team (for me it's looks like a bug :) ).
Thanks

@adhilto
Copy link
Collaborator

adhilto commented Jan 13, 2025

Got it. This does look like something we'll eventually want to fix, if nothing else so the functionality of Get-ScubaSpfRecord more closely aligns with what it actually does. But in the meantime, rest assured that the extra records are not skewing the assessment results.

Thanks again for reporting!

@hkelley
Copy link

hkelley commented Jan 19, 2025

I think there is a related issue where Get-ScubaSpfRecord is splitting long TXT records over multiple items in rdata, which causes the v=spf1 and -all to be separated from one another. This, in turn, may be causing this evaluation to fail:

SpfRecords := {Record | some Record in DNSResponse.rdata; startswith(Record, "v=spf1 "); contains(Record, "-all")} |

DomainsWithoutSpf contains DNSResponse.domain if {
    some DNSResponse in input.spf_records
    SpfRecords := {Record | some Record in DNSResponse.rdata; startswith(Record, "v=spf1 "); contains(Record, "-all")} | 

When I run this over my domain:

$result = Get-ScubaSpfRecord -Domains @([pscustomobject] @{DomainName='REDACTED.com'})

I get four items for rdata:

> $result.rdata.Count
4

> for($i=0;$i -lt $result.rdata.Count; $i++) {Write-Host "`nitem $i"; Write-Host $result.rdata[$i]}

item 0
MS=ms445xxx

item 1
cisco-ci-domain-verification=xxxxxx

item 2
v=spf1 a ip4:999.999.999.169 ip4:999.999.999.170 ip4:999.999.999.152 ip4:999.999.999.140 ip4:999.999.999.77 ip4:999.999.999.72 ip4:999.999.999.102 ip4:999.999.999.103 ip4:999.4.999.149 ip4:999.999.999.237 ip4:999.999.999.170 include:spf-000xxxxxx.pphosted.com include:

item 3
e2ma.net include:spf.protection.outlook.com include:amazonses.com include:_spf.salesforce.com ip4:999.999.999.196 ip4:999.999.999.174 ip4:999.999.999.207 ip4:999.999.999.142 ip4:999.999.999.250 ip4:999.1.999.157 -all

@adhilto
Copy link
Collaborator

adhilto commented Jan 27, 2025

@hkelley Thanks for the report. I created a separate issue to track that: #1534

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
public-reported This issue is reported by the public users of the tool.
Projects
None yet
Development

No branches or pull requests

4 participants