noggin icon indicating copy to clipboard operation
noggin copied to clipboard

Security Audit of Noggin Code Complete

Open amoloney1 opened this issue 4 years ago • 5 comments

The code for Noggin needs to be audited for potential security risks and passed before deployment to staging.

  • Add security headers https://github.com/fedora-infra/noggin/issues/333
    • Add justification lines to https://github.com/fedora-infra/noggin/issues/335
    • Review default config https://github.com/fedora-infra/noggin/issues/334 Move installation docs from gdoc to readthedocs

Acceptance Criteria:

  • The code is audited by a security personnel from Fedora
  • Any security issues with the code are found, reported to the Noggin team using a Security Audit label and then are fixed
  • The code passes all security reviews and signed off on

Definition of Done:

  • [ ] Code passes security checks
  • [ ] Installation Docs available
  • [ ] Security headers updates
  • [ ] Config notes reviewed

@puiterwijk Can you please action this request? Thank you kindly!

amoloney1 avatar Jul 20 '20 10:07 amoloney1

I will try to schedule this soon, is there any deadline when this is wanted? Additionally, how much flux can/should I still expect of the code, since a security audit is mostly useful after at least the security-sensitive code has stabilized somewhat.

puiterwijk avatar Jul 21 '20 10:07 puiterwijk

The only big change in the code that is incoming is us using the non-legacy API of python_freeipa. That's going to change a lot of lines in a lot of files but it should end up in the exact same operations with FreeIPA, so I don't think it'll change much from a security point of view. Apart from that, the code should be mostly done.

abompard avatar Jul 22 '20 14:07 abompard

Hi @puiterwijk would you have an estimate date on when you will be able to complete the security audit please? The team are hoping to deploy Noggin in Staging by Aug 10th so if we could get the code reviewed before then, time willing of course, that would be great! And thanks @abompard for adding more information to this ticket also! :)

amoloney1 avatar Jul 30 '20 10:07 amoloney1

For reference, the commit hashes currently under audit:

puiterwijk avatar Aug 12 '20 11:08 puiterwijk

According to the Fedora Infrastructure Application Security Policy, any deviations from the policy must be pointed out in the request for the security audit. I cannot find any notes in this ticket to that extend, so I'm now adding that here:

The violated sections of the Application Security Policy and their justifications:

  • Authentication: noggin requires a token for direct access to IPA, and this is not possible with OpenID Connect. Additionally, there is no API in noggin.
  • Authorization: noggin doesn't use OpenID Connect per the previous justification

These justifications have been accepted as part of the security audit, so these sections will not apply to noggin for the purpose of this audit.

puiterwijk avatar Aug 12 '20 12:08 puiterwijk