Conversation
AI-assisted changes.
I played with everything I could think of on the website:
direct debit donation
account creation, login, 2fa enabled, change email, delete account
domain list, filtering
rrset creation and deletion
scrolled through all pages of the web site
nils-wisiol
left a comment
nils-wisiol
left a comment
There was a problem hiding this comment.
review of the first commit
| CELERY_EMAIL_MESSAGE_EXTRA_ATTRIBUTES = ["connection"] | ||
|
|
||
| LIMIT_USER_DOMAIN_COUNT_DEFAULT = 15 | ||
| LIMIT_USER_DOMAIN_COUNT_DEFAULT = None |
| ) | ||
| REGISTER_LPS = bool(int(os.environ.get("DESECSTACK_API_REGISTER_LPS", "1"))) | ||
| _delegation_recheck_raw = os.environ.get( | ||
| "DESECSTACK_API_DELEGATION_SECURE_RECHECK_HOURS", "24" |
There was a problem hiding this comment.
what's the purpose of this? In crontab, we set it to every half hour
There was a problem hiding this comment.
This determines if a domain is skipped by the CLI command or not, not how often the command is run
|
|
||
| # Compute overlap of delegation NS hostnames and IP addresses with ours | ||
| ns_intersection = self.our_ns_set & {name.target for name in answer} | ||
| update["has_all_nameservers"] = ns_intersection == self.our_ns_set |
There was a problem hiding this comment.
don't we want to compute if all desec nameservers are in here? This looks like it computes the check if all nameservers given in NS records are there.
| has_ds = False | ||
| # AD bit indicates the resolver validated the DS answer. | ||
| authenticated = bool(res.flags & dns.flags.AD) | ||
| update["is_secured"] = bool(has_ds and authenticated) |
There was a problem hiding this comment.
I think this computes if the parent zone is correctly signed. We should test for intersection/equality of DS sets, no? Or perhaps successful authentication of the SOA record of domain_name?
|
|
||
| def get_insecure_delegated_domains(self, obj): | ||
| return obj.domains.filter(is_registered=True, is_delegated=True).exclude( | ||
| is_secured=True |
There was a problem hiding this comment.
this includes undelegated 'toy' domains, so they count towards the limit - do we want that?
| self.assertContains( | ||
| response, | ||
| "Insecure delegation limit", | ||
| status_code=status.HTTP_403_FORBIDDEN, |
There was a problem hiding this comment.
this should be accepted as the domain is guaranteed to be delegated securely and thus shouldn't count towards the insecure domain limit
| self.assertContains( | ||
| response, | ||
| "Insecure delegation limit", | ||
| status_code=status.HTTP_403_FORBIDDEN, |
There was a problem hiding this comment.
same comment as above
| insecure_count = request.user.domains.filter( | ||
| is_registered=True, is_delegated=True | ||
| ).exclude(is_secured=True).count() | ||
| return insecure_count < limit |
There was a problem hiding this comment.
grant permission if the domain in question is an autodelegation domain
|
|
||
| The API does not change or delete domains based on these checks. However, if | ||
| you have at least one domain delegated to deSEC without DNSSEC, you cannot | ||
| create additional domains until that domain is secured. |
There was a problem hiding this comment.
rephrase to accurately reflect limit policy
2 participants
delegation checker run times on my laptop on a 1000 domains sample (5 repeats):
The delegation checker is not comparing keys against nslord to avoid making many requests to nslord.