MDN says the hsts property in the SecurityInfo object is (source here):

true if the host uses Strict Transport Security, false otherwise.

However, when checking HSTS enabled websites like https://hsts.badssl.com, it shows as false (see attached screenshot). From further testing, it looks like this property actually means that that host was preloaded on the HSTS preload list, as it shows as true on https://preloaded-hsts.badssl.com.

Either this property or the documentation needs to be fixed. I would prefer the former and then a new property added to show when the HSTS was preloaded. The max-age value would also be useful here.

Hello,

Can you please describe the exact steps needed in order to reproduce this issue and the webextension used for testing?

Thanks,
Victor

(In reply to vcarciu from comment #1)

Can you please describe the exact steps needed in order to reproduce this issue and the webextension used for testing?

This example extension on MDN here should do the trick, just add console.log(securityInfo); above this line. Then visit any website that uses HSTS, but is not on the HSTS preload list (such as https://hsts.badssl.com). If you check the extension console, the hsts property in that SecurityInfo object will be false when it should be true.

Thank you for detailed steps.
Following the info from comment #2 , I was able to reproduce the issue, so I will mark the bug as confirmed.

We get this flag from nsISiteSecurityService.isSecureURI. I'm otherwise not sure what should happen here. Can you give any insight?

https://searchfox.org/mozilla-central/rev/1ce190047b9556c3c10ab4de70a0e61d893e2954/toolkit/components/extensions/webrequest/SecurityInfo.jsm#192

(In reply to Teal Dulcet [:tdulcet] from comment #2)
Then visit any website that uses HSTS, but is not on the HSTS preload list (such as https://hsts.badssl.com). If you check the extension console, the hsts property in that SecurityInfo object will be false when it should be true.

What happens with "repeat visits"? Is hsts always false? I am assuming that we might write the HSTS status only after the request has succeeded, so the info for the WebExtension might just be too early. The info in IsSecureURI (Cpp implementation) and HostHasHSTSEntry look correct to me. Mhh..

(In reply to Frederik Braun [:freddy] from comment #5)

What happens with "repeat visits"? Is hsts always false?

Yes, by refreshing the page and navigating to other pages on the same domain, the hsts property is always false.

The severity field is not set for this bug.
:mixedpuppy, could you have a look please?

For more information, please visit auto_nag documentation.

Is there anything we can do to make this more consistently correct? Or how should we document this behavior?

I still wonder where the actual bug is. I think this needs a further investigation whether there is an actual discrepancy between the WebExtension API and our internal C++ code for HSTS or if there's an actual HSTS bug. Because I can not seem to find any other known, related HSTS bug where this would be visible...

If this helps, I added some code to my add-on I found this bug with to manually check the HTTP response headers returned from onHeadersReceived for the HSTS header and include a console.assert() statement which verifies that this matches the value of the hsts property (i.e. if it finds the HSTS header than the hsts property should be true). If you install the add-on: https://addons.mozilla.org/firefox/addon/server-status/, open the browser action popup and then check the extension console, you would see an error whenever this is not the case (e.g. currently all websites that support HSTS, but are not on the HSTS preload list, as well as a few incorrectly configured sites on the HSTS preload list).

I would obviously much rather rely on Mozilla's C++ code to find and parse the HSTS headers, as it should be much faster and more robust than my and other extension developer's JS implementations. As you probably know, the HSTS header syntax is rather complex and nontrivial to parse (see RFC 6797). For example, here is the code used by one of the Mozilla Recommended Extensions with over 100K users:

const detectHSTS = (({name, value}) => (name && name.toLowerCase() === "strict-transport-security"
                                    && value && parseInt(value.substring(value.indexOf("=")+1), 10) >= 86400));

which still fails for one of the basic examples provided by RFC 6797 (see section 6.2): Strict-Transport-Security: max-age="31536000". For reference, you can see my code here.

I took a look, and it turns out that there are at least two issues:

1. OriginAttributes mismatch: NS_ShouldSecureUpgrade uses StoragePrincipalHelper::GetOriginAttributesForHSTS, but SecurityInfo.jsm uses channel.loadInfo.originAttributes
2. (not a major cause of this bug, but still wrong nonetheless): TOCTOU where the hsts property reflects the globally known HSTS state instead of the HSTS state at the time of the request.

Freddy, any suggestions here? Would it make sense to store the upgrade flag in LoadInfo? Or should we fix bug 1 without fixing bug 2, by somehow mirroring the logic of StoragePrincipalHelper::GetOriginAttributesForHSTS in SecurityInfo.jsm?

To investgate, I added browser.test.assertFalse(securityInfo.hsts, "hsts property"); after https://searchfox.org/mozilla-central/rev/5ba1bdb6f8fbf8e8c91720b1e584c0081022f1bc/toolkit/components/extensions/test/mochitest/test_ext_webrequest_hsts.html#28-31, and expected it to fail after running:

MOZ_LOG=nsHttp:4,nsSSService:4,sync ./mach test toolkit/components/extensions/test/mochitest/test_ext_webrequest_hsts.html --log-mach-verbose --tag=remote-webextensions --headless

However, the test completed without reporting (expected) failures. Interestingly, the logs did show some entries, so I suspected a potential OriginAttributes mismatch, which I confirmed by adding the following logging after https://searchfox.org/mozilla-central/rev/5ba1bdb6f8fbf8e8c91720b1e584c0081022f1bc/toolkit/components/extensions/webrequest/SecurityInfo.jsm#208

    dump(`HSTSSS ${uri?.host} OA= ${uneval(channel.loadInfo.originAttributes)}\n`);
    for (let o of lazy.sss.enumerate()) {
      dump(`entry: ${o.hostname} ${uneval(o.originAttributes)}\n`);
    }

Output for the test_hsts_request test task (note the differing partitionKey):

0:13.16 GECKO(36383) HSTSSS example.org false ({firstPartyDomain:"", geckoViewSessionContextId:"", inIsolatedMozBrowser:false, partitionKey:"", privateBrowsingId:0, userContextId:0})
0:13.16 GECKO(36383) [Parent 36383: Main Thread]: D/nsSSService Parsing state from 1676295880237,1,0
0:13.16 GECKO(36383) entry: example.org ({firstPartyDomain:"", geckoViewSessionContextId:"", inIsolatedMozBrowser:false, partitionKey:"(http,example.org)", privateBrowsingId:0, userContextId:0})

The HSTS state is read through NS_ShouldSecureUpgrade, which receives its OriginAttributes from StoragePrincipalHelper::GetOriginAttributesForHSTS, which is called at https://searchfox.org/mozilla-central/rev/5ba1bdb6f8fbf8e8c91720b1e584c0081022f1bc/netwerk/protocol/http/nsHttpChannel.cpp#513-517,536-537 .

The implementation of SecurityInfo.jsm passes channel.loadInfo.originAttributes, which is not the same. Consequently, the results are different.

Besides the incorrect parameter, I also see a TOCTOU bug here: SecurityInfo.jsm queries the STS state from the internal STS database instead of from the channel. As a result, the hsts flag does not reflect the STS state of the request, but of the database. In this scenario, the reported STS can be incorrect:

1. Start HTTPS request A, wait for headers and suspend.
2. Query HSTS info with getSecurityInfo for request A.
3. Start HTTPS request B that responds with Strict-Transport-Security header to populate the database.
4. Call getSecurityInfo again for request A.

Expected: step 2 and step 4 should both report false (i.e. the request does not have HSTS).
Actual: step 4 reports hsts: true because step 3 populated the HSTS database.

Freddy - see comment 11 for details. My specific question to you is:

Freddy, any suggestions here? Would it make sense to store the upgrade flag in LoadInfo? Or should we fix bug 1 without fixing bug 2, by somehow mirroring the logic of StoragePrincipalHelper::GetOriginAttributesForHSTS in SecurityInfo.jsm?

The test referenced in comment 11 is currently skipped on macOS/Linux due to intermittents; I tried to fix that in bug 1605515.

I agree that it would be preferable to have that info on the channel and storing that on the loadInfo object makes sense.
Returning the global flag would eventually lead to weird, hard-to-debug extension issues. A per-channel copy seems best.

StoragePrincipalHelper::GetOriginAttributesForHSTS is part of Core::Privacy:Anti-Tracking; moving this bug there.

See comment 11 and later for context. Basically, the work here is to store the HSTS flag in LoadInfo, and using that in SecurityInfo.

Assigning to Hannah. Thank you!

https://hg.mozilla.org/mozilla-central/rev/9789de6c40f0